273876 - libxul step 2 - everything up through widget (except libmozjs.so)

Status: RESOLVED FIXED

(Core Graveyard :: Embedding: GRE Core, defect)

Description

[Benjamin Smedberg]

I'm not going to touch libmozjs.so right now, I haven't thought through the
issues with frozen symbols there yet. But it doesn't matter, because it doesn't
link against xpcom/xpcom_core anyway. This patch combines everything up through
widget.

Comment 1

[Benjamin Smedberg]

Attachment: libxul tier 2, rev 1

Comment 2

[Darin Fisher]

Comment on attachment 168299 [details] [diff] [review]
libxul tier 2, rev 1

>Index: config/config.mk

>+# Force XPCOM/widget/gfx methods to be _declspec(dllexport) when we're
>+# building libxul libraries
>+ifdef MOZ_ENABLE_LIBXUL
>+ifdef LIBXUL_LIBRARY
>+ifeq ($(OS_ARCH),WINNT)

I wouldn't make this specific to WINNT.  Eventually, hidden visibility
stuff will be tied into these macros as well (See brian's patch).  And,
anyways, what about OS/2? ;-)


>Index: ipc/ipcd/extensions/transmngr/build/Makefile.in

what about the other IPC extension libs?  not that it really
matters...


>Index: toolkit/library/nsDllMain.cpp

>+ #include <windows.h>
>+ #include "nsToolkit.h"
>+ 
>+ extern "C" {
>+ extern HINSTANCE _pr_hInstance;
>+ }
>+ 
>+ #if defined(__GNUC__)
>+// If DllMain gets name mangled, it won't be seen.
>+extern "C" {
>+#endif

off-by-one indenting?


>+#endif
>\ No newline at end of file

nit


>+    nsRegionRectIterator i(a);
>+    i.Next();
>+    i.Prev();
>+    i.Reset();

Are you sure it is necessary to call each method?  Once you 
instantiate a class that's usually enough to make sure that
all of its methods are included.


>+}
>\ No newline at end of file

nit


r=darin

Comment 3

[Benjamin Smedberg]

For some reason, it appears necessary to call at least more than one method.
This may have something to do with the fact that this class does not have a
vtable, or apparently any internal callers. Once I incoporate the rest of tier_9
in libxul part 3 we don't have to dllexport the GFX stuff at all, and we can get
rid of that dlldeps file.

Fixed on trunk with other nits picked.

Comment 4

[Darin Fisher]

This seems to have busted the linux xulrunner build in a couple ways:

 gfx/src/psshared           <- needed LIBXUL_LIBRARY=1 (already fixed)
 libpr0n/decoders/icon/gtk  <- needs to be a standalone DSO for gnome integration
 modules/plugin/samples/... <- need to link against libxul (or not??)

I didn't get any further with my trunk build because I ran out of time.

Comment 5

[Benjamin Smedberg]

Fun stuff here ;) I fixed up psshared so that it was in the toolkit/library link
list, and a couple of other linker errors. libxul should now build on linux, but
only if you set LD_LIBRARY_PATH=/path/to/dist/bin.  This is because libxul.so
depends on libmozjs.so, but libmozjs.so is not specified explicitly on the link
line for all the tier 50/99 components and test apps. This can be fixed by bug
274089.

Comment 6

[Benjamin Smedberg]

I'm going to reopen this to get the -rpath-link magic.

Comment 7

[Benjamin Smedberg]

Attachment: Use -rpath-link to pull libmozjs.so needed by libxul.so

Comment 8

[Darin Fisher]

Comment on attachment 168508 [details] [diff] [review]
Use -rpath-link to pull libmozjs.so needed by libxul.so

so, if this is needed, then why is the xulrunner tinderbox green?

Comment 9

[Darin Fisher]

> so, if this is needed, then why is the xulrunner tinderbox green?

nevermind, i should have looked at the tinderbox log files first... i see that
the tinderbox script sets LD_LIBRARY_PATH.

Comment 10

[cls]

Comment on attachment 168508 [details] [diff] [review]
Use -rpath-link to pull libmozjs.so needed by libxul.so

I don't think all linkers support -rpath-link.	Also, we've purposefully
avoided using anything rpath related up to this point.	It is possible (in an
admittedly contrived situation) that using -rpath-link could wind up linking
against the wrong version of mozjs...say libxul & mozjs were installed on the
system but there happens to be mozjs in $(DIST)/bin.  I think you'd be better
off just adding $(MOZ_JS_LIBS) to LIBXUL_LIBS until libmozjs gets incorporated
into libxul.

Comment 11

[Benjamin Smedberg]

Chris, we decided to keep libmozjs.so as a separate sharedlib permanently. I'm
going to need some sort of solution like this soon, for code which links only
against the frozen symbols in libxpcom.so and should not link against
libxpcom_core/libxul.

Comment 12

[cls]

The problem mentioned in comment #5 can be solved by adding MOZ_JS_LIBS to
LIBXUL_LIBS.  I think that's going to be required anyway for linkers which
require all symbols to be resolved at link time, like the AIX & HP-UX linkers. 
Comment #11 sounds like it's addressing a different problem.  I feel like I'm
missing some context here.
 

Comment 13

[Benjamin Smedberg]

I've got two related problems.

1) Gecko code which links against non-frozen symbols in libxul. Adding
MOZ_JS_LIBS to the link line will fix this problem, though I don't like it.

2) Application code which only links against the frozen symbols in libxpcom.so.
This will be the toolkit apps as I can wean them off non-frozen symbols. This
code explicitly must not link against libxul.so or libmozjs.so, even though
libxpcom.so has an ELF dependency on these libs. In this case (AFAICT) either
the -rpath-link flag or munging the environment is necessary to achieve the
desired result.

Comment 14

[Benjamin Smedberg]

Comment on attachment 168508 [details] [diff] [review]
Use -rpath-link to pull libmozjs.so needed by libxul.so

OK, we've decided to do this anyway, and deal with the ports that fail as
necessary. Got r=darin over IRC

Comment 15

[Benjamin Smedberg]

All fixed on trunk, finally!