Benoit Girard (:BenWa) Assignee
	Description • 11 years ago

      No description provided.

	Benoit Girard (:BenWa) Assignee
	Comment 2 • 11 years ago

https://tbpl.mozilla.org/?tree=Try&rev=d0e2e713964e

	(no longer active)
	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.

	Benoit Girard (:BenWa) 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.

	Benoit Girard (:BenWa) Assignee
	Comment 5 • 11 years ago

Had other changes qref into this.

	(no longer active)
	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!

	Benoit Girard (:BenWa) 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?

	(no longer active)
	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.

	Benoit Jacob [:bjacob] (mostly away)
	Comment 9 • 11 years ago

You need this to fix the linux build errors.

	Benoit Girard (:BenWa) Assignee
	Comment 10 • 11 years ago

https://tbpl.mozilla.org/?tree=Try&rev=f81de0447ab5

	(no longer active)
	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...