Bug 116978

Summary: [GTK] Needs to check for harfbuzz-icu
Product: WebKit Reporter: Emilio Pozuelo Monfort <pochu27>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: behdad, berto, commit-queue, mrobinson, william.jon.mccann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
mrobinson: review+
Patch none

Emilio Pozuelo Monfort
Reported 2013-05-29 11:02:17 PDT
Latest harfbuzz has splitted hb_icu into its own library libharfbuzz-icu.so. Therefore webkit needs to check for harfbuzz-icu in addition to harfbuzz to a) make sure harfbuzz-icu is available and b) get the necessary cflags / libs.
Attachments
Patch (1.61 KB, patch)
2013-05-30 03:47 PDT, Alberto Garcia
no flags
Patch (1.59 KB, patch)
2013-05-30 06:17 PDT, Alberto Garcia
mrobinson: review+
Patch (1.60 KB, patch)
2013-05-30 07:04 PDT, Alberto Garcia
no flags
Alberto Garcia
Comment 1 2013-05-30 03:00:12 PDT
I'll take a look.
Alberto Garcia
Comment 2 2013-05-30 03:47:47 PDT
Created attachment 203339 [details] Patch Here's the patch. Emilio, can you double check if it works for you?
Emilio Pozuelo Monfort
Comment 3 2013-05-30 04:19:12 PDT
I don't have harfbuzz 0.9.18 yet so not now. The patch looks good. Something you could do is abort the build if harfbuzz-icu is not available but harfbuzz is >= 0.9.18, with: if pkg-config --atleast-version 0.9.18 harfbuzz; then PKG_CHECK_MODULES(HARFBUZZ_ICU, harfbuzz-icu >= $harfbuzz_required_version) FREETYPE_CFLAGS+=" $HARFBUZZ_ICU_CFLAGS" FREETYPE_LIBS+=" $HARFBUZZ_ICU_LIBS" fi This is useful in case somebody builds harfbuzz but doesn't have libicu installed, as harfbuzz-icu will be automatically disabled and he may not notice.
Alberto Garcia
Comment 4 2013-05-30 06:11:34 PDT
(In reply to comment #3) > The patch looks good. Something you could do is abort the build if > harfbuzz-icu is not available but harfbuzz is >= 0.9.18 Yes, I was also considering doing that. I thought it's actually a better option in case someone builds harfbuzz without icu support. I'll upload a new patch.
Alberto Garcia
Comment 5 2013-05-30 06:17:38 PDT
Emilio Pozuelo Monfort
Comment 6 2013-05-30 06:54:27 PDT
Patch looks good. I should be able to test it tomorrow in case that's still useful by then.
Martin Robinson
Comment 7 2013-05-30 07:00:01 PDT
Comment on attachment 203347 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=203347&action=review > Source/autotools/FindDependencies.m4:379 > +# Since we support earlier HarfBuzz versions we keep this conditional by now. English nit: by now -> for now > Source/autotools/FindDependencies.m4:380 > +if $PKG_CONFIG --atleast-version 0.9.18 harfbuzz ; then Nit: extra space after 'harfbuzz'
Alberto Garcia
Comment 8 2013-05-30 07:04:28 PDT
Created attachment 203352 [details] Patch I fixed the nits that Martin pointed out.
Xan Lopez
Comment 9 2013-05-30 07:27:54 PDT
Comment on attachment 203352 [details] Patch r=me
WebKit Commit Bot
Comment 10 2013-05-30 07:55:35 PDT
Comment on attachment 203352 [details] Patch Clearing flags on attachment: 203352 Committed r150963: <http://trac.webkit.org/changeset/150963>
WebKit Commit Bot
Comment 11 2013-05-30 07:55:37 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.