712587 - Mark all graphitle2 functions as local

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Jacek Caban]

Attachment: 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.

Comment 1

[Jonathan Kew [: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.

Comment 2

[Jacek Caban]

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

Comment 3

[Jonathan Kew [:jfkthame]]

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.

Comment 4

[Jonathan Kew [:jfkthame]]

I believe the recent Graphite update in bug 721068, which introduced the GRAPHITE2_STATIC option, resolved this issue. Marking as fixed.

Comment 5

[Jacek Caban]

I can confirm that the bug is fixed. Thanks!