Builds with --disable-debug and some other options are broken when the environment contains variables like MOZ_DEBUG

RESOLVED FIXED in mozilla23

Status

defect
RESOLVED FIXED
6 years ago
2 years ago

People

(Reporter: bzbarsky, Assigned: glandium)

Tracking

unspecified
mozilla23
x86
macOS
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(7 attachments)

Steps to reproduce:

1)  Configure with --disable-javaxpcom --enable-tests --enable-debugger-info-modules --enable-debug-symbols --enable-accessibility --enable-codesighs --disable-install-strip --enable-shared-js --enable-application=browser --disable-debug --enable-optimize --enable-profiling --enable-instruments --enable-dtrace

2)  Try to compile.

EXPECTED RESULTS: Builds.

ACTUAL RESULTS: Fails because we're passing -DDEBUG to the compiler but didn't set all the other defines that are expected to be set if -DDEBUG.

ADDITIONAL INFORMATION: Backing out the patch for bug 857557 fixes the problem.
Note that --enable-debug-sumbols is the most likely option to be causing trouble here....
Er, "symbols".  ;)
I'm, uh, at a loss here. The config.status are identical. And, the differences in autoconf.mk are only empty variables. The only thing I can think of is that many of these empty variables have an extra space at the end. e.g. |MOZ_DEBUG = |

However, make ignores spaces like this when evaluating the variable's value. Even if the space were making it into the variable value, ifeq and friends ignore leading whitespace. This is explained at https://www.gnu.org/software/make/manual/make.html#Conditional-Syntax.

I'm scratching my head over this :/
So I tried hacking the ifdef MOZ_DEBUG bit in config.mk to look like this:

ifdef MOZ_DEBUG
  _DEBUG_CFLAGS += $(MOZ_DEBUG_ENABLE_DEFS) -I/ab$(MOZ_DEBUG)cd
  XULPPFLAGS += $(MOZ_DEBUG_ENABLE_DEFS) -I/ab$(MOZ_DEBUG)cd
else
  _DEBUG_CFLAGS += $(MOZ_DEBUG_DISABLE_DEFS)
  XULPPFLAGS += $(MOZ_DEBUG_DISABLE_DEFS)
endif

just to see what it looks like.  With that change, and after bug 857557, the output of make showbuild contains this gem in all the cflags-related stuff:

  -I/abWork around mouse event taps; see bug 699538cd
So the upshot is that MOZ_DEBUG is ending up defined and hence we take that first branch.

And the reason for _that_ is that it's defined in my environment in general, to the above string.  And if you read that bug you see why: when that env var is defined at _runtime_ we turn off some stuff that otherwise breaks debugging.

So the problem is that since the build system is not explicitly setting MOZ_DEBUG anymore the setting in my environment takes effect.  That seems pretty undesirable in general, not just for MOZ_DEBUG....
Summary: Builds with --disable-debug and some other options are broken → Builds with --disable-debug and some other options are broken when the environment contains variables like MOZ_DEBUG
Assignee: nobody → mh+mozilla
Comment on attachment 737102 [details] [diff] [review]
Restore empty substs after bug 857557, but put them in a separate file, unlisted as a dependency for everything

Review of attachment 737102 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good.

I'm still scratching my head over why things are breaking. Am I wrong about make's behavior when it comes to ignoring whitespace? Or, do we have undiscovered lines somewhere combining/comparing empty variables in such a way as to through off conditionals later on? If that's the case, then evaluating empty variables is definitely the right thing to do as we don't know where such code could live. Yet, if this were really a problem, I would have expected mass breakage when we initially removed all empty variables. So weird.
Attachment #737102 - Flags: review?(gps) → review+
(In reply to Gregory Szorc [:gps] from comment #13)
> Comment on attachment 737102 [details] [diff] [review]
> Restore empty substs after bug 857557, but put them in a separate file,
> unlisted as a dependency for everything
> 
> Review of attachment 737102 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good.
> 
> I'm still scratching my head over why things are breaking. Am I wrong about
> make's behavior when it comes to ignoring whitespace?

$ cat /tmp/test.mk
FOO =

foo:
	@echo FOO = $(FOO)
	@echo BAR = $(BAR)
$ make -f /tmp/test.mk
FOO =
BAR =
$ make -f /tmp/test.mk FOO=foo BAR=bar
FOO = foo
BAR = bar
$ FOO=foo BAR=bar make -f /tmp/test.mk
FOO =
BAR = bar

Arguments beat Makefile, Makefile beats environment.
Relanded with a smal fixup irc-reviewed by ted.
https://hg.mozilla.org/integration/mozilla-inbound/rev/2248290342c9
Note this might need a change in c-c.
https://hg.mozilla.org/mozilla-central/rev/2248290342c9
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.