use separate namespace for 3rd party libs

RESOLVED FIXED

Status

SeaMonkey
Build Config
RESOLVED FIXED
16 years ago
9 years ago

People

(Reporter: TVL, Unassigned)

Tracking

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [minimo])

Attachments

(3 attachments)

(Reporter)

Description

16 years ago
Where mozilla uses 3rd partly libs (zlib, libjpeg, expat, etc.); if an embedder
wants to take advantage of static linkage of mozilla and they also use some of
these third party libs, they have to use mozillas version (since the the final
static binary on any platform can only have one final symbol of the given name).
 for example, say some third party app uses expat in utf8 mode; and they then
decide to add a browser component via mozilla; to take advantage of the static
linkage savings, they would have to recode to use expat in ucs2 mode (since
mozilla uses it in that mode).

maybe the third party parts could use namespaces to scope them uniquely within
mozilla, otherwise, it will probably take some kinda of macros to redefine
apis/static whild building mozilla, that doesn't leak thru the mozilla headers
so the embedder doesn't pickup the macros by mistake.
We already support using the system version of our 3rd party libraries (except
expat) in the gmake build system.  However, with the exception of zlib, I don't
think our copies of the 3rd party libraries are pristine*.  We'll want to fix
that if possible before moz1.0.   I don't think that using namespaces is an
option unless we want to completely drop the idea of letting people use system
versions of these libraries and then the static embedding builds will wind up 2
copies of the 3rd party libraries that they use.

I don't understand the expat example.  Are you saying that expat can only be
built with support for a single mode at a time?

* pristine with the exception of porting fixes
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: mozilla1.0
Priority: -- → P3
Target Milestone: --- → mozilla1.0
(Reporter)

Comment 2

16 years ago
pristine - have those porting mods ever been set to the maintainers of those
libraries?

expat example - it's a build time option as to what a XML_Char builds as (2 byte
vs. 1 byte); then one's code is written accordingly.

system libraries is a an ok option for the unixes, but not for the desktops
(mac/windows) since there aren't 'system' versions.  even on the unix boxes,
this can fail when it comes to shipping a commercial product since the version
of libjpeg could be newer then the verison the product was built against, and
hence wouldn't work.  scoping the 3rd parties within mozilla would result in two
copies (the mozilla version and then the version of embedder uses), but it seems
like that should be a option as it make mozilla easier to embed.
To my knowledge, the only porting fix to zlib was to ifdef out a system function
declaration for VMS.  I don't know if the contributor of the patch sent the
patch upstream.

Ugh.  That sounds like a flaw in libexpat if it is meant to be used as a general
purpose library.  I can see why you would want scoping for that specific library.

System libraries can refer to any library outside of the mozilla tree.  You
could tell the build system to use the version of libjpeg from your embed-app
tree if you wanted.  When it comes to shipping a commercial product, you take
the shared library dependency into account by having it listed as one of the
platform requirements.  You could even offer the library as an optional part of
the application install if you're really worried about that dependency.  Anyway,
that's a bit of a moot point since all of the 3rd party libraries are statically
linked into dynamic components atm.

While scoping might make mozilla easier to embed, I don't think we should start
scoping all 3rd party libraries in the tree. If there's a specific reason to
scope (like expat is fundamentally broken), then I can see doing so.  For the
3rd party image libraries, I don't see the need as they have a standard
signature for a particular release.  If the in-tree versions of the libraries
aren't using the standard signature, then we need to fix that.

IMO, resolving the library dependency problems in the application is ultimately
the responsibility of the embedder.  Some embedders may not want to take the
penalty of having multiple copies of these libraries.  Right now, they can use a
single instance of the library by telling the build system to not build the
in-tree version.   In the scoping world, they are forced to take the penalty. 
Then we have the maintenance penalty of changing all of the external functions
of all of the in-tree libraries and macro-izing their use across the tree.  The
maintnenance penalty extends to whenever we want to land a newer version of the
3rd party library as well.
Keywords: mozilla1.0
Summary: 3rd party libs cause problems for static embedding → use separate namespace for 3rd party libs
Target Milestone: mozilla1.0 → Future
Yeah, I'm going to have to do this at some point with Xft and friends if I ever
want to get it working with gtk2.
Mass reassign to new default build assignee
Assignee: seawood → mozbugs-build
Priority: P3 → --

Comment 6

14 years ago
*** Bug 213605 has been marked as a duplicate of this bug. ***

Comment 7

14 years ago
Created attachment 128742 [details] [diff] [review]
New namespace for expat library (needed on IRIX with GTK+ 2.x)

Patch from #213605.

Comment 8

14 years ago
Setting dependency of this bug on bug #181936 which is a specific instance
of the problem (libpng).  I'll post a proposed libpng patch over there shortly.
Depends on: 181936
Mass reassign of Build/Config bugs to Leaf.
Assignee: mozbugs-build → leaf
Target Milestone: Future → ---

Comment 10

14 years ago
m as a prefix is reserved for member variables, that makes attachement 128742 
kinda evil. bug 192139 (update expat) is a spot to be aware of.

Comment 11

14 years ago
this is a blocker bug for minimo.

how about some #define's to redefine the XML_ namespace?  MOZ_XML_ or something
like that comes to mind as a better namespace.  based on bug 192139, i think we
want to minimize our changes to the stock expat header files.
Whiteboard: [minimo]
In bug #181936 I used moz_png as a prefix for exported functions in
libpng.  I assume this is somewhat less evil with respect to
http://www.mozilla.org/hacking/mozilla-style-guide.html
because the second letter is always lowercase.  But I'd be happy to
change the prefix to MOZ_PNG or MOZ_png.

Comment 13

14 years ago
another interesting thing about our use of expat is that we internally tell
expat that XML_Char is equivalent to PRUnichar, but we don't expose that fact in
xmlparse.h.  yet, all of our code that uses xmlparse has casts to convert from
PRUnichar to XML_Char.  this is obviously done to make the compiler happy, but
really we are doing ourselves a big disservice here by not fixing xmlparse.h to
include some of the MOZILLA_CLIENT specific definitions that are present in
xmldef.h.

i don't think we want to just #include "xmldef.h" into xmlparse.h (as is) since
it redefines malloc and friends (for no good reason as far as I can tell, but
that's a side issue).  maybe with some cleanup, we could just include xmldef.h
from xmlparse.h.  yeah, maybe that's the thing to do.

Comment 14

14 years ago
Created attachment 134198 [details] [diff] [review]
patch - use #define to fix expat

this is a very simple patch that does the job for expat.

Updated

14 years ago
Attachment #134198 - Flags: superreview?(hjtoi-bugzilla)
Attachment #134198 - Flags: review?(leaf)
Comment on attachment 134198 [details] [diff] [review]
patch - use #define to fix expat

This looks fine to me, but I'll leave the last word to peterv since he's
working on integrating new Expat into the tree, see bug 192139.
Attachment #134198 - Flags: superreview?(hjtoi-bugzilla) → superreview+
Attachment #134198 - Flags: review?(leaf) → review+

Comment 16

14 years ago
patch checked in:

Checking in xmlparse/xmlparse.h;
/cvsroot/mozilla/expat/xmlparse/xmlparse.h,v  <--  xmlparse.h
new revision: 1.15; previous revision: 1.14

this fixes the problem for expat.  does that mean this bug can be marked FIXED?
 based on comment #1, i would think so.
It's fixed with respect to expat and now libpng, but there are other 3rd
party libraries such as libjpeg, libz, and (only in 1.4.x, libmng).  They
could all be treated with similar "MOZ_XXX" macro-prefixes.  Maybe wait a
few days and see if any bugs are triggered by the libpng and expat checkins,
though (I don't expect any).  I'm willing to prepare patches for those 3 libs
as time permits. Would you want to see the patches posted here or in a
separate bug report for each library, with dependency set to block this one?
libjpeg hasn't changed during the last few years, afaik. do we really need such
a set of defines for it?
The embedded libjpeg is actually slightly modified and is probably binary
incompatible with libjpeg found on the system.  For example, DCT_FLOAT_SUPPORTED
is turned off in mozilla's libjpeg, along with a few other features.
Also, libart in the SVG builds is modified and should have a namespace of some
sort, this causes epiphany to fail to render SVG properly (
http://bugzilla.gnome.org/show_bug.cgi?id=111123 ).
the modified libart also causes bug 223646 I believe
I've posted a patch to bug #223646 which is similar to the libpng patch; it
changes the libart exported function names from "art_*()" to "MOZ_ART_*()".
added dependency on bug #223646 (libart)
Depends on: 223646
Created attachment 136818 [details] [diff] [review]
fix some more things in expat

There were a few symbols missed in expat.  I removed hashtable.c entirely
because we don't need anything from that file (all of those functions are
duplicated with |static| in xmlparse.c)
Attachment #136818 - Flags: superreview?(dbaron)
Attachment #136818 - Flags: review?(darin)
Attachment #136818 - Flags: superreview?(dbaron) → superreview+

Comment 25

14 years ago
Comment on attachment 136818 [details] [diff] [review]
fix some more things in expat

r=darin (thanks bryner!)
Attachment #136818 - Flags: review?(darin) → review+

Comment 26

14 years ago
/* avoid conflicts wiht system versions of libexpat */

typo on "wiht" ;)
Comment on attachment 136818 [details] [diff] [review]
fix some more things in expat

requesting 1.6b approval.  This will fix bustage on linux gtk2 static builds,
as we're using for Firebird.
Attachment #136818 - Flags: approval1.6b?

Comment 28

14 years ago
Comment on attachment 136818 [details] [diff] [review]
fix some more things in expat

a=asa (on behalf of drivers) for checkin to 1.6beta
Attachment #136818 - Flags: approval1.6b? → approval1.6b+
Comment on attachment 136818 [details] [diff] [review]
fix some more things in expat

checked in. leaving bug open for other dependencies (like libart).

Updated

13 years ago
Depends on: 226733

Updated

13 years ago
Assignee: leaf → cmp
Product: Browser → Seamonkey

Comment 30

12 years ago
Mass reassign of open bugs for chase@mozilla.org to build@mozilla-org.bugs.
Assignee: chase → build
Mass re-assign of bugs that aren't on the build team radar, so bugs assigned to build@mozilla-org.bugs reflects reality.

If there is a bug you really think we need to be looking at, please *email* build@mozilla.org with a bug number and explanation.
Assignee: build → nobody
QA Contact: granrosebugs → build-config
All libs I know of use namespaced symbols now.
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.