Closed Bug 1378410 Opened 3 years ago Closed 2 years ago

Add Nightly-only build flag support to generated bindings

Categories

(Firefox Build System :: Android Studio and Gradle Integration, enhancement)

All
Android
enhancement
Not set

Tracking

(firefox57 fixed)

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: jchen, Assigned: jchen)

References

(Blocks 1 open bug)

Details

Attachments

(5 files, 3 obsolete files)

We talked about adding preprocessing support to the generated JNI bindings so that a Nightly-only feature will not cause builds to permafail once they move to Beta (e.g. bug 1368925).

Nick mentioned in bug 1368954 comment 3 that this doesn't cover all cases. In particular, it does not support Beta-only build flags. Nevertheless, I think it's still worth a shot, because "Nightly-only" should cover enough use cases, and adding preprocessing is easy enough to implement. The alternative is generating and using fresh bindings for every build, but that's a tough problem to solve given the existing Fennec build process.
(In reply to Jim Chen [:jchen] [:darchons] from comment #0)
> We talked about adding preprocessing support to the generated JNI bindings
> so that a Nightly-only feature will not cause builds to permafail once they
> move to Beta (e.g. bug 1368925).
> 
> Nick mentioned in bug 1368954 comment 3 that this doesn't cover all cases.
> In particular, it does not support Beta-only build flags. Nevertheless, I
> think it's still worth a shot,

I concur.  Roll on!
Add a BuildFlag annotation, which when specified for classes, will wrap
generated code in `#ifdef` or `#ifndef` blocks. This functionality is
used for conditionally excluding generated code when NIghtly becomes
Beta, without the need to regenerate bindings.
Attachment #8884906 - Flags: review?(snorp)
Preprocess the generated bindings to support the new BuildFlag
annotation, so that we can compare bindings despite build flag changes.

The build system preprocessor is used because it's easy-to-use and
invoking the actual C++ preprocessor would require much more work.
However, as a result, we need to use a custom preprocessing marker (//#)
so that the preprocessor doesn't try to process files from `#include`
lines in the binding files.
Attachment #8884907 - Flags: review?(nalexander)
Comment on attachment 8884906 [details] [diff] [review]
1. Support BuildFlag annotation for generated bindings (v1)

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

lgtm with nits

::: build/annotationProcessors/utils/Utils.java
@@ +331,5 @@
> +    public static String getIfdefHeader(String ifdef) {
> +        if (ifdef.isEmpty()) {
> +            return "";
> +        } else if (ifdef.startsWith("!")) {
> +            return "//#ifndef " + ifdef.substring(1) + "\n";

Remove // here and other places? Debug code?
Attachment #8884906 - Flags: review?(snorp) → review+
Comment on attachment 8884906 [details] [diff] [review]
1. Support BuildFlag annotation for generated bindings (v1)

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

This approach looks good!

::: build/annotationProcessors/utils/Utils.java
@@ +328,5 @@
>          }
>      }
> +
> +    public static String getIfdefHeader(String ifdef) {
> +        if (ifdef.isEmpty()) {

Considering being defensive: `ifdef == null || ...`.

@@ +332,5 @@
> +        if (ifdef.isEmpty()) {
> +            return "";
> +        } else if (ifdef.startsWith("!")) {
> +            return "//#ifndef " + ifdef.substring(1) + "\n";
> +        }

Jim addresses why this is necessary: this will be preprocessed once (at binding generation time) and then again (at compile time).  Therefore, you need two preprocessing tokens.

::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/annotation/BuildFlag.java
@@ +21,5 @@
> +     * Preprocessor macro for conditionally building the generated bindings.
> +     * "MOZ_FOO" wraps generated bindings in "#ifdef MOZ_FOO / #endif"
> +     * "!MOZ_FOO" wraps generated bindings in "#ifndef MOZ_FOO / #endif"
> +     */
> +    String value() default "";

Can you confirm that you use this like:
```
@BuildFlag(value="!MOZ_FOO")
```

If that's correct, can I gently suggest using
```
@BuildFlag(ifdef="MOZ_FOO")
@BuildFlag(ifndef="MOZ_BAR")
```
I find what you've expressed both too general ("value" could be all sorts of things) and not general enough (there are other blocks besides ifdef).
Attachment #8884906 - Flags: review+
Comment on attachment 8884907 [details] [diff] [review]
2. Preprocess generated bindings (v1)

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

I'm confident that this will work, but I can't really read it.  This section is getting larger and growing functionality.

I'd like to move this to Python, which is a lot easier to read and to test.  Can you try to express this using `GENERATED_FILES`, like at http://searchfox.org/mozilla-central/source/mobile/android/base/moz.build#137?  You can use that file as a template or add a new function to it.

You should know that my end goal for this code is that we upgrade to the Android-Gradle build plugin (https://bugzilla.mozilla.org/show_bug.cgi?id=1366644), it becomes a real Java annotation processor, and that we interleave the Gradle build stages with the C++ compile stages so that we get rid of the diffing and really build the C++ before it is used.
Attachment #8884907 - Flags: review?(nalexander) → feedback+
> Can you confirm that you use this like:
> ```
> @BuildFlag(value="!MOZ_FOO")
> ```
> 
> If that's correct, can I gently suggest using
> ```
> @BuildFlag(ifdef="MOZ_FOO")
> @BuildFlag(ifndef="MOZ_BAR")
> ```
> I find what you've expressed both too general ("value" could be all sorts of
> things) and not general enough (there are other blocks besides ifdef).

You use it as `@BuildFlag("MOZ_FOO")` or `@BuildFlag("!MOZ_FOO")` (naming the field "value" makes it the default), but I can make it `ifdef=/ifndef=` (though to me `BuildFlag("MOZ_FOO")` seems more clear).
(In reply to Jim Chen [:jchen] [:darchons] from comment #7)
> > Can you confirm that you use this like:
> > ```
> > @BuildFlag(value="!MOZ_FOO")
> > ```
> > 
> > If that's correct, can I gently suggest using
> > ```
> > @BuildFlag(ifdef="MOZ_FOO")
> > @BuildFlag(ifndef="MOZ_BAR")
> > ```
> > I find what you've expressed both too general ("value" could be all sorts of
> > things) and not general enough (there are other blocks besides ifdef).
> 
> You use it as `@BuildFlag("MOZ_FOO")` or `@BuildFlag("!MOZ_FOO")` (naming
> the field "value" makes it the default), but I can make it `ifdef=/ifndef=`
> (though to me `BuildFlag("MOZ_FOO")` seems more clear).

I did not realize "value" was default.  I think your way is clearer in this case.  Roll on!
Attachment #8884907 - Attachment is obsolete: true
Comment on attachment 8897101 [details]
Bug 1378410 - 2. Generate JNI bindings using Python script;

https://reviewboard.mozilla.org/r/168386/#review173772

This looks good.  Long term, we use Gradle's annotation processing for this rather than `moz.build`; and seeing the size of this patch, I'm sorry I pushed back against the ugly-and-expedient `Makefile.in` change.

::: mobile/android/base/generate_build_config.py:156
(Diff revision 1)
>                                         marker='#')
>      includes.add(os.path.join(buildconfig.topobjdir, 'buildid.h'))
>      return includes
> +
> +def generate_jni_bindings(output_file, ap_jar, *args):
> +    # jar names and flags are separated by '-'

nit: capitalized complete sentences, please.

::: mobile/android/base/generate_build_config.py:156
(Diff revision 1)
>                                         marker='#')
>      includes.add(os.path.join(buildconfig.topobjdir, 'buildid.h'))
>      return includes
> +
> +def generate_jni_bindings(output_file, ap_jar, *args):
> +    # jar names and flags are separated by '-'

Isn't it more usual to have `command flag1 ... flagN -- input1 ... inputM`?  So double dashes and flags first?

You might just want to use `argparse` straight-up, although it's your call.

::: mobile/android/base/generate_build_config.py:173
(Diff revision 1)
> +        prefix,
> +    ) + target_jars)
> +
> +    # Preprocess bindings and compare new version against existing version.
> +    DEFINES = _defines()
> +    DEFINES['MOZ_PREPROCESSOR'] = 1

I don't see this in the tree.  Perhaps its use will become clear, but it probably warrants a comment.

::: mobile/android/base/generate_build_config.py:192
(Diff revision 1)
> +    src_files = [os.path.join(srcpath, f) for f in new_files]
> +
> +    if not all(os.path.exists(f) for f in new_files):
> +        raise Exception('Not all files generated')
> +
> +    try:

Is there an advantage to doing these as a set, as opposed to pushing the loop out and doing them one at a time?  It seems like you do a bunch of set operations and sacrifice clear error messaging and straightforward exceptions, and I don't see what you gain from the set handling.

::: mobile/android/base/moz.build:1828
(Diff revision 1)
>          'video/VideoRendererEventListener.java',
>      ]]
> +
> +# Generate separate JNI bindings for GeckoView and for Fennec.
> +def generate_jni_bindings(generated_files, srcpath, prefix, update_cmd, jars):
> +    set = {0}.__class__

This is because `set` isn't exposed to the sandbox, correct?  We should just expose it around http://searchfox.org/mozilla-central/source/python/mozbuild/mozbuild/frontend/sandbox.py#112, although this is resourceful :)

::: mobile/android/base/moz.build:1834
(Diff revision 1)
> +
> +    ap_jar = '!/build/annotationProcessors/annotationProcessors.jar'
> +    target_jars = ['!' + jar.name + '.jar' for jar in jars]
> +
> +    # classpath includes the target jars' dependencies.
> +    classpath = ':'.join(set().union(*(jar.extra_jars for jar in jars)))

Hey!  My "rich objects" that are more than strings paid off for once.

::: mobile/android/base/moz.build:1836
(Diff revision 1)
> +    target_jars = ['!' + jar.name + '.jar' for jar in jars]
> +
> +    # classpath includes the target jars' dependencies.
> +    classpath = ':'.join(set().union(*(jar.extra_jars for jar in jars)))
> +
> +    # Use a dummy file as the "output" file because our model for generating

I think it would be nicer to make the annotation processor write to a temp file or stdout and have the `GENERATED_FILES` Python script handle that, but this isn't worth the time.
Attachment #8897101 - Flags: review?(nalexander) → review+
Comment on attachment 8897103 [details]
Bug 1378410 - 4. Remove obsolete JNI binding Makefile code;

https://reviewboard.mozilla.org/r/168390/#review173776

::: commit-message-686f0:5
(Diff revision 1)
> +Bug 1378410 - 4. Remove obsolete JNI binding Makefile code; r?nalexander
> +
> +Remove the obsolete JNI binding code from Makefile.in. Only the binding
> +file names in the GARBAGE list are kept, because otherwise the build
> +system is unaware of these files.

The build system is definitely aware of these files, from the `moz.build` integration.  Can you explain what you mean here?

Also, I expect this to fail in `--with-gradle` builds, since you'll try to generate the JNI files even when `$TOPOBJDIR/mobile/android/base/**/*.jar` don't exist.  (The code you deleted is guarded by `#ifndef MOZ_BUILD_MOBILE_ANDROID_WITH_GRADLE`.)

Did you handle this in the earlier part and I missed it?  A try build with `android-api-15-gradle` would probably expose this.

r- just so that I check back on this.
Attachment #8897103 - Flags: review?(nalexander) → review-
jchen: looking at the patch sequence and the eventual form of this code, I think my request to express this in Python (https://bugzilla.mozilla.org/show_bug.cgi?id=1378410#c6) was not wise.  Sorry to make you burn cycles pursuing this.  I can only say that the added code seems not that much larger than the removed code; and the Python is certainly more readable.
(In reply to Nick Alexander :nalexander [parental leave until September 15th] from comment #16)
> jchen: looking at the patch sequence and the eventual form of this code, I
> think my request to express this in Python
> (https://bugzilla.mozilla.org/show_bug.cgi?id=1378410#c6) was not wise. 
> Sorry to make you burn cycles pursuing this.  I can only say that the added
> code seems not that much larger than the removed code; and the Python is
> certainly more readable.

No worries! I actually do like the Python suggestion. For example, we had to maintain a separate classpath list for the annotation processor in Makefile.in, but now it's all derived from the moz.build jar dependencies, which is much cleaner.
Comment on attachment 8897101 [details]
Bug 1378410 - 2. Generate JNI bindings using Python script;

https://reviewboard.mozilla.org/r/168386/#review173772

> Isn't it more usual to have `command flag1 ... flagN -- input1 ... inputM`?  So double dashes and flags first?
> 
> You might just want to use `argparse` straight-up, although it's your call.

recursivemake.py puts the flags at the very end [1], so I think I'll keep it the way it is.

[1] http://searchfox.org/mozilla-central/rev/e5b13e6224dbe3182050cf442608c4cb6a8c5c55/python/mozbuild/mozbuild/backend/recursivemake.py#547

> I don't see this in the tree.  Perhaps its use will become clear, but it probably warrants a comment.

Added a comment. I added `MOZ_PREPROCESSOR` so we can distinguish the two preprocessing passes - one pass for comparison during binding generation and another pass when compiling the generated files. Having `MOZ_PREPROCESSOR` lets us use the same preprocessor token ('#') for both passes, which I thought was more elegant.

> Is there an advantage to doing these as a set, as opposed to pushing the loop out and doing them one at a time?  It seems like you do a bunch of set operations and sacrifice clear error messaging and straightforward exceptions, and I don't see what you gain from the set handling.

It was just for simplicity. My thinking was it didn't really matter which file was causing a failure; we should show the "JNI code has changed" message in all such cases.

> Hey!  My "rich objects" that are more than strings paid off for once.

:)
Comment on attachment 8897103 [details]
Bug 1378410 - 4. Remove obsolete JNI binding Makefile code;

https://reviewboard.mozilla.org/r/168390/#review173776

> The build system is definitely aware of these files, from the `moz.build` integration.  Can you explain what you mean here?
> 
> Also, I expect this to fail in `--with-gradle` builds, since you'll try to generate the JNI files even when `$TOPOBJDIR/mobile/android/base/**/*.jar` don't exist.  (The code you deleted is guarded by `#ifndef MOZ_BUILD_MOBILE_ANDROID_WITH_GRADLE`.)
> 
> Did you handle this in the earlier part and I missed it?  A try build with `android-api-15-gradle` would probably expose this.
> 
> r- just so that I check back on this.

Heh so my try run passed under --with-gradle, but only because the generate-bindings step actually caused all the jar files to be built -- outside of gradle. Good catch! I added a guard for MOZ_BUILD_MOBILE_ANDROID_WITH_GRADLE.

I think the build system only knows about our "dummy file" given to GENERATED_FILES (i.e. "Generated.bindings"). It doesn't otherwise know about the annotation processor output files (i.e. "GeneratedJNIWrappers.cpp", "GeneratedJNIWrappers.h", etc.)
Comment on attachment 8897101 [details]
Bug 1378410 - 2. Generate JNI bindings using Python script;

https://reviewboard.mozilla.org/r/168386/#review174616

::: mobile/android/base/moz.build:1849
(Diff revisions 1 - 2)
>      generated_files[output].inputs = [ap_jar] + target_jars
> -    # inputs and flags are passed to the script together, so use '-' as a separator.
> +    # Inputs and flags are passed to the script together, so use '-' as a separator.
>      generated_files[output].flags = ['-', classpath, srcpath, prefix, update_cmd]
>      generated_files[output].tier = 'libs'
>  
> +# Don't perform binding generation when building under gradle because

Include a reference to https://bugzilla.mozilla.org/show_bug.cgi?id=1384312 please.

::: build/docs/mozbuild-files.rst:37
(Diff revision 2)
>     convention of symbols.
>  3. Some symbols are inherited from previously-executed ``moz.build``
>     files.
>  
>  The limited subset of Python is actually an extremely limited subset.
> -Only a few symbols from ``__builtins__`` are exposed. These include
> +Only a few symbols from ``__builtin__`` are exposed. These include

Make this a Pre: patch and get gps to stamp this.  I think it's fine and we should do it, but there may be some dragon here that I don't understand.
Comment on attachment 8897103 [details]
Bug 1378410 - 4. Remove obsolete JNI binding Makefile code;

https://reviewboard.mozilla.org/r/168390/#review174618

This looks fine.  Might be good to conditionalize on artifact builds as well, since it doesn't make sense to change generated bindings in an artifact build, but that can follow.

Thanks!
Attachment #8897103 - Flags: review?(nalexander) → review+
Comment on attachment 8897099 [details]
Bug 1378410 - 2. Preprocess generated bindings;

https://reviewboard.mozilla.org/r/168382/#review175126

I don't think we should add more ways to rely on tiers. Tiers are heavily biased towards the current make build system, and we're moving away from that. Which also means that anything that encodes some kind of order based on tiers makes moving away from the current make build system harder.
Comment on attachment 8897101 [details]
Bug 1378410 - 2. Generate JNI bindings using Python script;

https://reviewboard.mozilla.org/r/168386/#review175128

::: mobile/android/base/moz.build:1834
(Diff revision 3)
> +def generate_jni_bindings(generated_files, srcpath, prefix, update_cmd, jars):
> +    ap_jar = '!/build/annotationProcessors/annotationProcessors.jar'
> +    target_jars = ['!' + jar.name + '.jar' for jar in jars]
> +
> +    # classpath includes the target jars' dependencies.
> +    classpath = ':'.join(set().union(*(jar.extra_jars for jar in jars)))

That yields a non-deterministic class path.
(In reply to Mike Hommey [:glandium] from comment #33)
> Comment on attachment 8897099 [details]
> Bug 1378410 - 0. Add 'tier' attribute for generated files;
> 
> https://reviewboard.mozilla.org/r/168382/#review175126
> 
> I don't think we should add more ways to rely on tiers. Tiers are heavily
> biased towards the current make build system, and we're moving away from
> that. Which also means that anything that encodes some kind of order based
> on tiers makes moving away from the current make build system harder.

I think we're limited to using tiers here because of Android build dependency issues beyond the scope of this bug. Is there a better way to delay the generation step to what would effectively be the libs tier?
Why isn't misc good enough for you?
AFAICT, the files we are generating here depend on the Fennec Java sources being built, and building Fennec Java sources futher depends on a bunch of magic related to building locales and branding, which somehow depends on jar.mn manifests being processed. As a result, putting the generation step in misc causes some kind of dependency issue, where we can't find an output from jar.mn processing when building Java sources.
(In reply to Jim Chen [:jchen] [:darchons] from comment #37)
> AFAICT, the files we are generating here depend on the Fennec Java sources
> being built, and building Fennec Java sources futher depends on a bunch of
> magic related to building locales and branding, which somehow depends on
> jar.mn manifests being processed. As a result, putting the generation step
> in misc causes some kind of dependency issue, where we can't find an output
> from jar.mn processing when building Java sources.

Is it one particular jar.mn or all of them?
Just the branding ones under mobile/android/branding/*/locales/jar.mn, e.g. mobile/android/branding/unofficial/locales/jar.mn
Add the following at the end of config/recurse.mk, which documents all the known inter-dependencies:

mobile/android/base/misc: $(MOZ_BRANDING_DIRECTORY)/locales/misc

and that should work with the misc tier.
Comment on attachment 8898538 [details]
Bug 1378410 - 2a. Expose `set` to moz.build sandbox;

https://reviewboard.mozilla.org/r/169896/#review175354

Yeah, not being able to construct an empty set using set literal syntax is a Python wart.
Attachment #8898538 - Flags: review?(gps) → review+
(In reply to Mike Hommey [:glandium] from comment #40)
> Add the following at the end of config/recurse.mk, which documents all the
> known inter-dependencies:
> 
> mobile/android/base/misc: $(MOZ_BRANDING_DIRECTORY)/locales/misc
> 
> and that should work with the misc tier.

I have tried multiple variations on this and never been able to make it work.  There's a very subtle interaction between m/a/base/locales and m/a/base that I doubt this will capture.  But I would love to be proved wrong!
Yeah that gave an error,

> *** No rule to make target 'mobile/android/branding/unofficial/locales/misc', needed by 'mobile/android/base/misc'.  Stop.
Comment on attachment 8897099 [details]
Bug 1378410 - 2. Preprocess generated bindings;

https://reviewboard.mozilla.org/r/168382/#review175552

I agree with glandium in #c33. Hopefully #c40 works - if not, we can try to work something else out.
Attachment #8897099 - Flags: review?(mshal) → review-
Yeah the suggestion in comment 40 gave an error (comment 43), so we need something else. Maybe a special filename suffix to indicate the generated file needs libs tier?
The reason comment 40 doesn't work is because jar.mn is actually processed in libs, not misc... (essentially for historical and l10n reasons). What this also means is that the fact it worked for you by putting your stuff in the libs tier is accidental, it could very well not have worked either, depending whether the jar.mn is processed before or after recursing your directory, so either way, some kind of cross-directory dependency is still needed.

Moving jar.mn processing to misc may or may not be easy. It's a two lines change to do it technically, but then there's figuring out everything that needs to further change to not break l10n repacks.

What do you need specifically from the branding locale?
mobile/android/base/locales needs `brand.dtd` [1], which is copied to `$(topobjdir)/dist/bin/chrome/$(AB_CD)/locale/branding/brand.dtd` during jar.mn processing. In local builds, `brand.dtd` comes from `$(MOZ_BRANDING_DIRECTORY)/locales/misc`, but I suppose when building for l10n it can come from somewhere else. Not sure how that is done.

[1] http://searchfox.org/mozilla-central/rev/48ea452803907f2575d81021e8678634e8067fc2/mobile/android/base/locales/Makefile.in#19
(In reply to Nick Alexander :nalexander [parental leave until September 15th] from comment #16)
> jchen: looking at the patch sequence and the eventual form of this code, I
> think my request to express this in Python
> (https://bugzilla.mozilla.org/show_bug.cgi?id=1378410#c6) was not wise. 
> Sorry to make you burn cycles pursuing this.  I can only say that the added
> code seems not that much larger than the removed code; and the Python is
> certainly more readable.

jchen, others: these thickets of l10n and subtle brand processing are not worth hacking through.  Jim, can you revisit my initial comments and make the changes necessary to land this with your proposed Makefile.in?  Then we can get the functionality in, file follow-up to improve the situation (which might just be "do it with Gradle annotation processors"), and move on with our lives.

I would like to see an example of the two layers of preprocessing.  I know you defined a variable, but I think I would prefer a marker for each processing stage, like:

%ifdef MOZ_NIGHTLY
...
#define THING_THAT_DEPENDS_ON_MOZ_NIGHTLY
%endif

But I may be missing something, so an example would help.
Flags: needinfo?(nchen)
Because the same files will go through both the build system preprocessor and the C preprocessor, we need to make sure the preprocessing lines are valid in both.

In my original patch, I used "//#" as the preprocessing marker, which appears as a valid marker to build system, and appears as a comment to C. Using "%" would work for the build system, but would appear as an invalid expression to C.

Using "#" works for both build system and C, but the "#include" lines in the files are meant for C only, not for build system; so I added "MOZ_PREPROCESSOR" to make build system ignore the "#include" lines. Despite that, I thought it was more elegant to use the same "#" marker for both preprocessors.
Flags: needinfo?(nchen)
Attachment #8897101 - Attachment is obsolete: true
Attachment #8897103 - Attachment is obsolete: true
Comment on attachment 8897099 [details]
Bug 1378410 - 2. Preprocess generated bindings;

https://reviewboard.mozilla.org/r/168382/#review180570

Let's get something landed here and iterate.  Sorry for the run around, Jim.

::: mobile/android/base/Makefile.in:543
(Diff revision 4)
>  #   FennecJNIWrappers.h and FennecJNINatives.h
>  ifndef MOZ_BUILD_MOBILE_ANDROID_WITH_GRADLE
> +BINDING_BUILD_FLAGS = \
> +  $(NULL)
> +
> +preprocess-binding = ($(call py_action,preprocessor, \

nit: include argument comments, please (see other examples in this file).

::: mobile/android/base/Makefile.in:545
(Diff revision 4)
> +BINDING_BUILD_FLAGS = \
> +  $(NULL)
> +
> +preprocess-binding = ($(call py_action,preprocessor, \
> +                      $(foreach flag,$(BINDING_BUILD_FLAGS),$(if $($(flag)),-D$(flag))) \
> +                      -DMOZ_PREPROCESSOR $(1)) || echo $(1))

OK.  I'd still prefer different markers, like:

//%ifdef MOZ_PREPROCESSOR
//#define INNER
//%endif

but it's your rodeo.
Attachment #8897099 - Flags: review?(nalexander) → review+
Comment on attachment 8897102 [details]
Bug 1378410 - 3. Update generated bindings;

https://reviewboard.mozilla.org/r/168388/#review180584
Attachment #8897102 - Flags: review+
Comment on attachment 8897100 [details]
Bug 1378410 - 1. Support BuildFlag annotation for generated bindings;

https://reviewboard.mozilla.org/r/168384/#review180588
Attachment #8897100 - Flags: review+
Comment on attachment 8897100 [details]
Bug 1378410 - 1. Support BuildFlag annotation for generated bindings;

Already reviewed
Attachment #8897100 - Flags: review?(snorp) → review+
Comment on attachment 8897100 [details]
Bug 1378410 - 1. Support BuildFlag annotation for generated bindings;

https://reviewboard.mozilla.org/r/168384/#review180592
Attachment #8897100 - Flags: review+
Pushed by nchen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1bf78591e454
1. Support BuildFlag annotation for generated bindings; r=snorp
https://hg.mozilla.org/integration/autoland/rev/704e761343e0
2a. Expose `set` to moz.build sandbox; r=gps
https://hg.mozilla.org/integration/autoland/rev/8e135c63527f
2. Preprocess generated bindings; r=nalexander
https://hg.mozilla.org/integration/autoland/rev/2244d8d4cdfb
3. Update generated bindings; r=jchen
Product: Firefox for Android → Firefox Build System
Target Milestone: Firefox 57 → mozilla57
You need to log in before you can comment on or make changes to this bug.