Closed Bug 273876 Opened 20 years ago Closed 19 years ago

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

Categories

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

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.8beta2

People

(Reporter: benjamin, Assigned: benjamin)

References

Details

Attachments

(2 files)

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.
Attachment #168299 - Flags: review?(darin)
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
Attachment #168299 - Flags: review?(darin) → review+
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.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
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.
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.
Blocks: 274089
I'm going to reopen this to get the -rpath-link magic.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #168508 - Flags: review?(cls)
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?
> 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 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.
Attachment #168508 - Flags: review?(cls) → review-
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.
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.
 
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 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
Attachment #168508 - Flags: review- → review+
All fixed on trunk, finally!
Status: REOPENED → RESOLVED
Closed: 20 years ago19 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.8beta2
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: