Js subconfigure is being passed --enable-application=foo

RESOLVED FIXED in Firefox 48

Status

defect
RESOLVED FIXED
3 years ago
Last year

People

(Reporter: glandium, Assigned: glandium)

Tracking

(Blocks 1 bug)

unspecified
mozilla48
Dependency tree / graph

Firefox Tracking Flags

(firefox48 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Assignee

Description

3 years ago
The js subconfigure is defaulting --enable-application/--enable-project to js, but it's being fed e.g. --enable-application=mobile/android on android builds.
Assignee

Updated

3 years ago
Blocks: 1254410

Updated

3 years ago
Blocks: 1254987
Attachment #8728818 - Flags: review?(gps) → review+
Comment on attachment 8728818 [details]
MozReview Request: Bug 1255312 - Force --enable-application/--enable-project=js when calling js subconfigure

https://reviewboard.mozilla.org/r/39157/#review36063
Assignee

Comment 3

3 years ago
We need a less shortsighted fix. We already need --with-external-source-dir not to be passed down to js for c-c, and we're going to have the same kind of problem when we start moving things out of old-configure that are supported in top-level and not in js.

So let's see.

Options known to old-configure.ins only are all listed in build/moz.configure/old.configure. All those should be passed down if they appear on the command line.

With js/moz.configure being included by toolkit/moz.configure, we've committed to options known to js/src/configure being known to top-level configure, which is good. So we'd definitely want to pass down any of those options appearing in js/moz.configure.

And we don't want to pass down any other option.
Assignee

Comment 4

3 years ago
The complication is that passing down options to js/src/configure is done by old-configure...
Assignee

Comment 5

3 years ago
... but... options handled by python configure for the top-level don't need to be passed to top-level old-configure.in, since it doesn't handle those options anyways...
Assignee

Updated

3 years ago
Blocks: pyconfigure
Assignee

Updated

3 years ago
Attachment #8728818 - Attachment is obsolete: true
Assignee

Comment 6

3 years ago
So far, we've been passing down all configure_args from mozconfig as
well as every flag appearing on sys.argv. This is overly broad and
causes problems for some options, like --enable-application.

However, we don't need all these options to be passed.

For the top-level old-configure, we need to pass the flags it can
handle, as well as the flags that we want passed down to
js/src/configure.

For js/src/old-configure, we only need to pass the flags it can handle.

The flags an old-configure can handle is defined by the list of flags
in @old_configure_options. The list of flags to pass down to
js/src/configure is defined by extra_old_configure_args.

And since the mozconfig configure_args are being injected into python
configure processing, the list of values we get in old_configure includes
the mozconfig configure_args.

Review commit: https://reviewboard.mozilla.org/r/39405/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/39405/
Attachment #8729355 - Flags: review?(cmanchester)
Comment on attachment 8729355 [details]
MozReview Request: Bug 1255312 - Limit the options we pass down to old-configure

https://reviewboard.mozilla.org/r/39405/#review36247

::: build/moz.configure/old.configure:377
(Diff revision 1)
> +    # We also pass it the options from js/moz.configure so that it can pass
> +    # them down to js/src/configure. Note this list is empty when running
> +    # js/src/configure, in which case we don't need to pass those options
> +    # to old-configure since old-configure doesn't handle them anyways.
> +    if extra_old_configure_args:
> +        cmd += extra_old_configure_args

If I set "--enable-js-shell" in my mozconfig, commenting out this loop doesn't have any interesting effect on the resulting config.status, because we're including js/moz.configure in toolkit/moz.configure. Is this necessary as long as that's the case?
Attachment #8729355 - Flags: review?(cmanchester)
Assignee

Comment 8

3 years ago
--enable-js-shell is special, because its default in top-level is disabled and in js/src it is enabled. So if it's not passed down properly, you get the default from js/src, which is enabled, so you don't see that, in fact, you broke it by removing the extra_old_configure_args. If you change the default in js/moz.configure, you should see a difference.
Assignee

Updated

3 years ago
Attachment #8729355 - Flags: review?(cmanchester)
Ok, thanks for the explanation.
Comment on attachment 8729355 [details]
MozReview Request: Bug 1255312 - Limit the options we pass down to old-configure

https://reviewboard.mozilla.org/r/39405/#review36255
Attachment #8729355 - Flags: review?(cmanchester) → review+

Comment 12

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/fa7740691562
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48

Updated

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