Move --enable-debug and MOZ_DEBUG_FLAGS to Python configure

RESOLVED FIXED in Firefox 50

Status

defect
RESOLVED FIXED
3 years ago
Last year

People

(Reporter: chmanchester, Assigned: chmanchester)

Tracking

(Depends on 1 bug, Blocks 1 bug)

unspecified
mozilla50
Dependency tree / graph

Firefox Tracking Flags

(firefox50 fixed)

Details

Attachments

(1 attachment)

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.
Attachment #8762226 - Flags: review?(mh+mozilla)
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 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/
Attachment #8762226 - Attachment description: Bug 1279369 - Move --enable-debug and MOZ_DEBUG_FLAGS to Python configure. → Bug 1279369 - Move --enable-debug, MOZ_DEBUG_FLAGS, and --enable-debug-symbols to Python configure.
Attachment #8762226 - Flags: review?(mh+mozilla)
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.
Attachment #8762226 - Flags: review?(mh+mozilla) → review+
Pushed by cmanchester@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2f946ab553f7
Move --enable-debug, MOZ_DEBUG_FLAGS, and --enable-debug-symbols to Python configure. r=glandium
https://hg.mozilla.org/mozilla-central/rev/2f946ab553f7
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.