Closed Bug 1349022 Opened 7 years ago Closed 7 years ago

Build system passes -DTRACING, which nothing cares about

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(firefox55 fixed)

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: jimb, Assigned: jimb)

Details

Attachments

(1 file)

As far as I can tell, there is no code in Mozilla Central that checks the TRACING preprocessor symbol, so there is no need for build/autoconf/compiler-opts.m4 to pass it.
Attachment #8849342 - Flags: review?(mh+mozilla)
Comment on attachment 8849342 [details] [diff] [review]
Don't put -DTRACING in the C/C++ flags for debug builds; nobody uses it.

IIRC it's used in system headers on MSVC.
Attachment #8849342 - Flags: review?(mh+mozilla)
I can't find anything about this on msdn.microsoft.com. I asked my local Windows programming expert:

<jimb> mhowell: Does #defining TRACING do anything interesting with Windows system headers?
<mhowell> jimb: nothing I know of

So I think this is [citation needed].
Flags: needinfo?(mh+mozilla)
Did some more searching and couldn't find anything mentioning a TRACING symbol. Also grepped the MSVC and Windows 8.1 SDK headers and didn't get any hits.

Plus, "TRACING" isn't nearly verbose enough to be a Microsoft symbol name. ;)
Comment on attachment 8849342 [details] [diff] [review]
Don't put -DTRACING in the C/C++ flags for debug builds; nobody uses it.

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

I went ahead just checking the entire preprocessed output with and without -DTRACING, on all platforms. There's no difference, except for the buildid in nsBrowserApp.i and nsAppRunner.i (which is expected).
Attachment #8849342 - Flags: review+
Flags: needinfo?(mh+mozilla)
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/80c08496fae6
Don't put -DTRACING in the C/C++ flags for debug builds since nobody uses it. r=glandium
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/80c08496fae6
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: