1279369 - Move --enable-debug and MOZ_DEBUG_FLAGS to Python configure

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Chris Manchester (limited bugmail, email directly)]

I'd like to be able to have --enable-debug work for artifact builds, re-factoring this leads naturally to moving this stuff over to Python configure.

Comment 1

[Chris Manchester (limited bugmail, email directly)]

Attachment: Bug 1279369 - Move --enable-debug, MOZ_DEBUG_FLAGS, and --enable-debug-symbols to Python configure.

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

Comment 2

[Mike Hommey [:glandium]]

Comment on attachment 8762226 [details]
Bug 1279369 - Move --enable-debug, MOZ_DEBUG_FLAGS, and --enable-debug-symbols to Python configure.

https://reviewboard.mozilla.org/r/59048/#review56288

I'd prefer if you moved --enable-debug-symbols at the same time.

::: build/moz.configure/toolchain.configure:624
(Diff revision 1)
> +@depends('MOZ_DEBUG_FLAGS', '--enable-debug', default_debug_flags)
> +def debug_flags(env_debug_flags, enable_debug_flags, default_debug_flags):
> +    # If MOZ_DEBUG_FLAGS is set, and --enable-debug is set to a value,
> +    # --enable-debug takes precedence.
> +    if len(enable_debug_flags):
> +        return enable_debug_flags[0].replace('\\\ ', ' ')

This is, interestingly, not what `sed -e 's|\\\ | |g'` does. The equivalent would be replace('\\ ', ' '), but either way, it's useless: It was cargo culted from --enable-optimize in bug 284767, and --enable-optimize got it in bug 68246 because passing --enable-optimize="-O2 -finlines" was setting MOZ_OPTIMIZE to '-O2\ -finlines'. IOW, autoconf was the one adding the backslashes in the first place somehow, which is not a problem we have with python configure. (and btw, it doesn't seem to be a problem with autoconf 2.13 either)

Comment 3

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8762226 [details]
Bug 1279369 - Move --enable-debug, MOZ_DEBUG_FLAGS, and --enable-debug-symbols to Python configure.

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

Comment 4

[Mike Hommey [:glandium]]

Comment on attachment 8762226 [details]
Bug 1279369 - Move --enable-debug, MOZ_DEBUG_FLAGS, and --enable-debug-symbols to Python configure.

https://reviewboard.mozilla.org/r/59048/#review56448

These are ideas of possible changes. I leave to you to decide whether they are worth doing.

::: build/moz.configure/toolchain.configure:636
(Diff revision 2)
> +@depends('MOZ_DEBUG_FLAGS', '--enable-debug', '--enable-debug-symbols',
> +         default_debug_flags)
> +def debug_flags(env_debug_flags, enable_debug_flags, debug_symbols_flags,
> +                default_debug_flags):
> +    if len(enable_debug_flags) and len(debug_symbols_flags):
> +        die('--enable-debug-symbols flags cannot be used with --enable-debug flags')

Taking a step back, we should actually error out for `--enable-debug=flags --enable-debug-symbols` as well, because we can't guarantee --enable-debug-symbols is doing anything when --enable-debug=flags is used.

Maybe it would all be simpler if we just did imply_option('--enable-debug-symbols', depends_if('--enable-debug')(lambda v: v))...

::: build/moz.configure/toolchain.configure:637
(Diff revision 2)
> +    # If MOZ_DEBUG_FLAGS is set, and --enable-debug is set to a value,
> +    # --enable-debug takes precedence.
> +    if len(enable_debug_flags):
> +        return enable_debug_flags[0]
> +    # If MOZ_DEBUG_FLAGS is set, and --enable-debug-symbols is set to a value,
> +    # --enable-debug-symbols takes precedence.
> +    if len(debug_symbols_flags):
> +        return debug_symbols_flags[0]
> +    if env_debug_flags:
> +        return env_debug_flags[0]

maybe just "enumerate" then:

for var in (enable_debug_flags, debug_symbols_flags, env_debug_flags):
    if len(var):
        return var[0]

... although if we go with imply_option, this comment becomes less relevant.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2f946ab553f7
Move --enable-debug, MOZ_DEBUG_FLAGS, and --enable-debug-symbols to Python configure. r=glandium

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2f946ab553f7