Mark all graphitle2 functions as local

RESOLVED FIXED

Status

()

Core
Graphics
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Jacek Caban, Assigned: Jacek Caban)

Tracking

Trunk
x86
Windows 7
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
Created attachment 583434 [details] [diff] [review]
fix v1.0

In our case, both graphite2 and the code using it ends up linked in the same library, but its functions are marked as exported/imported. It's not well handled by mingw. Marking them all as GR2_LOCAL has other nice effect on platforms supporting visibility("hidden"). I've made the change in a way that touches upstream code as minimally as possible.
Attachment #583434 - Flags: review?(jfkthame)
AFAICT, this ought to work, but I'd prefer to get upstream to accept a patch that allows us to control this from the makefile rather than applying a local code-level patch.

I just pushed https://hg.mozilla.org/try/rev/67bbb9e7eaa9 to tryserver to see what effect this has (ignore the gfxMacFont debugging chunk accidentally included there!), and will negotiate with upstream about the change provided this works as intended.
(Assignee)

Comment 2

6 years ago
(In reply to Jonathan Kew (:jfkthame) from comment #1)
> AFAICT, this ought to work, but I'd prefer to get upstream to accept a patch
> that allows us to control this from the makefile rather than applying a
> local code-level patch.

If they would accept it, that's indeed better.

> I just pushed https://hg.mozilla.org/try/rev/67bbb9e7eaa9 to tryserver to
> see what effect this has (ignore the gfxMacFont debugging chunk accidentally
> included there!),

It's also a part of my try commit:

https://tbpl.mozilla.org/?tree=Try&rev=905f704e269f

Sorry I didn't mention it earlier.

> and will negotiate with upstream about the change provided this works as intended.

Thanks.
Oops, my tryserver job was pointless as I pushed from an old tree where Graphite was disabled in configure.in. :(

New push at https://hg.mozilla.org/try/rev/bb69fb296160 confirms that this does resolve the dll-visibility warnings. I'll discuss with upstream.
Attachment #583434 - Flags: review?(jfkthame)
I believe the recent Graphite update in bug 721068, which introduced the GRAPHITE2_STATIC option, resolved this issue. Marking as fixed.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Depends on: 721068
Resolution: --- → FIXED
(Assignee)

Comment 5

6 years ago
I can confirm that the bug is fixed. Thanks!
You need to log in before you can comment on or make changes to this bug.