1257958 - Move android "feature flags" to Python configure

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Chris Manchester (limited bugmail, email directly)]

There are several, and I'm going to keep nalexander in the loop on these, so splitting from bug 1257326.

Comment 1

[Chris Manchester (limited bugmail, email directly)]

I found 6 variables along the way that are only checked in preprocessor directives, and are therefore not necessary to include in substs, so they are not included there as a result of this series. They are:

MOZ_ANDROID_APZ
MOZ_ANDROID_BEAM
MOZ_ANDROID_DOWNLOADS_INTEGRATION
MOZ_ANDROID_DOWNLOAD_CONTENT_SERVICE
MOZ_LOCALE_SWITCHER
MOZ_SWITCHBOARD

Comment 2

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: Bug 1257958 - Move MOZ_ANDROID_APZ to Python configure.

And do not port the subst, because its value is not used.

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

Comment 3

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: Bug 1257958 - Add a template to construct an environment-set option that sets the corresponding variable. r=glandium

Fennec has several things that fall into this category, this will reduce duplication
significantly.

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

Comment 4

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: Bug 1257958 - Move MOZ_LOCALE_SWITCHER to Python configure. r=nalexander

And do not port the subst, because its value is not used.

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

Comment 5

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: Bug 1257958 - Move MOZ_ANDROID_DOWNLOADS_INTEGRATION to Python configure.

And do not port the substs, its value is not used.

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

Comment 6

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: Bug 1257958 - Move MOZ_ANDROID_BEAM to Python configure.

And do not port the subst, its value is not used.

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

Comment 7

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: Bug 1257958 - Move MOZ_ANDROID_SEARCH_ACTIVITY to Python configure.

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

Comment 8

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: Bug 1257958 - Move MOZ_ANDROID_MLS_STUMBLER to Python configure.

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

Comment 9

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: Bug 1257958 - Move MOZ_ANDROID_EXCLUDE_FONTS to Python configure. r=nalexander

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

Comment 10

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: Bug 1257958 - Move MOZ_ANDROID_DOWNLOAD_CONTENT_SERVICE to moz.build.

And do not port the subst, its value is not used.

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

Comment 11

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: Bug 1257958 - Move MOZ_SWITCHBOARD to Python configure.

This does not port the subst, because its value is not used,
and removes MOZ_SWITCHBOARD from AppConstants.jsm, because its
value is not used.

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

Comment 12

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: Bug 1257958 - Move MOZ_ANDROID_GCM to Python configure.

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

Comment 13

[Chris Manchester (limited bugmail, email directly)]

Nick, can you glance over this patch series and confirm I've done something understandable and reasonable to work with for these flags? Thank you.

Comment 14

[Nick Alexander :nalexander [he/him]]

(In reply to Chris Manchester (:chmanchester) from comment #13)
> Nick, can you glance over this patch series and confirm I've done something
> understandable and reasonable to work with for these flags? Thank you.

Chris and I talked over Video.  From a scratch etherpad:  """My desired API looks like:

feature('MOZ_ANDROID_FOO', conditions.is_nightly || !conditions.is_release)

I don't care if we ever declare --enable-android-foo, we can do this all with mozconfig env var declarations if we choose to.

I want to subst everywhere, globally define nowhere (since changing global defines recompiles C code and is expensive), and locally add to DEFINES."""

Chris told me there was a temporary restriction that templates can't set_config, and that the restriction would likely be addressed within a working week.  We agreed to revisit this when we can get a shorter form for each feature flag definition, surfacing the milestone flags more explicitly.

Comment 15

[Chris Manchester (limited bugmail, email directly)]

For reference, bug 1257823 is the proposal that looks promising, and would allow set_config to happen from a template. I'll cancel review here based on our discussion and comment 14.

Comment 16

[Mike Hommey [:glandium]]

(In reply to Nick Alexander :nalexander from comment #14)
> (In reply to Chris Manchester (:chmanchester) from comment #13)
> > Nick, can you glance over this patch series and confirm I've done something
> > understandable and reasonable to work with for these flags? Thank you.
> 
> Chris and I talked over Video.  From a scratch etherpad:  """My desired API
> looks like:
> 
> feature('MOZ_ANDROID_FOO', conditions.is_nightly || !conditions.is_release)

Note this form cannot work.

What could work is something like:

feature('MOZ_ANDROID_FOO', milestone, lambda m: m.is_nightly || !m.is_release)

With some hack it /might/ be possible to do:

feature('MOZ_ANDROID_FOO', lambda milestone: milestone.is_nightly || !milestone.is_release)

But definitely not without a function being involved.

Comment 17

[Nick Alexander :nalexander [he/him]]

(In reply to Mike Hommey [:glandium] from comment #16)
> (In reply to Nick Alexander :nalexander from comment #14)
> > (In reply to Chris Manchester (:chmanchester) from comment #13)
> > > Nick, can you glance over this patch series and confirm I've done something
> > > understandable and reasonable to work with for these flags? Thank you.
> > 
> > Chris and I talked over Video.  From a scratch etherpad:  """My desired API
> > looks like:
> > 
> > feature('MOZ_ANDROID_FOO', conditions.is_nightly || !conditions.is_release)
> 
> Note this form cannot work.
> 
> What could work is something like:
> 
> feature('MOZ_ANDROID_FOO', milestone, lambda m: m.is_nightly ||
> !m.is_release)
> 
> With some hack it /might/ be possible to do:
> 
> feature('MOZ_ANDROID_FOO', lambda milestone: milestone.is_nightly ||
> !milestone.is_release)
> 
> But definitely not without a function being involved.

One can do things with conditions being a custom type that collects the expression tree before evaluation, but I agree, lambda is simpler.

Comment 18

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://reviewboard.mozilla.org/r/41141/#review37831

::: mobile/android/moz.configure:11
(Diff revision 1)
>  
> +option(env='MOZ_ANDROID_GCM',
> +       help='Enable GCM on Android.')
> +
> +@depends('MOZ_ANDROID_GCM', milestone)
> +def android_gsm(value, milestone):

gsm?

Comment 19

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8732353 [details]
MozReview Request: Bug 1257958 - Add a template to construct an environment-set option that sets the corresponding variable. r=glandium

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

Comment 20

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8732359 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_EXCLUDE_FONTS to Python configure. r=nalexander

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

Comment 21

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8732354 [details]
MozReview Request: Bug 1257958 - Move MOZ_LOCALE_SWITCHER to Python configure. r=nalexander

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

Comment 22

[Chris Manchester (limited bugmail, email directly)]

Nick, I have a new approach based on our discussion in the three commits starting with comment 19, can you sign off on this before we proceed? Thanks.

Comment 23

[Nick Alexander :nalexander [he/him]]

(In reply to Chris Manchester (:chmanchester) from comment #22)
> Nick, I have a new approach based on our discussion in the three commits
> starting with comment 19, can you sign off on this before we proceed? Thanks.

Yeah, this looks much better.  My only request is that we use more descriptive names, like:

feature_enabled_nowhere(...)
feature_enabled_everywhere(...)
feature_enabled_nightly_only(...)
feature_enabled_after_beta(...)

Otherwise, this looks good.  BTW, no need to break this into a bunch of small patches if it's work to do so.  A pre or two and then the meat works for me.

Comment 24

[Mike Hommey [:glandium]]

(In reply to Nick Alexander :nalexander from comment #23)
> (In reply to Chris Manchester (:chmanchester) from comment #22)
> > Nick, I have a new approach based on our discussion in the three commits
> > starting with comment 19, can you sign off on this before we proceed? Thanks.
> 
> Yeah, this looks much better.  My only request is that we use more
> descriptive names, like:
> 
> feature_enabled_nowhere(...)
> feature_enabled_everywhere(...)
> feature_enabled_nightly_only(...)
> feature_enabled_after_beta(...)

seems like it would be better to have predefined callbacks for the condition, and just call
flag('FOO', 'This is foo', nightly_only).

flag is not very descriptive, though.

Comment 25

[Nick Alexander :nalexander [he/him]]

(In reply to Mike Hommey [:glandium] from comment #24)
> (In reply to Nick Alexander :nalexander from comment #23)
> > (In reply to Chris Manchester (:chmanchester) from comment #22)
> > > Nick, I have a new approach based on our discussion in the three commits
> > > starting with comment 19, can you sign off on this before we proceed? Thanks.
> > 
> > Yeah, this looks much better.  My only request is that we use more
> > descriptive names, like:
> > 
> > feature_enabled_nowhere(...)
> > feature_enabled_everywhere(...)
> > feature_enabled_nightly_only(...)
> > feature_enabled_after_beta(...)
> 
> seems like it would be better to have predefined callbacks for the
> condition, and just call
> flag('FOO', 'This is foo', nightly_only).
> 
> flag is not very descriptive, though.

Fine by me.  I was trying to avoid things that *should* look like "nightly | aurora", given earlier discussion.

Comment 26

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8732353 [details]
MozReview Request: Bug 1257958 - Add a template to construct an environment-set option that sets the corresponding variable. r=glandium

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

Comment 27

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8732359 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_EXCLUDE_FONTS to Python configure. r=nalexander

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

Comment 28

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8732354 [details]
MozReview Request: Bug 1257958 - Move MOZ_LOCALE_SWITCHER to Python configure. r=nalexander

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

Comment 29

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8732359 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_EXCLUDE_FONTS to Python configure. r=nalexander

https://reviewboard.mozilla.org/r/41135/#review38029

Very nice.

::: mobile/android/moz.configure:8
(Diff revision 3)
>  # This Source Code Form is subject to the terms of the Mozilla Public
>  # License, v. 2.0. If a copy of the MPL was not distributed with this
>  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
>  
> +conditional_feature('MOZ_ANDROID_EXCLUDE_FONTS',
> +                    'Whether to exclude font files in the build',

nit: ... font files *from* the build.

Comment 30

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8732353 [details]
MozReview Request: Bug 1257958 - Add a template to construct an environment-set option that sets the corresponding variable. r=glandium

https://reviewboard.mozilla.org/r/41123/#review38031

wfm but glandium should have the final say.

::: build/moz.configure/init.configure:620
(Diff revision 3)
> +@template
> +def enabled_everywhere(_):
> +    return True
> +
> +@template
> +def enabled_in_nightly(milestone):

Let's define the major ones, and comment on how they map to the existing variables.

Comment 31

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8732354 [details]
MozReview Request: Bug 1257958 - Move MOZ_LOCALE_SWITCHER to Python configure. r=nalexander

https://reviewboard.mozilla.org/r/41125/#review38033

Looks good.

::: mobile/android/moz.configure:12
(Diff revision 3)
>  conditional_feature('MOZ_ANDROID_EXCLUDE_FONTS',
>                      'Whether to exclude font files in the build',
>                      enabled_in_nightly)
>  
> +conditional_feature('MOZ_LOCALE_SWITCHER',
> +                    'Enable runtime locale switching.',

nit: either drop this period, or add one above.  They should be consistent, and they should reflect whatever is the --help convention.

Comment 32

[Mike Hommey [:glandium]]

Comment on attachment 8732353 [details]
MozReview Request: Bug 1257958 - Add a template to construct an environment-set option that sets the corresponding variable. r=glandium

https://reviewboard.mozilla.org/r/41123/#review38411

::: build/moz.configure/init.configure:601
(Diff revision 3)
>                       is_nightly=is_nightly,
>                       is_release=is_release)
>  
> +# A template to add a "flag" (environment variable) whose default
> +# will be determined by the given condition. A condition is a callable
> +# that will receive the value of the current milestone as its only

Being limited to a milestone test seems fairly limited.

Instead, you could define enabled_in_nightly as:

@depends(milestone)
def enabled_in_nightly(milestone):
    return milestone.is_nightly

and @depends(condition) in the conditional_feature template.

That being said, set_config changes in bug 1257823  too (I'm waiting for try results to push it to mozreview)...

Comment 33

[Chris Manchester (limited bugmail, email directly)]

https://reviewboard.mozilla.org/r/41123/#review38411

> Being limited to a milestone test seems fairly limited.
> 
> Instead, you could define enabled_in_nightly as:
> 
> @depends(milestone)
> def enabled_in_nightly(milestone):
>     return milestone.is_nightly
> 
> and @depends(condition) in the conditional_feature template.
> 
> That being said, set_config changes in bug 1257823  too (I'm waiting for try results to push it to mozreview)...

Bug 1257823 is going to help this a lot, I'll re-work this once that lands.

Comment 34

[Mike Hommey [:glandium]]

(In reply to Mike Hommey [:glandium] from comment #32)
> @depends(milestone)
> def enabled_in_nightly(milestone):
>     return milestone.is_nightly

BTW, one-offs can also be done in one-liners:

template_name(..., depends(milestone)(lambda m: m.is_nightly))

Comment 35

[Chris Manchester (limited bugmail, email directly)]

(In reply to Chris Manchester (:chmanchester) from comment #33)
> https://reviewboard.mozilla.org/r/41123/#review38411
> 
> > Being limited to a milestone test seems fairly limited.
> > 
> > Instead, you could define enabled_in_nightly as:
> > 
> > @depends(milestone)
> > def enabled_in_nightly(milestone):
> >     return milestone.is_nightly
> > 
> > and @depends(condition) in the conditional_feature template.
> > 
> > That being said, set_config changes in bug 1257823  too (I'm waiting for try results to push it to mozreview)...
> 
> Bug 1257823 is going to help this a lot, I'll re-work this once that lands.

Actually I don't see a lot to change other than to address this issue. I was thinking I could just set_config('MOZ_ANDROID...', delayed_getattr( milestone, 'is_nightly')) and not even define an option for these, but that would give up the ability to set things with env variables.

Comment 36

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8732353 [details]
MozReview Request: Bug 1257958 - Add a template to construct an environment-set option that sets the corresponding variable. r=glandium

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/41123/diff/3-4/

Comment 37

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8732359 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_EXCLUDE_FONTS to Python configure. r=nalexander

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/41135/diff/3-4/

Comment 38

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8732354 [details]
MozReview Request: Bug 1257958 - Move MOZ_LOCALE_SWITCHER to Python configure. r=nalexander

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/41125/diff/3-4/

Comment 39

[Mike Hommey [:glandium]]

Comment on attachment 8732353 [details]
MozReview Request: Bug 1257958 - Add a template to construct an environment-set option that sets the corresponding variable. r=glandium

Unflagging myself, as the patch is outdated.

Comment 40

[Chris Manchester (limited bugmail, email directly)]

(In reply to Mike Hommey [:glandium] from comment #39)
> Comment on attachment 8732353 [details]
> MozReview Request: Bug 1257958 - Add a template for feature flags that
> depend on the current milestone. r=glandium
> 
> Unflagging myself, as the patch is outdated.

Which part is outdated? I've been working with this on top of central, it applies cleanly and works as expected. The subsequent patches are out of date with respect to how they get defines to package-manifest.in, I'm updating that now.

Comment 41

[Chris Manchester (limited bugmail, email directly)]

I see, the patches that just hit inbound outdate this.

Comment 42

[Mike Hommey [:glandium]]

Bug 1257823 did, but bug 1260066 that just landed enforces it.

Comment 43

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: Bug 1257958 - Move MOZ_ANDROID_GCM to Python configure. r=nalexander

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

Comment 44

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: Bug 1257958 - Move MOZ_ANDROID_DOWNLOADS_INTEGRATION to Python configure. r=nalexander

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

Comment 45

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: Bug 1257958 - Move MOZ_ANDROID_BEAM to Python configure. r=nalexander

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

Comment 46

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: Bug 1257958 - Move MOZ_ANDROID_SEARCH_ACTIVITY to Python configure. r=nalexander

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

Comment 47

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: Bug 1257958 - Move MOZ_ANDROID_MLS_STUMBLER to Python configure. r=nalexander

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

Comment 48

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: Bug 1257958 - Move MOZ_ANDROID_DOWNLOAD_CONTENT_SERVICE to Python configure. r=nalexander

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

Comment 49

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: Bug 1257958 - Move MOZ_SWITCHBOARD to Python configure. r=nalexander

This removes MOZ_SWITCHBOARD from AppConstants.jsm, because its
value is not used.

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

Comment 50

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: Bug 1257958 - Move MOZ_ANDROID_APZ to Python configure. r=nalexander

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

Comment 51

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8732353 [details]
MozReview Request: Bug 1257958 - Add a template to construct an environment-set option that sets the corresponding variable. r=glandium

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/41123/diff/4-5/

Comment 52

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8732359 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_EXCLUDE_FONTS to Python configure. r=nalexander

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/41135/diff/4-5/

Comment 53

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8732354 [details]
MozReview Request: Bug 1257958 - Move MOZ_LOCALE_SWITCHER to Python configure. r=nalexander

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/41125/diff/4-5/

Comment 54

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8732353 [details]
MozReview Request: Bug 1257958 - Add a template to construct an environment-set option that sets the corresponding variable. r=glandium

https://reviewboard.mozilla.org/r/41123/#review40427

::: build/moz.configure/init.configure:689
(Diff revision 5)
>  set_config('RELEASE_BUILD', delayed_getattr(milestone, 'is_release'))
>  set_define('RELEASE_BUILD', delayed_getattr(milestone, 'is_release'))
>  add_old_configure_assignment('RELEASE_BUILD',
>                               delayed_getattr(milestone, 'is_release'))
>  
> +# A template to add a "flag" (environment variable) whose default

I think it's very important to highlight that this is `set_config` only.  It might be worth adding a note on how to use it in `moz.build`, and how to make it a define (since that's often wanted).  Even just a reference to another file will help.

Comment 55

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8736466 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_GCM to Python configure. r=nalexander

https://reviewboard.mozilla.org/r/43251/#review40429

::: mobile/android/installer/Makefile.in:44
(Diff revision 1)
>  
>  ifdef MOZ_ANDROID_EXCLUDE_FONTS
>  DEFINES += -DMOZ_ANDROID_EXCLUDE_FONTS=1
>  endif
>  
> +ifdef MOZ_ANDROID_GCM

Wish there was a way to make this happen automatically.

::: mobile/android/installer/Makefile.in:44
(Diff revision 1)
>  
>  ifdef MOZ_ANDROID_EXCLUDE_FONTS
>  DEFINES += -DMOZ_ANDROID_EXCLUDE_FONTS=1
>  endif
>  
> +ifdef MOZ_ANDROID_GCM

Wish there was a way to make this happen automatically.

Comment 56

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8736467 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_DOWNLOADS_INTEGRATION to Python configure. r=nalexander

https://reviewboard.mozilla.org/r/43253/#review40431

Comment 57

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8736468 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_BEAM to Python configure. r=nalexander

https://reviewboard.mozilla.org/r/43255/#review40433

Comment 58

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8736469 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_SEARCH_ACTIVITY to Python configure. r=nalexander

https://reviewboard.mozilla.org/r/43257/#review40435

Comment 59

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8736470 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_MLS_STUMBLER to Python configure. r=nalexander

https://reviewboard.mozilla.org/r/43259/#review40437

Comment 60

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8736471 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_DOWNLOAD_CONTENT_SERVICE to Python configure. r=nalexander

https://reviewboard.mozilla.org/r/43261/#review40439

Comment 61

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8736472 [details]
MozReview Request: Bug 1257958 - Move MOZ_SWITCHBOARD to Python configure. r=nalexander

https://reviewboard.mozilla.org/r/43263/#review40441

::: toolkit/modules/AppConstants.jsm
(Diff revision 1)
>    true,
>  #else
>    false,
>  #endif
>  
> -  MOZ_SWITCHBOARD:

I'm not against removing this, but long term I want `AppConstants.jsm` and `AppConstants.java` to be generated automatically (and include everything), so I'm not seeing the win.

Your call.

Comment 62

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8736473 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_APZ to Python configure. r=nalexander

https://reviewboard.mozilla.org/r/43265/#review40443

Comment 63

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8732359 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_EXCLUDE_FONTS to Python configure. r=nalexander

https://reviewboard.mozilla.org/r/41135/#review40445

Comment 64

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8732354 [details]
MozReview Request: Bug 1257958 - Move MOZ_LOCALE_SWITCHER to Python configure. r=nalexander

https://reviewboard.mozilla.org/r/41125/#review40447

Comment 65

[Nick Alexander :nalexander [he/him]]

chmanchester -- nice series!  Sorry to make you wait on my reviews.

Comment 66

[Nick Alexander :nalexander [he/him]]

Let's plan to email dev-builds and mobile-firefox-dev.  I can do this if you prefer.

Comment 67

[Chris Manchester (limited bugmail, email directly)]

https://reviewboard.mozilla.org/r/43251/#review40429

> Wish there was a way to make this happen automatically.

Right :/ According to glandium this is really the way to do this for now.

Comment 68

[Chris Manchester (limited bugmail, email directly)]

https://reviewboard.mozilla.org/r/43263/#review40441

> I'm not against removing this, but long term I want `AppConstants.jsm` and `AppConstants.java` to be generated automatically (and include everything), so I'm not seeing the win.
> 
> Your call.

Ok, makes sense. If this is remotely likely to be checked from js in the future I'm creating a footgun by removing this, so I'll leave it for now.

Comment 69

[Mike Hommey [:glandium]]

Comment on attachment 8732353 [details]
MozReview Request: Bug 1257958 - Add a template to construct an environment-set option that sets the corresponding variable. r=glandium

https://reviewboard.mozilla.org/r/41123/#review40535

::: build/moz.configure/init.configure:694
(Diff revision 5)
> +def conditional_feature(name, help, condition):
> +    opt = option(env=name, help=help)
> +
> +    @depends(opt.option, condition)
> +    def flag_implementation(value, condition):
> +        return value or (value.origin == 'default' and
> +                         condition)
> +
> +    set_config(name, flag_implementation)

You'd have more flexibility if you made the template return flag_implementation and let the caller do the set_config. That would make it two lines, but would allow to use set_define or add_old_configure_assignment as well.

The template itself should go in util.configure. The conditions, since they depend on milestone, need to go after milestone, unfortunately.

Also, the template can be used with conditions that have nothing to do with the milestone, so it feels restrictive to only comment about that.

::: build/moz.configure/init.configure:704
(Diff revision 5)
> +        return value or (value.origin == 'default' and
> +                         condition)
> +
> +    set_config(name, flag_implementation)
> +
> +@depends(milestone)

This could just depend on '--help', since it returns True unconditionally.

Comment 70

[Chris Manchester (limited bugmail, email directly)]

https://reviewboard.mozilla.org/r/41123/#review40535

> You'd have more flexibility if you made the template return flag_implementation and let the caller do the set_config. That would make it two lines, but would allow to use set_define or add_old_configure_assignment as well.
> 
> The template itself should go in util.configure. The conditions, since they depend on milestone, need to go after milestone, unfortunately.
> 
> Also, the template can be used with conditions that have nothing to do with the milestone, so it feels restrictive to only comment about that.

The other thing I had in my queue at one point was "set_as_define" and "set_for_old_configure" keyword args to conditional_feature, which is verbose but effective. I'm not really going to take a stand on keeping these down to one line, but maybe Nick can chime in on this.

Comment 71

[Nick Alexander :nalexander [he/him]]

(In reply to Chris Manchester (:chmanchester) from comment #70)
> https://reviewboard.mozilla.org/r/41123/#review40535
> 
> > You'd have more flexibility if you made the template return flag_implementation and let the caller do the set_config. That would make it two lines, but would allow to use set_define or add_old_configure_assignment as well.
> > 
> > The template itself should go in util.configure. The conditions, since they depend on milestone, need to go after milestone, unfortunately.
> > 
> > Also, the template can be used with conditions that have nothing to do with the milestone, so it feels restrictive to only comment about that.
> 
> The other thing I had in my queue at one point was "set_as_define" and
> "set_for_old_configure" keyword args to conditional_feature, which is
> verbose but effective. I'm not really going to take a stand on keeping these
> down to one line, but maybe Nick can chime in on this.

I don't care either way.  I think we should land something simple like this and re-jig it if we need to later.

Comment 72

[Chris Manchester (limited bugmail, email directly)]

(In reply to Nick Alexander :nalexander from comment #71)
> (In reply to Chris Manchester (:chmanchester) from comment #70)
> > https://reviewboard.mozilla.org/r/41123/#review40535
> > 
> > > You'd have more flexibility if you made the template return flag_implementation and let the caller do the set_config. That would make it two lines, but would allow to use set_define or add_old_configure_assignment as well.
> > > 
> > > The template itself should go in util.configure. The conditions, since they depend on milestone, need to go after milestone, unfortunately.
> > > 
> > > Also, the template can be used with conditions that have nothing to do with the milestone, so it feels restrictive to only comment about that.
> > 
> > The other thing I had in my queue at one point was "set_as_define" and
> > "set_for_old_configure" keyword args to conditional_feature, which is
> > verbose but effective. I'm not really going to take a stand on keeping these
> > down to one line, but maybe Nick can chime in on this.
> 
> I don't care either way.  I think we should land something simple like this
> and re-jig it if we need to later.

Ok, then I will implement Mike's suggestion.

Comment 73

[Chris Manchester (limited bugmail, email directly)]

Hm... so I can't:

exclude_fonts = conditional_feature('MOZ_ANDROID_EXCLUDE_FONTS',
                                    'Whether to exclude font files from the build',
                                    enabled_everywhere)

set_config(*exclude_fonts)

because:

 0:00.57 KeyError: u'Cannot assign `exclude_fonts` because it is neither a @depends nor a @template'

Comment 74

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8732353 [details]
MozReview Request: Bug 1257958 - Add a template to construct an environment-set option that sets the corresponding variable. r=glandium

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/41123/diff/5-6/

Comment 75

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8732359 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_EXCLUDE_FONTS to Python configure. r=nalexander

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/41135/diff/5-6/

Comment 76

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8732354 [details]
MozReview Request: Bug 1257958 - Move MOZ_LOCALE_SWITCHER to Python configure. r=nalexander

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/41125/diff/5-6/

Comment 77

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8736466 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_GCM to Python configure. r=nalexander

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

Comment 78

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8736467 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_DOWNLOADS_INTEGRATION to Python configure. r=nalexander

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

Comment 79

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8736468 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_BEAM to Python configure. r=nalexander

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

Comment 80

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8736469 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_SEARCH_ACTIVITY to Python configure. r=nalexander

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

Comment 81

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8736470 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_MLS_STUMBLER to Python configure. r=nalexander

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

Comment 82

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8736471 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_DOWNLOAD_CONTENT_SERVICE to Python configure. r=nalexander

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

Comment 83

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8736472 [details]
MozReview Request: Bug 1257958 - Move MOZ_SWITCHBOARD to Python configure. r=nalexander

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

Comment 84

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8736473 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_APZ to Python configure. r=nalexander

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

Comment 85

[Mike Hommey [:glandium]]

Comment on attachment 8732353 [details]
MozReview Request: Bug 1257958 - Add a template to construct an environment-set option that sets the corresponding variable. r=glandium

https://reviewboard.mozilla.org/r/41123/#review44827

::: build/moz.configure/init.configure:689
(Diff revision 6)
> +# the corresponding variable in set_config, where the value of that variable
> +# passed to set_config only depends on the provided value and default.
> +# If required, the set_as_define and set_for_old_configure arguments
> +# will additionally cause the variable to be set using set_define and
> +# add_old_configure_assignment. util.configure would be an appropriate place for
> +# this, but it uses add_old_configure_assignment, which is defined in this file.

huh? add_old_configure_assignment is declared (and used) in init.configure.

::: build/moz.configure/init.configure:701
(Diff revision 6)
> +        if value:
> +            if len(value):
> +                return value

one of the upsides of NegativeOptionValue being mostly the same type as PositiveOptionValue (a tuple), and NegativeOptionValues always being empty, you can just write:
if len(value):
    return value

::: build/moz.configure/init.configure:714
(Diff revision 6)
> +@depends('--help')
> +def enabled_everywhere(_):
> +    return True

Why not just pass default=True?

::: build/moz.configure/init.configure:718
(Diff revision 6)
> +# milestone.is_nightly corresponds to cases NIGHTLY_BUILD is set.
> +@depends(milestone, '--help')
> +def enabled_in_nightly(milestone, _):
> +    return milestone.is_nightly

why not pass default=delayed_getattr(milestone, 'is_nightly') ? mmmm I can answer this one: that wouldn't work because of the missing required dependency on --help... On the long term, I think delayed_getattr() would be better than many ad-hoc functions, but because of the --help dependency, this will have to wait (I'm planning to lift the requirement for most --help dependencies, stay tuned)

Comment 86

[Mike Hommey [:glandium]]

https://reviewboard.mozilla.org/r/41123/#review44827

> huh? add_old_configure_assignment is declared (and used) in init.configure.

Err, I thought the comment was in old.configure. Forget it.

Comment 87

[Chris Manchester (limited bugmail, email directly)]

https://reviewboard.mozilla.org/r/41123/#review44827

> one of the upsides of NegativeOptionValue being mostly the same type as PositiveOptionValue (a tuple), and NegativeOptionValues always being empty, you can just write:
> if len(value):
>     return value

That didn't quite work, because PositiveOptionValue has a len of 0.

Comment 88

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8732353 [details]
MozReview Request: Bug 1257958 - Add a template to construct an environment-set option that sets the corresponding variable. r=glandium

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/41123/diff/6-7/

Comment 89

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8732359 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_EXCLUDE_FONTS to Python configure. r=nalexander

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/41135/diff/6-7/

Comment 90

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8732354 [details]
MozReview Request: Bug 1257958 - Move MOZ_LOCALE_SWITCHER to Python configure. r=nalexander

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/41125/diff/6-7/

Comment 91

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8736466 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_GCM to Python configure. r=nalexander

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

Comment 92

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8736467 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_DOWNLOADS_INTEGRATION to Python configure. r=nalexander

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

Comment 93

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8736468 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_BEAM to Python configure. r=nalexander

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

Comment 94

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8736469 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_SEARCH_ACTIVITY to Python configure. r=nalexander

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

Comment 95

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8736470 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_MLS_STUMBLER to Python configure. r=nalexander

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

Comment 96

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8736471 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_DOWNLOAD_CONTENT_SERVICE to Python configure. r=nalexander

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

Comment 97

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8736472 [details]
MozReview Request: Bug 1257958 - Move MOZ_SWITCHBOARD to Python configure. r=nalexander

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

Comment 98

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8736473 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_APZ to Python configure. r=nalexander

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

Comment 99

[Mike Hommey [:glandium]]

(In reply to Chris Manchester (:chmanchester) from comment #87)
> https://reviewboard.mozilla.org/r/41123/#review44827
> 
> > one of the upsides of NegativeOptionValue being mostly the same type as PositiveOptionValue (a tuple), and NegativeOptionValues always being empty, you can just write:
> > if len(value):
> >     return value
> 
> That didn't quite work, because PositiveOptionValue has a len of 0.

That's covered by the alternative path, that does return bool(value)

Comment 100

[Chris Manchester (limited bugmail, email directly)]

(In reply to Mike Hommey [:glandium] from comment #99)
> (In reply to Chris Manchester (:chmanchester) from comment #87)
> > https://reviewboard.mozilla.org/r/41123/#review44827
> > 
> > > one of the upsides of NegativeOptionValue being mostly the same type as PositiveOptionValue (a tuple), and NegativeOptionValues always being empty, you can just write:
> > > if len(value):
> > >     return value
> > 
> > That didn't quite work, because PositiveOptionValue has a len of 0.
> 
> That's covered by the alternative path, that does return bool(value)

Ah, I misunderstood the suggestion. But still, for a NegativeOptionValue we need to return None, not False, for set_define.

Comment 101

[Mike Hommey [:glandium]]

Oh, I missed the indentation on the last line.

Comment 102

[Mike Hommey [:glandium]]

Comment on attachment 8732353 [details]
MozReview Request: Bug 1257958 - Add a template to construct an environment-set option that sets the corresponding variable. r=glandium

https://reviewboard.mozilla.org/r/41123/#review45003

Comment 103

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8732353 [details]
MozReview Request: Bug 1257958 - Add a template to construct an environment-set option that sets the corresponding variable. r=glandium

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/41123/diff/7-8/

Comment 104

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8732359 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_EXCLUDE_FONTS to Python configure. r=nalexander

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/41135/diff/7-8/

Comment 105

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8732354 [details]
MozReview Request: Bug 1257958 - Move MOZ_LOCALE_SWITCHER to Python configure. r=nalexander

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/41125/diff/7-8/

Comment 106

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8736466 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_GCM to Python configure. r=nalexander

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43251/diff/3-4/

Comment 107

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8736467 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_DOWNLOADS_INTEGRATION to Python configure. r=nalexander

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43253/diff/3-4/

Comment 108

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8736468 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_BEAM to Python configure. r=nalexander

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43255/diff/3-4/

Comment 109

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8736469 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_SEARCH_ACTIVITY to Python configure. r=nalexander

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43257/diff/3-4/

Comment 110

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8736470 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_MLS_STUMBLER to Python configure. r=nalexander

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43259/diff/3-4/

Comment 111

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8736471 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_DOWNLOAD_CONTENT_SERVICE to Python configure. r=nalexander

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43261/diff/3-4/

Comment 112

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8736472 [details]
MozReview Request: Bug 1257958 - Move MOZ_SWITCHBOARD to Python configure. r=nalexander

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43263/diff/3-4/

Comment 113

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8736473 [details]
MozReview Request: Bug 1257958 - Move MOZ_ANDROID_APZ to Python configure. r=nalexander

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43265/diff/3-4/

Comment 114

[Chris Manchester (limited bugmail, email directly)]

I'm going to carry forward the r+'s from Nick. The changes since them are cosmetic, with the exception of MOZ_ANDROID_APZ, whose default recently went from enabled in nightly to always on in the underlying confvars.sh.

Comment 115

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6ef6252740a3
https://hg.mozilla.org/integration/mozilla-inbound/rev/083f95a42cc8
https://hg.mozilla.org/integration/mozilla-inbound/rev/ddcd38c4444f
https://hg.mozilla.org/integration/mozilla-inbound/rev/755ad040785f
https://hg.mozilla.org/integration/mozilla-inbound/rev/69c8a0dd4059
https://hg.mozilla.org/integration/mozilla-inbound/rev/7ad61e955e0f
https://hg.mozilla.org/integration/mozilla-inbound/rev/df22a122b132
https://hg.mozilla.org/integration/mozilla-inbound/rev/9a78e646fa77
https://hg.mozilla.org/integration/mozilla-inbound/rev/356573b6a2ed
https://hg.mozilla.org/integration/mozilla-inbound/rev/ea3a604240fa
https://hg.mozilla.org/integration/mozilla-inbound/rev/e6650e1461ff

Comment 116

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6ef6252740a3
https://hg.mozilla.org/mozilla-central/rev/083f95a42cc8
https://hg.mozilla.org/mozilla-central/rev/ddcd38c4444f
https://hg.mozilla.org/mozilla-central/rev/755ad040785f
https://hg.mozilla.org/mozilla-central/rev/69c8a0dd4059
https://hg.mozilla.org/mozilla-central/rev/7ad61e955e0f
https://hg.mozilla.org/mozilla-central/rev/df22a122b132
https://hg.mozilla.org/mozilla-central/rev/9a78e646fa77
https://hg.mozilla.org/mozilla-central/rev/356573b6a2ed
https://hg.mozilla.org/mozilla-central/rev/ea3a604240fa
https://hg.mozilla.org/mozilla-central/rev/e6650e1461ff