Closed Bug 1253553 Opened 4 years ago Closed 4 years ago

Move --enable-artifact-builds, --disable-compile-environment and --enable-build-backend to moz.configure

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(firefox48 fixed)

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: glandium, Assigned: glandium)

References

Details

Attachments

(1 file)

They are all tied together.
Note I'm using raw primitives until patterns emerge and we decide on
some helpers.

Review commit: https://reviewboard.mozilla.org/r/38137/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/38137/
Attachment #8726649 - Flags: review?(nalexander)
Attachment #8726649 - Flags: review?(cmanchester)
Attachment #8726649 - Flags: review?(nalexander) → review+
Comment on attachment 8726649 [details]
MozReview Request: Bug 1253553 - Move --enable-artifact-builds, --disable-compile-environment and --enable-build-backend to moz.configure

https://reviewboard.mozilla.org/r/38137/#review34881

::: build/moz.configure/old.configure:66
(Diff revision 1)
> -                      python):
> +                      python, compile_env):

It would help grep if the name of the @depends function and the name of the variable were the same, although perhaps that obscures more than clarifies.

::: moz.configure:33
(Diff revision 1)
> +def build_backends_choices(help):

Again, why `help`?  Worth calling the pattern out until it's standard.

::: moz.configure:38
(Diff revision 1)
> +option('--enable-build-backend', nargs='+', choices=build_backends_choices,

This choices=decorated function strikes my eye as odd.

::: moz.configure:48
(Diff revision 1)
> +    set_config('BUILD_BACKENDS', unique_list(all_backends))

Why doesn't this produce `['FM+RM', 'RM', 'FM']`?  Is that not a problem?
Comment on attachment 8726649 [details]
MozReview Request: Bug 1253553 - Move --enable-artifact-builds, --disable-compile-environment and --enable-build-backend to moz.configure

https://reviewboard.mozilla.org/r/38137/#review35125

Great stuff.
Attachment #8726649 - Flags: review?(cmanchester) → review+
https://reviewboard.mozilla.org/r/38137/#review35205

::: moz.configure:26
(Diff revision 1)
> +    if value:
> +        set_config('COMPILE_ENVIRONMENT', '1')

I'm finding this breaks an artifact enabled build locally, because of this check, which relies on COMPILE_ENVIRONMENT being set to an empty value:

https://dxr.mozilla.org/mozilla-central/rev/be593a64d7c6a826260514fe758ef32a6ee580f7/python/mozbuild/mozbuild/frontend/emitter.py#184
https://hg.mozilla.org/mozilla-central/rev/addd5fc00284
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Blocks: 1255197
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.