1300290 - stylo: implied --enable-jemalloc=moz with --enable-stylo conflicts with --enable-jemalloc

Status: RESOLVED FIXED

(Firefox Build System :: General, defect, P3)

Description

[Bobby Holley (:bholley)]

When I add --enable-stylo to build/mozconfig.rust on incubator/stylo, I get the following on TaskCluster builds:

mozbuild.configure.options.ConflictingOptionError: Cannot add '--enable-jemalloc=moz' to the implied set because it conflicts with '--enable-jemalloc' that was added earlier

Obviously this is a temporary setup, but it would be nice to get this solved somehow (in the mean time I've commented out the imply_option line. Any suggestions glandium?

Comment 1

[Mike Hommey [:glandium]]

Remove --enable-jemalloc from your mozconfig.

Comment 2

[Bobby Holley (:bholley)]

(In reply to Mike Hommey [:glandium] from comment #1)
> Remove --enable-jemalloc from your mozconfig.

This is on our CI builds, so these these are the builder mozconfigs (not local ones). Should we just hack these up in incubator/stylo? If so, what's the easiest way to do that? Modify memory.configure?

Comment 3

[Mike Hommey [:glandium]]

No, we should just remove --enable-jemalloc from all mozconfigs in the tree. (and more generally, all the things that are defaults)

Comment 4

[Chris Peterson [:cpeterson]]

Unblocking the stylo-incubator meta bug because this jemalloc issue is not specific to the incubator/stylo test repo.

Comment 5

[Gregory Szorc [:gps]]

This started breaking again on the "upstream" bookmark of the stylo repo. I don't understand why it started breaking: that head should be similar to the "stylo" bookmark/head in terms of file content related to this part of the build.

https://public-artifacts.taskcluster.net/ZwCFExBCSZWKuRC51YlgVQ/0/public/logs/live_backing.log

Comment 6

[Gregory Szorc [:gps]]

Ah, the stylo bookmark (unmerged foo into mozilla-central) has the following patch:

diff --git a/toolkit/moz.configure b/toolkit/moz.configure
--- a/toolkit/moz.configure
+++ b/toolkit/moz.configure
@@ -571,7 +571,10 @@ def stylo(value):

 set_config('MOZ_STYLO', stylo)
 set_define('MOZ_STYLO', stylo)
-imply_option('--enable-jemalloc', depends_if('--enable-stylo')(lambda _: 'moz'))
+
+# Avoid mozbuild.configure.options.ConflictingOptionError until bug 1300290 is
+# resolved.
+#imply_option('--enable-jemalloc', depends_if('--enable-stylo')(lambda _: 'moz'))


We should get this unhacked and integrated into mozilla-central.

Comment 7

[Gregory Szorc [:gps]]

Attachment: Bug 1300290 - Avoid --enable-dmd and --enable-stylo setting conflicting --enable-jemalloc values;

Before, --enable-dmd implied --enable-jemalloc. If --enable-stylo was
also set, it tried to imply --enable-jemalloc=moz. Configure barfed
due to setting the value twice.

The commit refactors the logic for implying the --enable-jemalloc value
to set the proper value depending on the state of dmd and stylo.

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

Comment 8

[Bobby Holley (:bholley)]

Yeah, sorry I never followed up here - thanks for taking this Greg!

Comment 9

[Mike Hommey [:glandium]]

Comment on attachment 8815962 [details]
Bug 1300290 - Avoid --enable-dmd and --enable-stylo setting conflicting --enable-jemalloc values;

https://reviewboard.mozilla.org/r/96730/#review96960

::: toolkit/moz.configure:68
(Diff revision 1)
>  
>  set_config('MOZ_DMD', dmd)
>  set_define('MOZ_DMD', dmd)
>  add_old_configure_assignment('MOZ_DMD', dmd)
>  imply_option('--enable-profiling', dmd)
> -imply_option('--enable-jemalloc', dmd)
> +# --enable-jemalloc is handled below.

s/handled/implied/

::: toolkit/moz.configure:582
(Diff revision 1)
> +    if stylo:
> +        return 'moz'
> +    elif dmd:
> +        return True
> +
> +imply_option('--enable-jemalloc', depends_if(jemalloc)(lambda x: x),

depends_if(jemalloc)(lambda x: x), considering jemalloc returns one of ('moz', True, None) is equivalent to:
depends(jemalloc)(lambda x: x), which is equivalent to:
jemalloc.

::: toolkit/moz.configure:583
(Diff revision 1)
> +        return 'moz'
> +    elif dmd:
> +        return True
> +
> +imply_option('--enable-jemalloc', depends_if(jemalloc)(lambda x: x),
> +             '--enable-dmd or --enable-stylo')

reason=

Comment 10

[Gregory Szorc [:gps]]

Comment on attachment 8815962 [details]
Bug 1300290 - Avoid --enable-dmd and --enable-stylo setting conflicting --enable-jemalloc values;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/96730/diff/1-2/

Comment 11

[Mike Hommey [:glandium]]

Comment on attachment 8815962 [details]
Bug 1300290 - Avoid --enable-dmd and --enable-stylo setting conflicting --enable-jemalloc values;

https://reviewboard.mozilla.org/r/96730/#review97420

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e907fb6bfcfe
Avoid --enable-dmd and --enable-stylo setting conflicting --enable-jemalloc values; r=glandium

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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