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)
Core Graveyard
Embedding: GRE Core
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla1.8beta2
People
(Reporter: benjamin, Assigned: benjamin)
References
Details
Attachments
(2 files)
80.83 KB,
patch
|
darin.moz
:
review+
|
Details | Diff | Splinter Review |
667 bytes,
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•20 years ago
|
||
Assignee | ||
Updated•20 years ago
|
Attachment #168299 -
Flags: review?(darin)
Comment 2•20 years ago
|
||
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+
Assignee | ||
Comment 3•20 years ago
|
||
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
Comment 4•20 years ago
|
||
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.
Assignee | ||
Comment 5•20 years ago
|
||
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
Assignee | ||
Comment 6•20 years ago
|
||
I'm going to reopen this to get the -rpath-link magic.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 7•20 years ago
|
||
Assignee | ||
Updated•20 years ago
|
Attachment #168508 -
Flags: review?(cls)
Comment 8•20 years ago
|
||
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•20 years ago
|
||
> 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•20 years ago
|
||
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-
Assignee | ||
Comment 11•20 years ago
|
||
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•20 years ago
|
||
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.
Assignee | ||
Comment 13•20 years ago
|
||
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.
Assignee | ||
Comment 14•19 years ago
|
||
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+
Assignee | ||
Comment 15•19 years ago
|
||
All fixed on trunk, finally!
Status: REOPENED → RESOLVED
Closed: 20 years ago → 19 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.8beta2
Updated•8 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•