1219530 - Add a MOZ_RUST conditional #define for the mp4parse-rust library

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Matthew Gregan [:kinetik]]

build/autoconf/rust.m4 needs to AC_DEFINE(MOZ_RUST) when --enable-rust succeeds so that existing code calling into Rust code can be built conditionally.

Comment 1

[Matthew Gregan [:kinetik]]

Attachment: bug1219530_v0.patch

Comment 2

[Mike Hommey [:glandium]]

Comment on attachment 8680424 [details] [diff] [review]
bug1219530_v0.patch

Review of attachment 8680424 [details] [diff] [review]:
-----------------------------------------------------------------

I think a better thing to do is to add ad-hoc defines for each rust library/feature that's going to be used, possibly triggered by confvars.sh variables, or something similar.

Comment 3

[Matthew Gregan [:kinetik]]

That makes sense; I'll morph this bug to cover this for our use case.

Comment 4

[Ralph Giles (:rillian) | needinfo me]

Attachment: Add MOZ_RUST_MP4PARSE

Is this what you had in mind?

Define a new MOZ_RUST_MP4PARSE switch in browser/confvars.sh. Check for this in configure, and if rust is enabled and the toolchain is available (MOZ_RUST) set a cpp define with the same name.

Don't provide a user-facing configure --enable switch per irc discussion with Ted.

FWIW I disagree with this approach for the mp4parse code; we don't intend this to optional code, so gating on the presence of the rust toolchain is sufficient. I'm happy to lead by example if other groups want finer-grained build configuration though.

Comment 5

[Mike Hommey [:glandium]]

Comment on attachment 8682079 [details] [diff] [review]
Add MOZ_RUST_MP4PARSE

Review of attachment 8682079 [details] [diff] [review]:
-----------------------------------------------------------------

::: configure.in
@@ +4030,5 @@
>      AC_DEFINE(MOZ_B2GDROID)
>  fi
>  
> +# Propagate feature switches for code written in rust from confvars.sh
> +if test -n "MOZ_RUST" -a -n "MOZ_RUST_MP4PARSE"; then

To make it easier for additional things to enter the picture, it might be better to make it two nested ifs.

Comment 6

[Ralph Giles (:rillian) | needinfo me]

Attachment: Add MOZ_RUST_MP4PARSE v2

Nest configure conditionals to make it easier to add other rust feature tests. Carrying forward r=glandium. Thanks for the review!

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ce56b70293c6

Comment 8

[[e-mail]]

Attachment: correct

Comment 9

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/e2bfee574e4e - apparently, curiously enough, that builds on OS X debug, but every other flavor of every platform says things like https://treeherder.mozilla.org/logviewer.html#?job_id=16711985&repo=mozilla-inbound

Comment 10

[Ralph Giles (:rillian) | needinfo me]

Gah. Thanks for the fix.

I guess android libstdc++ is still pretty old.

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/43ce9581fb64

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/43ce9581fb64

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/43ce9581fb64