Shared, non-libxul builds failing due to ANGLE changes

RESOLVED FIXED

Status

()

Core
Canvas: WebGL
--
blocker
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: standard8, Assigned: vlad)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

8 years ago
Created attachment 457527 [details] [diff] [review]
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.
Attachment #457527 - Flags: review?(vladimir)
(Reporter)

Comment 1

8 years ago
Created attachment 457533 [details] [diff] [review]
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.
Attachment #457533 - Flags: review?(vladimir)
(Reporter)

Comment 2

8 years ago
(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.
(Reporter)

Updated

8 years ago
Blocks: 571172
No longer depends on: 571172
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.
Duplicate of this bug: 579141
Created attachment 457635 [details] [diff] [review]
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.
Assignee: bugzilla → vladimir
Attachment #457527 - Attachment is obsolete: true
Attachment #457533 - Attachment is obsolete: true
Attachment #457635 - Flags: review?(me)
Attachment #457527 - Flags: review?(vladimir)
Attachment #457533 - Flags: review?(vladimir)
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+
http://hg.mozilla.org/mozilla-central/rev/fa2c2d6d3d8f
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(Reporter)

Comment 8

8 years ago
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.
(Reporter)

Comment 9

8 years ago
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
Created attachment 458039 [details] [diff] [review]
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.
Attachment #458039 - Flags: review?(vladimir)
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.
Keywords: checkin-needed

Comment 14

8 years ago
Pushed as http://hg.mozilla.org/mozilla-central/rev/ee0f5ab96cf2 - thanks to Neil, Callek, and vlad!
Keywords: checkin-needed
You need to log in before you can comment on or make changes to this bug.