1306543 - gfxFontconfigFonts.cpp: error:'g_unicode_script_from_iso15924 was not declared in this scope

Status: RESOLVED FIXED

(SeaMonkey :: Build Config, defect)

Description

[Edmund Wong (:ewong)]

just filing this... 

linux64 bustage:

/usr/bin/ccache /builds/slave/rel-c-rel-lnx64-bld/build/gcc/bin/gcc -std=gnu99 -o /builds/slave/rel-c-rel-lnx64-bld/build/objdir/security/nss/lib/freebl/md5.o -c -O2 -gdwarf-2 -fPIC -DLINUX2_1 -m64 -pipe -ffunction-sections -fdata-sections -DLINUX -Dlinux -DHAVE_STRERROR -Wall -DNSS_NO_GCC48 -DXP_UNIX -DSHLIB_SUFFIX=\"so\" -DSHLIB_PREFIX=\"lib\" -DSHLIB_VERSION=\"3\" -DSOFTOKEN_SHLIB_VERSION=\"3\" -DRIJNDAEL_INCLUDE_TABLES -UDEBUG -DNDEBUG -D_REENTRANT -DNSS_NO_INIT_SUPPORT -DUSE_UTIL_DIRECTLY -DNO_NSPR_10_SUPPORT -DSSL_DISABLE_DEPRECATED_CIPHER_SUITE_NAMES -DNO_PKCS11_BYPASS -DNSS_ENABLE_TLS_1_3 -DNSS_USE_64 -DFREEBL_LOWHASH -DNSS_X86_OR_X64 -DNSS_X64 -DNSS_BEVAND_ARCFOUR -DMPI_AMD64 -DMP_ASSEMBLY_MULTIPLY -DNSS_USE_COMBA -DMP_IS_LITTLE_ENDIAN -DUSE_HW_AES -DINTEL_GCM -DMP_API_COMPATIBLE -I/builds/slave/rel-c-rel-lnx64-bld/build/objdir/dist/include/nspr -I/builds/slave/rel-c-rel-lnx64-bld/build/objdir/dist/include/nspr -I/builds/slave/rel-c-rel-lnx64-bld/build/objdir/dist/include/nss -I/builds/slave/rel-c-rel-lnx64-bld/build/objdir/dist/private/nss -Impi -Iecl  md5.c
/builds/slave/rel-c-rel-lnx64-bld/build/gcc/bin/../lib/gcc/x86_64-unknown-linux-gnu/4.8.5/../../../../x86_64-unknown-linux-gnu/bin/as: /lib64/libz.so.1: no version information available (required by /builds/slave/rel-c-rel-lnx64-bld/build/gcc/bin/../lib/gcc/x86_64-unknown-linux-gnu/4.8.5/../../../../x86_64-unknown-linux-gnu/bin/as)
/usr/bin/ccache /builds/slave/rel-c-rel-lnx64-bld/build/gcc/bin/gcc -std=gnu99 -o /builds/slave/rel-c-rel-lnx64-bld/build/objdir/security/nss/lib/freebl/sha512.o -c -O2 -gdwarf-2 -fPIC -DLINUX2_1 -m64 -pipe -ffunction-sections -fdata-sections -DLINUX -Dlinux -DHAVE_STRERROR -Wall -DNSS_NO_GCC48 -DXP_UNIX -DSHLIB_SUFFIX=\"so\" -DSHLIB_PREFIX=\"lib\" -DSHLIB_VERSION=\"3\" -DSOFTOKEN_SHLIB_VERSION=\"3\" -DRIJNDAEL_INCLUDE_TABLES -UDEBUG -DNDEBUG -D_REENTRANT -DNSS_NO_INIT_SUPPORT -DUSE_UTIL_DIRECTLY -DNO_NSPR_10_SUPPORT -DSSL_DISABLE_DEPRECATED_CIPHER_SUITE_NAMES -DNO_PKCS11_BYPASS -DNSS_ENABLE_TLS_1_3 -DNSS_USE_64 -DFREEBL_LOWHASH -DNSS_X86_OR_X64 -DNSS_X64 -DNSS_BEVAND_ARCFOUR -DMPI_AMD64 -DMP_ASSEMBLY_MULTIPLY -DNSS_USE_COMBA -DMP_IS_LITTLE_ENDIAN -DUSE_HW_AES -DINTEL_GCM -DMP_API_COMPATIBLE -I/builds/slave/rel-c-rel-lnx64-bld/build/objdir/dist/include/nspr -I/builds/slave/rel-c-rel-lnx64-bld/build/objdir/dist/include/nspr -I/builds/slave/rel-c-rel-lnx64-bld/build/objdir/dist/include/nss -I/builds/slave/rel-c-rel-lnx64-bld/build/objdir/dist/private/nss -Impi -Iecl  sha512.c
/builds/slave/rel-c-rel-lnx64-bld/build/gcc/bin/../lib/gcc/x86_64-unknown-linux-gnu/4.8.5/../../../../x86_64-unknown-linux-gnu/bin/as: /lib64/libz.so.1: no version information available (required by /builds/slave/rel-c-rel-lnx64-bld/build/gcc/bin/../lib/gcc/x86_64-unknown-linux-gnu/4.8.5/../../../../x86_64-unknown-linux-gnu/bin/as)
/builds/slave/rel-c-rel-lnx64-bld/build/mozilla/gfx/thebes/gfxFontconfigFonts.cpp: In member function 'virtual already_AddRefed<gfxFont> gfxPangoFontGroup::FindFontForChar(uint32_t, uint32_t, uint32_t, gfxFontGroup::Script, gfxFont*, uint8_t*)':
/builds/slave/rel-c-rel-lnx64-bld/build/mozilla/gfx/thebes/gfxFontconfigFonts.cpp:1634:66: error: 'g_unicode_script_from_iso15924' was not declared in this scope
       (const PangoScript)g_unicode_script_from_iso15924(scriptTag);
                                                                  ^
make[4]: *** [gfxFontconfigFonts.o] Error 1

Comment 1

[Edmund Wong (:ewong)]

Attachment: gfxFontconfigFonts.i

Comment 2

[Mike Hommey [:glandium]]

Bug 724538 silently broke building against glib < 2.30. Configure still claims compatibility with 2.22. This breaks building against gtk2 in our buildbot Centos 6 environment (only has 2.28).

Comment 3

[Mike Hommey [:glandium]]

This will also prevent Debian LTS update to ESR52. Can we have an alternative code path for glib < 2.30?

Comment 4

[Mike Hommey [:glandium]]

Aha! There's a hb_glib_script_from_script function that wraps g_unicode_script_from_iso15924 with a fallback for glib < 2.29.14.

Comment 5

[Mike Hommey [:glandium]]

Attachment: Avoid using g_unicode_script_from_iso15924 directly

for compatibility with glib < 2.29.14.

Comment 6

[Jonathan Kew [:jfkthame]]

Comment on attachment 8796439 [details] [diff] [review]
Avoid using g_unicode_script_from_iso15924 directly

Review of attachment 8796439 [details] [diff] [review]:
-----------------------------------------------------------------

Yeah, seems like that should be fine.

Comment 7

[Mike Hommey [:glandium]]

Attachment: Avoid using g_unicode_script_from_iso15924 directly

Comment 8

[Mike Hommey [:glandium]]

Attachment: Avoid using g_unicode_script_from_iso15924 directly

With a typo fix.

Comment 9

[Jonathan Kew [:jfkthame]]

Comment on attachment 8796486 [details] [diff] [review]
Avoid using g_unicode_script_from_iso15924 directly

Review of attachment 8796486 [details] [diff] [review]:
-----------------------------------------------------------------

Ah, so there was an additional twist to this. OK, assuming Behdad is likely to accept the upstream PR.... if he doesn't, we'll end up overwriting the hb-glib patch here next time we take an update, and the problem will resurface.

Comment 10

[Edmund Wong (:ewong)]

Attachment: fixed up patch for m-r relbranch

Comment 11

[Edmund Wong (:ewong)]

fwiw, :glandium's patch didn't apply cleanly (trunk was using 1.30 harfbuzz.. 
release seems to be using 1.26.)

thanks glandium!

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/63a5218537c6
Avoid using g_unicode_script_from_iso15924 directly. r=jfkthame

Comment 13

[Edmund Wong (:ewong)]

Attachment: updated fix patch

Comment 14

[Edmund Wong (:ewong)]

of course, the previous push was bad coz I c&p'd the partial patch. Missed
the last part.  It wasn't glandium's patch. Just my bad.. fwiw.

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/63a5218537c6

Comment 16

[Ian Stakenvicius]

This breaks the ability to use a system harfbuzz, doesn't it?

Comment 17

[Mike Hommey [:glandium]]

(In reply to Ian Stakenvicius from comment #16)
> This breaks the ability to use a system harfbuzz, doesn't it?

No, as long as you use a version that has hb-glib.h.

Comment 18

[Ian Stakenvicius]

Ok, well, this is what I get attempting to build seamonkey-2.46_build6's source:

    INPUT("../../gfx/skia/Unified_cpp_gfx_skia3.o")
    INPUT("../../gfx/skia/Unified_cpp_gfx_skia4.o")
    INPUT("../../gfx/skia/Unified_cpp_gfx_skia5.o")
    INPUT("../../gfx/skia/Unified_cpp_gfx_skia6.o")
    INPUT("../../gfx/skia/Unified_cpp_gfx_skia7.o")
    INPUT("../../gfx/skia/Unified_cpp_gfx_skia8.o")
    INPUT("../../gfx/skia/Unified_cpp_gfx_skia9.o")
    INPUT("StaticXULComponentsEnd/StaticXULComponentsEnd.o")

/usr/lib/gcc/x86_64-pc-linux-gnu/4.9.3/bin/ld: warning: hidden symbol 'hb_glib_script_from_script' in /usr/lib64/libharfbuzz.so is referenced by DSO /usr/lib64/libpangoft2-1.0.so
/usr/lib/gcc/x86_64-pc-linux-gnu/4.9.3/bin/ld: error: /buildspace/seamonkey-2.46/seamonk/toolkit/library/../../gfx/thebes/gfxFontconfigFonts.o: requires dynamic R_X86_64_PC32 reloc against 'hb_glib_script_from_script' which may overflow at runtime; recompile with -fPIC
/usr/lib/gcc/x86_64-pc-linux-gnu/4.9.3/bin/ld: error: read-only segment has dynamic relocations
/usr/lib/gcc/x86_64-pc-linux-gnu/4.9.3/bin/ld: error: hidden symbol 'hb_glib_script_from_script' is not defined locally
collect2: error: ld returned 1 exit status


...I'm still working on diagnosing this of course, but it would seem that attempting to dynamically link to harfbuzz rather than linking to it statically the way it does when bundled is causing an issue.  (I have confirmed that my libharfbuzz.so contains hb_glib_script_from_script and that this symbol isn't hidden or otherwise restricted in the lib)

Comment 19

[Mike Hommey [:glandium]]

It's funny it only complains about that symbol, because there is hb-*.h header in config/system-headers.

Comment 20

[Mike Hommey [:glandium]]

(In reply to Mike Hommey [:glandium] from comment #19)
> It's funny it only complains about that symbol, because there is hb-*.h
> header in config/system-headers.

there is *no* hb-*.h header.

Comment 21

[Mike Hommey [:glandium]]

(also note that building against system harfbuzz is actually not supported out of the box)

Comment 22

[Ian Stakenvicius]

(In reply to Mike Hommey [:glandium] from comment #21)
> (also note that building against system harfbuzz is actually not supported
> out of the box)

True, but bug 847568 is coming along and I figured it would be relevant to point out the incompatibility.

Thank you very much though for the pointer -- indeed, the patch from said bug didn't add hb-glib.h to config/system-headers, adding that addresses the issue.  I'll update the other bug.

Comment 23

[Rainer Bielefeld]

Target Milestone: Please see Bug 1293618 comment #38

Comment 24

[Edmund Wong (:ewong)]

This needs to be transplanted to m-b's SEA248b1_2017021701_RELBRANCH and also to
whatever is Gecko 51's m-r.

Comment 25

[Edmund Wong (:ewong)]

Comment on attachment 8796486 [details] [diff] [review]
Avoid using g_unicode_script_from_iso15924 directly

Approval Request Comment
[Feature/Bug causing the regression]: 
[User impact if declined]: Cannot build
[Is this code covered by automated tests?]:
[Has the fix been verified in Nightly?]: Yes (via. https://hg.mozilla.org/mozilla-central/rev/63a5218537c6)
[Needs manual test from QE? If yes, steps to reproduce]:  None
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]:
[String changes made/needed]: None

This needs to be transplanted to the Gecko 51's Mozilla-Release (likely
at https://hg.mozilla.org/releases/mozilla-release/rev/FIREFOX_RELEASE_51_END)
and on a RELBRANCH (for SeaMonkey 2.48 release, which is based on Gecko 51).

Comment 26

[Edmund Wong (:ewong)]

Comment on attachment 8796486 [details] [diff] [review]
Avoid using g_unicode_script_from_iso15924 directly

Approval Request Comment
[Feature/Bug causing the regression]:
[User impact if declined]: cannot build SeaMonkey w/ GTK2
[Is this code covered by automated tests?]:
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: 
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]:
[String changes made/needed]: None

This needs to be pushed to m-b's SEA248b1_2017021701_RELBRANCH.

Comment 27

[Mike Hommey [:glandium]]

There is nothing to transplant on beta. It landed in the 52 cycle. As for m-r, I doubt it's open for 51 anymore, since 52 releases in 2~3 days.

Comment 28

[Edmund Wong (:ewong)]

(In reply to Mike Hommey [:glandium] from comment #27)
> There is nothing to transplant on beta. It landed in the 52 cycle. As for
> m-r, I doubt it's open for 51 anymore, since 52 releases in 2~3 days.

Sorry, I meant the Gecko 51 m-b, specifically the 51 m-b's 
SEA248b1_2017021701_RELBRANCH and 51 m-r.

Comment 29

[Julien Cristau [:jcristau] (back April 22)]

Comment on attachment 8796486 [details] [diff] [review]
Avoid using g_unicode_script_from_iso15924 directly

If this is only for a seamonkey relbranch it can probably go in without relman approval.