578938 - Shared, non-libxul builds failing due to ANGLE changes

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Mark Banner (:standard8)]

Attachment: Option 1 - export requires symbols

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.

Comment 1

[Mark Banner (:standard8)]

Attachment: Option 2 - static angle library

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.

Comment 2

[Mark Banner (:standard8)]

(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.

Comment 3

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

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.

Comment 5

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: fix

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 6

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

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.

Comment 7

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

http://hg.mozilla.org/mozilla-central/rev/fa2c2d6d3d8f

Comment 8

[Mark Banner (:standard8)]

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.

Comment 9

[Mark Banner (:standard8)]

I will try clobbering those windows boxes though, just in case.

Comment 10

[[e-mail]]

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) ?

Comment 11

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

yeah, !define && !define is better

Comment 12

[Justin Wood (:Callek)]

Attachment: fix static too

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.

Comment 13

[Justin Wood (:Callek)]

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.

Comment 14

[Robert Kaiser]

Pushed as http://hg.mozilla.org/mozilla-central/rev/ee0f5ab96cf2 - thanks to Neil, Callek, and vlad!