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

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Mike Hommey [:glandium]]

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.

Comment 1

[Mike Hommey [:glandium]]

Attachment: MozReview Request: Bug 1255312 - Force --enable-application/--enable-project=js when calling js subconfigure

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

Comment 2

[Gregory Szorc [:gps]]

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

Comment 3

[Mike Hommey [:glandium]]

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.

Comment 4

[Mike Hommey [:glandium]]

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

Comment 5

[Mike Hommey [:glandium]]

... 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...

Comment 6

[Mike Hommey [:glandium]]

Attachment: MozReview Request: Bug 1255312 - Limit the options we pass down to old-configure

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/

Comment 7

[Chris Manchester (limited bugmail, email directly)]

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?

Comment 8

[Mike Hommey [:glandium]]

--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.

Comment 9

[Chris Manchester (limited bugmail, email directly)]

Ok, thanks for the explanation.

Comment 10

[Chris Manchester (limited bugmail, email directly)]

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

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fa7740691562

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/fa7740691562