All users were logged out of Bugzilla on October 13th, 2018

Move android "feature flags" to Python configure

RESOLVED FIXED in Firefox 48

Status

RESOLVED FIXED
3 years ago
8 months ago

People

(Reporter: chmanchester, Assigned: chmanchester)

Tracking

(Blocks: 1 bug)

unspecified
mozilla48
Dependency tree / graph

Firefox Tracking Flags

(firefox48 fixed)

Details

Attachments

(11 attachments, 8 obsolete attachments)

58 bytes, text/x-review-board-request
glandium
: review+
Details
58 bytes, text/x-review-board-request
nalexander
: review+
Details
58 bytes, text/x-review-board-request
nalexander
: review+
Details
58 bytes, text/x-review-board-request
nalexander
: review+
Details
58 bytes, text/x-review-board-request
nalexander
: review+
Details
58 bytes, text/x-review-board-request
nalexander
: review+
Details
58 bytes, text/x-review-board-request
nalexander
: review+
Details
58 bytes, text/x-review-board-request
nalexander
: review+
Details
58 bytes, text/x-review-board-request
nalexander
: review+
Details
58 bytes, text/x-review-board-request
nalexander
: review+
Details
58 bytes, text/x-review-board-request
nalexander
: review+
Details
(Assignee)

Description

3 years ago
There are several, and I'm going to keep nalexander in the loop on these, so splitting from bug 1257326.
(Assignee)

Comment 1

3 years ago
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
(Assignee)

Comment 2

3 years ago
Created attachment 8732352 [details]
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/
Attachment #8732352 - Flags: review?(mh+mozilla)
(Assignee)

Comment 3

3 years ago
Created attachment 8732353 [details]
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/
Attachment #8732353 - Flags: review?(mh+mozilla)
(Assignee)

Comment 4

3 years ago
Created attachment 8732354 [details]
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/
Attachment #8732354 - Flags: review?(mh+mozilla)
(Assignee)

Comment 5

3 years ago
Created attachment 8732355 [details]
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/
Attachment #8732355 - Flags: review?(mh+mozilla)
(Assignee)

Comment 6

3 years ago
Created attachment 8732356 [details]
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/
Attachment #8732356 - Flags: review?(mh+mozilla)
(Assignee)

Comment 7

3 years ago
Created attachment 8732357 [details]
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/
Attachment #8732357 - Flags: review?(mh+mozilla)
(Assignee)

Comment 8

3 years ago
Created attachment 8732358 [details]
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/
Attachment #8732358 - Flags: review?(mh+mozilla)
(Assignee)

Comment 9

3 years ago
Created attachment 8732359 [details]
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/
Attachment #8732359 - Flags: review?(mh+mozilla)
(Assignee)

Comment 10

3 years ago
Created attachment 8732360 [details]
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/
Attachment #8732360 - Flags: review?(mh+mozilla)
(Assignee)

Comment 11

3 years ago
Created attachment 8732361 [details]
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/
Attachment #8732361 - Flags: review?(mh+mozilla)
(Assignee)

Comment 12

3 years ago
Created attachment 8732362 [details]
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/
Attachment #8732362 - Flags: review?(mh+mozilla)
(Assignee)

Comment 13

3 years ago
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.
Flags: needinfo?(nalexander)
(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.
Flags: needinfo?(nalexander)
(Assignee)

Comment 15

3 years ago
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.
(Assignee)

Updated

3 years ago
Attachment #8732352 - Flags: review?(mh+mozilla)
(Assignee)

Updated

3 years ago
Attachment #8732353 - Flags: review?(mh+mozilla)
(Assignee)

Updated

3 years ago
Attachment #8732354 - Flags: review?(mh+mozilla)
(Assignee)

Updated

3 years ago
Attachment #8732355 - Flags: review?(mh+mozilla)
(Assignee)

Updated

3 years ago
Attachment #8732356 - Flags: review?(mh+mozilla)
(Assignee)

Updated

3 years ago
Attachment #8732357 - Flags: review?(mh+mozilla)
(Assignee)

Updated

3 years ago
Attachment #8732358 - Flags: review?(mh+mozilla)
(Assignee)

Updated

3 years ago
Attachment #8732359 - Flags: review?(mh+mozilla)
(Assignee)

Updated

3 years ago
Attachment #8732360 - Flags: review?(mh+mozilla)
(Assignee)

Updated

3 years ago
Attachment #8732361 - Flags: review?(mh+mozilla)
(Assignee)

Updated

3 years ago
Attachment #8732362 - Flags: review?(mh+mozilla)
(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.
Blocks: 1247781
(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.
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?
(Assignee)

Updated

3 years ago
Attachment #8732353 - Attachment description: MozReview Request: Bug 1257958 - Add a template for defines that are unconditionally enabled. → MozReview Request: Bug 1257958 - Add a template for feature flags that depend on the current milestone.
(Assignee)

Comment 19

3 years ago
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/
(Assignee)

Comment 20

3 years ago
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/
(Assignee)

Comment 21

3 years ago
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/
(Assignee)

Updated

3 years ago
Attachment #8732352 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Attachment #8732355 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Attachment #8732356 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Attachment #8732357 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Attachment #8732358 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Attachment #8732360 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Attachment #8732361 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Attachment #8732362 - Attachment is obsolete: true
(Assignee)

Comment 22

3 years ago
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.
Flags: needinfo?(nalexander)
(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.
Flags: needinfo?(nalexander)
(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.
(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.
(Assignee)

Updated

3 years ago
Attachment #8732353 - Attachment description: MozReview Request: Bug 1257958 - Add a template for feature flags that depend on the current milestone. → MozReview Request: Bug 1257958 - Add a template for feature flags that depend on the current milestone. r=glandium
Attachment #8732353 - Flags: review?(mh+mozilla)
(Assignee)

Comment 26

3 years ago
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/
(Assignee)

Updated

3 years ago
Attachment #8732359 - Attachment description: MozReview Request: Bug 1257958 - Move MOZ_ANDROID_EXCLUDE_FONTS to Python configure. → MozReview Request: Bug 1257958 - Move MOZ_ANDROID_EXCLUDE_FONTS to Python configure. r=nalexander
Attachment #8732359 - Flags: review?(nalexander)
(Assignee)

Comment 27

3 years ago
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/
(Assignee)

Updated

3 years ago
Attachment #8732354 - Attachment description: MozReview Request: Bug 1257958 - Move MOZ_LOCALE_SWITCHER to Python configure. → MozReview Request: Bug 1257958 - Move MOZ_LOCALE_SWITCHER to Python configure. r=nalexander
Attachment #8732354 - Flags: review?(nalexander)
(Assignee)

Comment 28

3 years ago
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/
Attachment #8732359 - Flags: review?(nalexander) → review+
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 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.
Attachment #8732353 - Flags: review+
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.
Attachment #8732354 - Flags: review?(nalexander) → review+
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)...
Attachment #8732353 - Flags: review?(mh+mozilla)
(Assignee)

Comment 33

3 years ago
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.
(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))
(Assignee)

Comment 35

3 years ago
(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.
(Assignee)

Updated

3 years ago
Depends on: 1259876
(Assignee)

Comment 36

3 years ago
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/
Attachment #8732353 - Flags: review?(mh+mozilla)
(Assignee)

Comment 37

3 years ago
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/
(Assignee)

Comment 38

3 years ago
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 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.
Attachment #8732353 - Flags: review?(mh+mozilla)
(Assignee)

Comment 40

3 years ago
(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.
Flags: needinfo?(mh+mozilla)
(Assignee)

Comment 41

3 years ago
I see, the patches that just hit inbound outdate this.
Flags: needinfo?(mh+mozilla)
Bug 1257823 did, but bug 1260066 that just landed enforces it.
(Assignee)

Comment 43

3 years ago
Created attachment 8736466 [details]
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/
Attachment #8736466 - Flags: review?(nalexander)
Attachment #8736467 - Flags: review?(nalexander)
Attachment #8736468 - Flags: review?(nalexander)
Attachment #8736469 - Flags: review?(nalexander)
Attachment #8736470 - Flags: review?(nalexander)
Attachment #8736471 - Flags: review?(nalexander)
Attachment #8736472 - Flags: review?(nalexander)
Attachment #8736473 - Flags: review?(nalexander)
Attachment #8732353 - Flags: review?(mh+mozilla)
(Assignee)

Comment 44

3 years ago
Created attachment 8736467 [details]
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/
(Assignee)

Comment 45

3 years ago
Created attachment 8736468 [details]
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/
(Assignee)

Comment 46

3 years ago
Created attachment 8736469 [details]
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/
(Assignee)

Comment 47

3 years ago
Created attachment 8736470 [details]
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/
(Assignee)

Comment 48

3 years ago
Created attachment 8736471 [details]
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/
(Assignee)

Comment 49

3 years ago
Created attachment 8736472 [details]
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/
(Assignee)

Comment 50

3 years ago
Created attachment 8736473 [details]
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/
(Assignee)

Comment 51

3 years ago
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/
(Assignee)

Comment 52

3 years ago
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/
(Assignee)

Comment 53

3 years ago
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/
(Assignee)

Updated

3 years ago
Attachment #8732354 - Flags: review+ → review?(nalexander)
(Assignee)

Updated

3 years ago
Attachment #8732359 - Flags: review+ → review?(nalexander)
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.
Attachment #8732353 - Flags: review+
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.
Attachment #8736466 - Flags: review?(nalexander) → review+
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
Attachment #8736467 - Flags: review?(nalexander) → review+
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
Attachment #8736468 - Flags: review?(nalexander) → review+
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
Attachment #8736469 - Flags: review?(nalexander) → review+
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
Attachment #8736470 - Flags: review?(nalexander) → review+
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
Attachment #8736471 - Flags: review?(nalexander) → review+
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.
Attachment #8736472 - Flags: review?(nalexander) → review+
Attachment #8736473 - Flags: review?(nalexander) → review+
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 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
Attachment #8732359 - Flags: review?(nalexander) → review+
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
Attachment #8732354 - Flags: review?(nalexander) → review+
chmanchester -- nice series!  Sorry to make you wait on my reviews.
Status: NEW → ASSIGNED
Let's plan to email dev-builds and mobile-firefox-dev.  I can do this if you prefer.
(Assignee)

Comment 67

3 years ago
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.
(Assignee)

Comment 68

3 years ago
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 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.
Attachment #8732353 - Flags: review?(mh+mozilla)
(Assignee)

Comment 70

3 years ago
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.
(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.
(Assignee)

Comment 72

3 years ago
(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.
(Assignee)

Comment 73

3 years ago
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'
(Assignee)

Comment 74

3 years ago
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/
Attachment #8732353 - Attachment description: MozReview Request: Bug 1257958 - Add a template for feature flags that depend on the current milestone. r=glandium → MozReview Request: Bug 1257958 - Add a template to construct an environment-set option that sets the corresponding variable. r=glandium
Attachment #8732353 - Flags: review?(mh+mozilla)
(Assignee)

Comment 75

3 years ago
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/
(Assignee)

Comment 76

3 years ago
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/
(Assignee)

Comment 77

3 years ago
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/
(Assignee)

Comment 78

3 years ago
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/
(Assignee)

Comment 79

3 years ago
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/
(Assignee)

Comment 80

3 years ago
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/
(Assignee)

Comment 81

3 years ago
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/
(Assignee)

Comment 82

3 years ago
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/
(Assignee)

Comment 83

3 years ago
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/
(Assignee)

Comment 84

3 years ago
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 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)
Attachment #8732353 - Flags: review?(mh+mozilla)
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.
(Assignee)

Comment 87

3 years ago
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.
(Assignee)

Comment 88

3 years ago
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/
Attachment #8732353 - Flags: review?(mh+mozilla)
(Assignee)

Comment 89

3 years ago
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/
(Assignee)

Comment 90

3 years ago
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/
(Assignee)

Comment 91

3 years ago
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/
(Assignee)

Comment 92

3 years ago
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/
(Assignee)

Comment 93

3 years ago
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/
(Assignee)

Comment 94

3 years ago
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/
(Assignee)

Comment 95

3 years ago
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/
(Assignee)

Comment 96

3 years ago
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/
(Assignee)

Comment 97

3 years ago
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/
(Assignee)

Comment 98

3 years ago
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/
(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)
(Assignee)

Comment 100

3 years ago
(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.
Oh, I missed the indentation on the last line.
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
Attachment #8732353 - Flags: review?(mh+mozilla) → review+
(Assignee)

Comment 103

3 years ago
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/
Attachment #8736466 - Attachment description: MozReview Request: Bug 1257958 - Move MOZ_ANDROID_GCM to Python configure. → MozReview Request: Bug 1257958 - Move MOZ_ANDROID_GCM to Python configure. r=nalexander
Attachment #8736467 - Attachment description: MozReview Request: Bug 1257958 - Move MOZ_ANDROID_DOWNLOADS_INTEGRATION to Python configure. → MozReview Request: Bug 1257958 - Move MOZ_ANDROID_DOWNLOADS_INTEGRATION to Python configure. r=nalexander
Attachment #8736468 - Attachment description: MozReview Request: Bug 1257958 - Move MOZ_ANDROID_BEAM to Python configure. → MozReview Request: Bug 1257958 - Move MOZ_ANDROID_BEAM to Python configure. r=nalexander
Attachment #8736469 - Attachment description: MozReview Request: Bug 1257958 - Move MOZ_ANDROID_SEARCH_ACTIVITY to Python configure. → MozReview Request: Bug 1257958 - Move MOZ_ANDROID_SEARCH_ACTIVITY to Python configure. r=nalexander
Attachment #8736470 - Attachment description: MozReview Request: Bug 1257958 - Move MOZ_ANDROID_MLS_STUMBLER to Python configure. → MozReview Request: Bug 1257958 - Move MOZ_ANDROID_MLS_STUMBLER to Python configure. r=nalexander
Attachment #8736471 - Attachment description: MozReview Request: Bug 1257958 - Move MOZ_ANDROID_DOWNLOAD_CONTENT_SERVICE to moz.build. → MozReview Request: Bug 1257958 - Move MOZ_ANDROID_DOWNLOAD_CONTENT_SERVICE to Python configure. r=nalexander
Attachment #8736472 - Attachment description: MozReview Request: Bug 1257958 - Move MOZ_SWITCHBOARD to Python configure. → MozReview Request: Bug 1257958 - Move MOZ_SWITCHBOARD to Python configure. r=nalexander
Attachment #8736473 - Attachment description: MozReview Request: Bug 1257958 - Move MOZ_ANDROID_APZ to Python configure. → MozReview Request: Bug 1257958 - Move MOZ_ANDROID_APZ to Python configure. r=nalexander
(Assignee)

Comment 104

3 years ago
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/
(Assignee)

Comment 105

3 years ago
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/
(Assignee)

Comment 106

3 years ago
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/
(Assignee)

Comment 107

3 years ago
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/
(Assignee)

Comment 108

3 years ago
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/
(Assignee)

Comment 109

3 years ago
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/
(Assignee)

Comment 110

3 years ago
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/
(Assignee)

Comment 111

3 years ago
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/
(Assignee)

Comment 112

3 years ago
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/
(Assignee)

Comment 113

3 years ago
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/
(Assignee)

Comment 114

3 years ago
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.

Updated

8 months ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.