Closed Bug 578938 Opened 10 years ago Closed 10 years ago
Shared, non-libxul builds failing due to ANGLE changes
We've got several failures on non-libxul, fully-shared builds. - libangle fails to build because it needs NSPR and MOZALLOC libs in EXTRA_DSO_LDOPTS (easy to fix). - libgklayout fails to build because libangle doesn't export the functions that are required elsewhere. For the latter issue, I think we have two options: 1) Export the required symbols, this requires modification to the imported source of angle. 2) Build angle as a static lib into the gfx library. I've a patch for the first of these, but do we do any modification to the imported code, would modification be acceptable? If not I think the only option would be to make it a static lib into the gfx library, similar to what bug 562659 was suggesting for ycbcr.
Possible option 2 - this makes angle a static library and links it into layout. The only downside is that we're linking something in gfx into layout, however if we don't export the symbols, that's the only option we've got.
(In reply to comment #0) > Created attachment 457527 [details] [diff] [review] > Option 1 - export requires symbols Note: this patch doesn't work on windows due to the fastcall on the ShFinalize function. I'm leaving it up there for now until I get some feedback on which way we want to go.
Hmm.. I like option 1 better, but it would be better if it was something we could upstream. Though I guess we could just keep it as a local patch. Do we even need to patch the cpp file? Seems like just getting the declspec's in the .h file should be enough. Also, that __fastcall can just go, I don't think there's any reason for that to be __fastcall.
Ok, here's a fix; I'll check this in along with a .patch file. This can also really be upstreamed, though it's not essential.
Comment on attachment 457635 [details] [diff] [review] fix >-DEFINES += -DANGLE_USE_NSPR >+DEFINES += -DANGLE_USE_NSPR -DANGLE_BUILD >+ >+ifndef MOZ_ENABLE_LIBXUL >+EXTRA_DSO_LDOPTS = $(LIBS_DIR) $(MOZ_COMPONENT_LIBS) >+endif Is LIBS_DIR necessary here? >+#ifndef MOZ_ENANBLE_LIBXUL MOZ_ENABLE_LIBXUL r=me with that.
Attachment #457635 - Flags: review?(me) → review+
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
We seem to have fixed most of it, but static builds on windows are still busted: http://tinderbox.mozilla.org/showlog.cgi?log=Thunderbird/1279266383.1279269742.25059.gz I've played around with a few things, but not worked out a solution yet.
I will try clobbering those windows boxes though, just in case.
Comment on attachment 457635 [details] [diff] [review] fix >+#ifndef MOZ_ENANBLE_LIBXUL >+#ifdef ANGLE_BUILD >+#define ANGLE_EXPORT __declspec(dllexport) > #else >+#define ANGLE_EXPORT __declspec(dllimport) >+#endif >+#else >+#define ANGLE_EXPORT >+#endif Sadly this is still broken for static Windows builds, as it needs the null #define as per LIBXUL builds. Locally I just tried wrapping an extra #ifndef MOZ_STATIC_BUILD ... #else #define ANGLE_EXPORT #endif but I guess you might prefer if I used !defined(MOZ_ENABLE_LIBXUL) && !define(MOZ_STATIC_BUILD) ?
yeah, !define && !define is better
Vlad, this should fix static too; I marked the patch author as Neil, as it was none of my investigation/testing of this fix though.
Attachment #458039 - Flags: review?(vladimir)
10 years ago
Attachment #458039 - Flags: review?(vladimir) → review+
I won't get around to landing this for a day or two, please push earlier if you can. Remember, this is Neil's patch not my own.
Pushed as http://hg.mozilla.org/mozilla-central/rev/ee0f5ab96cf2 - thanks to Neil, Callek, and vlad!
You need to log in before you can comment on or make changes to this bug.