Closed
Bug 1279369
Opened 5 years ago
Closed 5 years ago
Move --enable-debug and MOZ_DEBUG_FLAGS to Python configure
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(firefox50 fixed)
RESOLVED
FIXED
mozilla50
Tracking | Status | |
---|---|---|
firefox50 | --- | fixed |
People
(Reporter: chmanchester, Assigned: chmanchester)
References
(Depends on 1 open bug, Blocks 1 open bug)
Details
Attachments
(1 file)
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.
Assignee | ||
Comment 1•5 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/59048/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/59048/
Attachment #8762226 -
Flags: review?(mh+mozilla)
Updated•5 years ago
|
Attachment #8762226 -
Flags: review?(mh+mozilla)
Comment 2•5 years ago
|
||
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)
Assignee | ||
Comment 3•5 years ago
|
||
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 4•5 years ago
|
||
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
Comment 6•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2f946ab553f7
Status: NEW → RESOLVED
Closed: 5 years ago
status-firefox50:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Updated•3 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•