119934 - use separate namespace for 3rd party libs

Status: RESOLVED FIXED

(SeaMonkey :: Build Config, defect)

Description

[TVL]

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.

Comment 1

[hacker formerly known as [e-mail]]

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

Comment 2

[TVL]

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.

Comment 3

[hacker formerly known as [e-mail]]

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.

Comment 4

[Christopher Blizzard (:blizzard)]

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.

Comment 5

[hacker formerly known as [e-mail]]

Mass reassign to new default build assignee

Comment 6

[cls]

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

Comment 7

[The Written Word]

Attachment: New namespace for expat library (needed on IRIX with GTK+ 2.x)

Patch from #213605.

Comment 8

[Glenn Randers-Pehrson]

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.

Comment 9

[(default assignee for unassigned bugs. does not receive bugmail)]

Mass reassign of Build/Config bugs to Leaf.

Comment 10

[Axel Hecht]

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

[Darin Fisher]

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.

Comment 12

[Glenn Randers-Pehrson]

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

[Darin Fisher]

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

[Darin Fisher]

Attachment: patch - use #define to fix expat

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

Comment 15

[Heikki Toivonen (remove -bugzilla when emailing directly)]

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.

Comment 16

[Darin Fisher]

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.

Comment 17

[Glenn Randers-Pehrson]

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?

Comment 18

[Christian :Biesinger (don't email me, ping me on IRC)]

libjpeg hasn't changed during the last few years, afaik. do we really need such
a set of defines for it?

Comment 19

[Glenn Randers-Pehrson]

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.

Comment 20

[Benjamin Smedberg]

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 ).

Comment 21

[Christian :Biesinger (don't email me, ping me on IRC)]

the modified libart also causes bug 223646 I believe

Comment 22

[Glenn Randers-Pehrson]

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_*()".

Comment 23

[Glenn Randers-Pehrson]

added dependency on bug #223646 (libart)

Comment 24

[Brian Ryner (not reading)]

Attachment: 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)

Comment 25

[Darin Fisher]

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

r=darin (thanks bryner!)

Comment 26

[Darin Fisher]

/* avoid conflicts wiht system versions of libexpat */

typo on "wiht" ;)

Comment 27

[Brian Ryner (not reading)]

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.

Comment 28

[Asa Dotzler [:asa]]

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

a=asa (on behalf of drivers) for checkin to 1.6beta

Comment 29

[Brian Ryner (not reading)]

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

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

Comment 30

[Chase Phillips]

Mass reassign of open bugs for [e-mail] to [e-mail].

Comment 31

[J. Paul Reed [:preed]]

Mass re-assign of bugs that aren't on the build team radar, so bugs assigned to [e-mail] reflects reality.

If there is a bug you really think we need to be looking at, please *email* [e-mail] with a bug number and explanation.

Comment 32

[Benjamin Smedberg]

All libs I know of use namespaced symbols now.