Closed Bug 536144 Opened 12 years ago Closed 12 years ago

DEFINES not set properly for browser/app/application.ini preprocessing when building --with-libxul-sdk

Categories

(Firefox Build System :: General, defect)

x86
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: wolfiR, Assigned: wolfiR)

Details

Attachments

(1 file, 1 obsolete file)

Attached patch possible patch (obsolete) — Splinter Review
I'm trying to enable the crashreporter for xulrunner and firefox in distribution packages.
http://mxr.mozilla.org/mozilla-central/source/browser/app/application.ini#61

I'm wondering how 
#if MOZILLA_OFFICIAL
can work anywhere if substitution is done through Preprocessor.py (interestingly I cannot find the substitution of application.ini anywhere in the log so just guessing it's done from Preprocessor.py).
You just need to export MOZILLA_OFFICIAL=1 in your mozconfig (or environment):
http://hg.mozilla.org/build/buildbot-configs/file/tip/mozilla2/linux/mozilla-central/nightly/mozconfig#l19
and configure will set it:
http://mxr.mozilla.org/mozilla-central/source/configure.in#7980
and the Makefile where application.ini gets preprocessed sticks it in defines:
http://mxr.mozilla.org/mozilla-central/source/browser/app/Makefile.in#226
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → INVALID
Hmm, doesn't work here.
I have MOZILLA_OFFICIAL set in my environment and it's also set in config/autoconf.mk correctly. Still application.ini doesn't get the Enabled flag.
As said I'm missing to find the preprocessor run in the build log for whatever reason and so cannot confirm if the flag is added correctly (this is really weird since I usually can see every action in the build log).
Grepping the build log also doesn't show any usage of -DMOZILLA_OFFICIAL.
Now I moved this 

ifdef MOZILLA_OFFICIAL
DEFINES += -DMOZILLA_OFFICIAL
endif

up in the Makefile and now it works correctly.
So something is fishy I guess.
That DEFINES addition is too late for sure in the Makefile.
Just confirmed that it's missing from the preprocessor run where it lives now.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
http://mxr.mozilla.org/mozilla1.9.2/source/browser/app/Makefile.in#84

Here rules.mk gets loaded. Placing the addition DEFINE in front of that fixes my issue.
Ah. You're partially right. The problem here is that the addition to DEFINES happens on the other side of that ifdef, so it doesn't get set when building --with-libxul-sdk. It doesn't matter if you set the variable before or after rules.mk is included, but here it's not being set at all.
Status: REOPENED → NEW
Summary: crash reporter never gets enabled in application.ini → DEFINES not set properly for browser/app/application.ini preprocessing when building --with-libxul-sdk
Attached patch patchSplinter Review
Moves out the DEFINES from the wrong ifdef.
Not sure if I got everything what should live outside in any case.
Attachment #418614 - Attachment is obsolete: true
Attachment #418628 - Flags: review?(ted.mielczarek)
Assignee: nobody → mozilla
Comment on attachment 418628 [details] [diff] [review]
patch

Maybe not, but this should be fine.
Attachment #418628 - Flags: review?(ted.mielczarek) → review+
http://hg.mozilla.org/mozilla-central/rev/b52809a8acc7
Status: NEW → RESOLVED
Closed: 12 years ago12 years ago
Resolution: --- → FIXED
Component: Build Config → General
Product: Firefox → Firefox Build System
You need to log in before you can comment on or make changes to this bug.