Closed
Bug 1072605
Opened 10 years ago
Closed 10 years ago
gfx/2d/Logging.h checks for PR_LOGGING before including prlog.h
Categories
(Core :: Graphics, defect)
Core
Graphics
Tracking
()
RESOLVED
FIXED
mozilla35
People
(Reporter: erahm, Assigned: erahm)
References
Details
Attachments
(1 file, 1 obsolete file)
1.72 KB,
patch
|
bas.schouten
:
review+
|
Details | Diff | Splinter Review |
|PR_LOGGING| is defined in "prlog.h", but we check for it prior to including "prlog.h" [1] We should just include prlog.h outside of the #define scope and then change the various |#if defined(DEBUG) || defined(PR_LOGGING)| to just |#if defined(PR_LOGGING)| [1] http://hg.mozilla.org/mozilla-central/annotate/5e704397529b/gfx/2d/Logging.h#l30
Assignee | ||
Comment 1•10 years ago
|
||
This is pretty basic. I unconditionally #include <prlog.h> and only look at PR_LOGGING.
Attachment #8495408 -
Flags: review?(bas)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → erahm
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•10 years ago
|
||
try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=51919550f276
Comment 3•10 years ago
|
||
Comment on attachment 8495408 [details] [diff] [review] Just use PR_LOGGING to determine if logging is enabled Review of attachment 8495408 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/2d/Logging.h @@ +9,5 @@ > #include <string> > #include <sstream> > #include <stdio.h> > > +#include <prlog.h> You can't unconditionally include this file inside Moz2D. Since it's not available in the stand-alone build.
Attachment #8495408 -
Flags: review?(bas) → review-
Assignee | ||
Comment 4•10 years ago
|
||
Comment on attachment 8495408 [details] [diff] [review] Just use PR_LOGGING to determine if logging is enabled Review of attachment 8495408 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/2d/Logging.h @@ +9,5 @@ > #include <string> > #include <sstream> > #include <stdio.h> > > +#include <prlog.h> We currently do this in debug builds, so nothing has really changed. Unless we don't support stand-alone debug builds? If that's the case what would you suggest wrapping this include in?
Assignee | ||
Updated•10 years ago
|
Assignee | ||
Updated•10 years ago
|
Flags: needinfo?(bas)
Comment 5•10 years ago
|
||
Comment on attachment 8495408 [details] [diff] [review] Just use PR_LOGGING to determine if logging is enabled Review of attachment 8495408 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/2d/Logging.h @@ +9,5 @@ > #include <string> > #include <sstream> > #include <stdio.h> > > +#include <prlog.h> This is a recent change, it's good you pointed it out, ut's a little silly, Debug stand-alone builds don't define DEBUG (rather _DEBUG). It's actually a bug already, can we come up with a sane way of fixing this include, then I can fix the defining of DEBUG :).
Assignee | ||
Comment 6•10 years ago
|
||
MOZ_LOGGING should be sufficiently mozilla specific to keep prlog.h out of the standalone build. This should also fix bustage related to DEBUG builds on non-mozilla targets.
Attachment #8500230 -
Flags: review?(bas)
Assignee | ||
Updated•10 years ago
|
Attachment #8495408 -
Attachment is obsolete: true
Comment 7•10 years ago
|
||
Comment on attachment 8500230 [details] [diff] [review] Just use PR_LOGGING to determine if logging is enabled Review of attachment 8500230 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me!
Attachment #8500230 -
Flags: review?(bas) → review+
Assignee | ||
Comment 8•10 years ago
|
||
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/ef7d6554b13d
Flags: needinfo?(bas)
Comment 9•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ef7d6554b13d
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in
before you can comment on or make changes to this bug.
Description
•