Closed Bug 1255312 Opened 5 years ago Closed 5 years ago

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


(Firefox Build System :: General, defect)

Not set


(firefox48 fixed)

Tracking Status
firefox48 --- fixed


(Reporter: glandium, Assigned: glandium)


(Blocks 1 open bug)



(1 file, 1 obsolete file)

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.
Blocks: 1254410
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
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.
The complication is that passing down options to js/src/configure is done by old-configure...
... but... options handled by python configure for the top-level don't need to be passed to top-level, since it doesn't handle those options anyways...
Blocks: pyconfigure
Attachment #8728818 - Attachment is obsolete: true
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

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:
See other reviews:
Attachment #8729355 - Flags: review?(cmanchester)
Comment on attachment 8729355 [details]
MozReview Request: Bug 1255312 - Limit the options we pass down to old-configure

::: 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)
--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.
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
Attachment #8729355 - Flags: review?(cmanchester) → review+
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.