Closed
Bug 941782
Opened 11 years ago
Closed 11 years ago
Build dom/plugin/base & dom/plugin/ipc in unified mode
Categories
(Core Graveyard :: Plug-ins, defect)
Core Graveyard
Plug-ins
Tracking
(Not tracked)
RESOLVED
DUPLICATE
of bug 942633
People
(Reporter: BenWa, Assigned: BenWa)
References
Details
Attachments
(1 file, 3 obsolete files)
12.32 KB,
patch
|
BenWa
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•11 years ago
|
||
Assignee | ||
Comment 2•11 years ago
|
||
Comment 3•11 years ago
|
||
Comment on attachment 8336241 [details] [diff] [review]
patch
Review of attachment 8336241 [details] [diff] [review]:
-----------------------------------------------------------------
r=me with the below.
::: dom/plugins/ipc/moz.build
@@ +97,3 @@
> 'PluginInterposeOSX.mm',
> 'PluginUtilsOSX.mm',
> ]
In general I'd like us to avoid unifying anything that uses force prlogging. Please revert this to SOURCES, add a comment about why we don't unify it, and remove the next two hunks.
Attachment #8336241 -
Flags: review?(ehsan) → review+
Assignee | ||
Comment 4•11 years ago
|
||
Which files do you mean? I noticed that dom/plugins//base/nsPluginLogging.h force logging so this is a big problem since it's a header that could easily be pull in somewhere else later.
Assignee | ||
Comment 5•11 years ago
|
||
Had other changes qref into this.
Attachment #8336241 -
Attachment is obsolete: true
Comment 6•11 years ago
|
||
Hmm, crap!
$ find obj-ff-dbg -type f -name \*.pp | xargs grep nsPluginLogging.h | cut -d: -f1 | sort | uniq
obj-ff-dbg/dom/plugins/base/.deps/nsNPAPIPlugin.o.pp
obj-ff-dbg/dom/plugins/base/.deps/nsNPAPIPluginInstance.o.pp
obj-ff-dbg/dom/plugins/base/.deps/nsNPAPIPluginStreamListener.o.pp
obj-ff-dbg/dom/plugins/base/.deps/nsPluginHost.o.pp
obj-ff-dbg/dom/plugins/base/.deps/nsPluginStreamListenerPeer.o.pp
obj-ff-dbg/dom/plugins/base/.deps/nsPluginTags.o.pp
Can you please move the force prlogging from nsPluginLogging.h to each one of these files, and then exclude all of them from the unified build? (I think at that point this requires real peer review.)
Sorry for causing you pain!
Assignee | ||
Comment 7•11 years ago
|
||
I have an idea. Can we define MOZ_UNIFIED_SOURCE? We should only use this macro for defensive features like
#ifdef MOZ_UNIFIED_SOURCE
#error This file doesn't support unification because X
#endif
We could make 'fix' prlog to error out if it's include once with and once without force logging?
Comment 8•11 years ago
|
||
(In reply to Benoit Girard (:BenWa) from comment #7)
> I have an idea. Can we define MOZ_UNIFIED_SOURCE? We should only use this
> macro for defensive features like
> #ifdef MOZ_UNIFIED_SOURCE
> #error This file doesn't support unification because X
> #endif
>
> We could make 'fix' prlog to error out if it's include once with and once
> without force logging?
Great idea, filed as bug 941824.
Assignee | ||
Comment 10•11 years ago
|
||
Attachment #8336276 -
Attachment is obsolete: true
Attachment #8336990 -
Attachment is obsolete: true
Attachment #8337522 -
Flags: review+
Flags: needinfo?(bgirard)
Comment 11•11 years ago
|
||
Sorry Benoit, but bug 941854 uncovered some problems with this patch, and I fixed them as part of my patch in bug 942633. I think we should dupe this bug against that other one at this point :/
(For future reference, I'd appreciate if you could apply that patch locally while working on this stuff. I'll ping on that bug and ask for a fast review...
Updated•11 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → DUPLICATE
Updated•2 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•