1284589 - Use cargo package layout for mp4parse

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect)

Description

[Ralph Giles (:rillian)]

Nathan asked about building the in-tree mp4parse code with cargo. This bug is about importing Cargo.toml and switching to the standard cargo source package layout for the in-tree copy to make this easier. No actual build changes are intended at this point.

Comment 1

[Ralph Giles (:rillian)]

Attachment: Bug 1284589 - Move mp4parse source to match upstream.

Make Cargo.toml, build.rs and standard cargo source package
layout available in-tree to facilitate testing cargo-driven
builds of rust code.

Update the moz.build script to build using plain rustc as
before, but referencing the new source location.

Review commit: https://reviewboard.mozilla.org/r/62432/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/62432/

Comment 2

[Ralph Giles (:rillian)]

Attachment: Bug 1284589 - Result of running the update script.

Implement the changes by running the new update script.

Review commit: https://reviewboard.mozilla.org/r/62434/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/62434/

Comment 3

[Ralph Giles (:rillian)]

Note this still relies on a checked-in copy of the generated mp4parse.h C api header. We'd like cargo to generate this eventually of course. Building `term` and `syntex_syntax` for rusty-cheddar is going to add a few minutes to the clobber build time.

I left byteorder as a module, since making `extern crate` work is still to-do.

Comment 4

[Nathan Froyd [:froydnj]]

The second patch appears to completely remove the sources, and doesn't update moz.build to reflection that removal.  Was that intended?  AFAIK, we want the Rust sources checked into the tree all the time...

Comment 5

[Ralph Giles (:rillian)]

Oops, forgot to add the files at the new locations. Thanks!

Comment 6

[Ralph Giles (:rillian)]

Comment on attachment 8768119 [details]
Bug 1284589 - Result of running the update script.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62434/diff/1-2/

Comment 7

[Matthew Gregan [:kinetik]]

Comment on attachment 8768118 [details]
Bug 1284589 - Move mp4parse source to match upstream.

https://reviewboard.mozilla.org/r/62432/#review59258

Comment 8

[Matthew Gregan [:kinetik]]

Comment on attachment 8768119 [details]
Bug 1284589 - Result of running the update script.

https://reviewboard.mozilla.org/r/62434/#review59260

Comment 9

[Matthew Gregan [:kinetik]]

(In reply to Ralph Giles (:rillian) needinfo me from comment #3)
> Note this still relies on a checked-in copy of the generated mp4parse.h C
> api header. We'd like cargo to generate this eventually of course. Building
> `term` and `syntex_syntax` for rusty-cheddar is going to add a few minutes
> to the clobber build time.

We can also avoid that by switching to rust-bindgen without too much pain (just a bit of grunt work) if that work is already happening (or done) for Stylo.

Comment 10

[Ralph Giles (:rillian)]

(In reply to Matthew Gregan [:kinetik] from comment #9)

> We can also avoid that by switching to rust-bindgen without too much pain
> (just a bit of grunt work) if that work is already happening (or done) for
> Stylo.

Right. Small matter of adding libclang as a build dependency. :)

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/03ae19f02e2a
Move mp4parse source to match upstream. r=kinetik
https://hg.mozilla.org/integration/autoland/rev/452635f51360
Result of running the update script. r=kinetik

Comment 12

[Gregory Szorc [:gps]]

https://hg.mozilla.org/mozilla-central/rev/03ae19f02e2a
https://hg.mozilla.org/mozilla-central/rev/452635f51360