47.16 KB, patch
|Details | Diff | Splinter Review|
3.29 KB, patch
Heikki Toivonen (remove -bugzilla when emailing directly): superreview+
|Details | Diff | Splinter Review|
2.14 KB, patch
Darin Fisher: review+
|Details | Diff | Splinter Review|
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
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.
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
*** Bug 213605 has been marked as a duplicate of this bug. ***
Created attachment 128742 [details] [diff] [review] New namespace for expat library (needed on IRIX with GTK+ 2.x) Patch from #213605.
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.
Mass reassign of Build/Config bugs to Leaf.
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.
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.
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.
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.
Created attachment 134198 [details] [diff] [review] patch - use #define to fix expat this is a very simple patch that does the job for expat.
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.
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_*()".
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)
Comment on attachment 136818 [details] [diff] [review] fix some more things in expat r=darin (thanks bryner!)
/* 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.
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 on attachment 136818 [details] [diff] [review] fix some more things in expat checked in. leaving bug open for other dependencies (like libart).
Mass re-assign of bugs that aren't on the build team radar, so bugs assigned to email@example.com reflects reality. If there is a bug you really think we need to be looking at, please *email* firstname.lastname@example.org with a bug number and explanation.
All libs I know of use namespaced symbols now.