Bug 116978 – [GTK] Needs to check for harfbuzz-icu

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Emilio Pozuelo Monfort]

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.

[Alberto Garcia]

I'll take a look.

[Alberto Garcia]

Created attachment 203339 [details]
Patch

Here's the patch. Emilio, can you double check if it works for you?

[Emilio Pozuelo Monfort]

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]

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

Created attachment 203347 [details]
Patch

[Emilio Pozuelo Monfort]

Patch looks good. I should be able to test it tomorrow in case that's still useful by then.

[Martin Robinson]

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]

Created attachment 203352 [details]
Patch

I fixed the nits that Martin pointed out.

[Xan Lopez]

Comment on attachment 203352 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 203352 [details]
Patch

Clearing flags on attachment: 203352

Committed r150963: <http://trac.webkit.org/changeset/150963>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.