Closed Bug 1281603 Opened 4 years ago Closed 4 years ago

Can't --disable-debug anymore if MOZ_DEBUG is set in the environment

Categories

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

defect
Not set
blocker

Tracking

(firefox50 fixed)

RESOLVED FIXED
mozilla50
Tracking Status
firefox50 --- fixed

People

(Reporter: bzbarsky, Assigned: chmanchester)

References

Details

Attachments

(1 file)

I just tried to build my opt build today.  It produced a debug-with-optimizations built instead.  The configure bit looks like this:

 0:04.40 Adding configure options from /Users/bzbarsky/mozilla/vanilla/mozilla/.mozconfig-fox-opt
 0:04.40   --enable-tests
 0:04.40   --enable-debug-symbols
 0:04.40   --enable-accessibility
 0:04.40   --disable-install-strip
 0:04.40   --with-ccache
 0:04.40   --enable-js-shell
 0:04.40   --enable-application=browser
 0:04.40   --disable-debug
 0:04.40   --enable-optimize
 0:04.40   --enable-profiling
 0:04.40   --enable-instruments
 0:04.40   --enable-dtrace

but the resulting config.status has:

    "MOZ_DEBUG": "Work around mouse event taps; see bug 699538", 
    "MOZ_MACBUNDLE_NAME": "NightlyDebug.app", 

hg bisect says the cause is:

The first bad revision is:
changeset:   302125:2f946ab553f7
user:        Chris Manchester <cmanchester@mozilla.com>
date:        Mon Jun 20 16:02:01 2016 -0700
summary:     Bug 1279369 - Move --enable-debug, MOZ_DEBUG_FLAGS, and --enable-debug-symbols to Python configure. r=glandium

(ok, that seems pretty plausible).  Note that I have no clue where that "work around..." string is coming from.

Anyway, this makes it impossible to get performance measurements, which is a bit of a problem for me right now.
Flags: needinfo?(cmanchester)
Ah, looks like you reintroduced bug 861453.
Summary: Can't --disable-debug anymore → Can't --disable-debug anymore if MOZ_DEBUG is set in the environment
bz, please verify this fix when you have a chance.
Assignee: nobody → cmanchester
Flags: needinfo?(cmanchester)
Yep, that fixes things.  Thank you!
Comment on attachment 8764414 [details]
Bug 1281603 - Respect --disable-debug even when MOZ_DEBUG is set in the environment.

https://reviewboard.mozilla.org/r/60326/#review57176

::: moz.configure:94
(Diff revision 1)
>            nargs='?',
>            help='Enable building with developer debug info '
>                 '(using the given compiler flags).')
>  
> -add_old_configure_assignment('MOZ_DEBUG',
> -                             depends_if('--enable-debug')(lambda _: True))
> +@depends('--enable-debug')
> +def debug_value(value):

You should just go with depends('--enable-debug')(lambda v: bool(v)) and not care about default or not default.
Attachment #8764414 - Flags: review?(mh+mozilla) → review+
https://reviewboard.mozilla.org/r/60326/#review57176

> You should just go with depends('--enable-debug')(lambda v: bool(v)) and not care about default or not default.

This would be a change in behavior, because setting MOZ_DEBUG in the environment would no longer create a debug build.
https://reviewboard.mozilla.org/r/60326/#review57176

> This would be a change in behavior, because setting MOZ_DEBUG in the environment would no longer create a debug build.

I'm inclined to land this as is to preserve the behavior and get the regression fixed. We can discuss further improvements as a follow up.
Pushed by cmanchester@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0cb8668f2278
Respect --disable-debug even when MOZ_DEBUG is set in the environment. r=glandium
If you want MOZ_DEBUG to have an effect, there should be an env="MOZ_DEBUG" on the option(), or a separate option, considering it's not doing the same as --enable-debug.
(I'll also note that strictly transposing the current behavior is not necessarily what ought to be implemented, depending on the case)
https://hg.mozilla.org/mozilla-central/rev/0cb8668f2278
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
(In reply to Mike Hommey [:glandium] from comment #9)
> If you want MOZ_DEBUG to have an effect, there should be an env="MOZ_DEBUG"
> on the option(), or a separate option, considering it's not doing the same
> as --enable-debug.

Actually, considering that I should have taken the suggestion. Requiring "--enable-debug" to create a debug build seems like the most reasonable way forward, all things considered. I'll get that fixed in a follow up.
Depends on: 1282135
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.