Bug 182933 - [GTK][CMake] Support building with Enchant 2.x
Summary: [GTK][CMake] Support building with Enchant 2.x
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Adrian Perez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-02-19 11:07 PST by Adrian Perez
Modified: 2018-02-20 10:47 PST (History)
7 users (show)

See Also:


Attachments
Patch (2.10 KB, patch)
2018-02-19 11:40 PST, Adrian Perez
no flags Details | Formatted Diff | Diff
Patch (2.10 KB, patch)
2018-02-19 11:45 PST, Adrian Perez
no flags Details | Formatted Diff | Diff
Patch (2.10 KB, patch)
2018-02-20 09:34 PST, Adrian Perez
no flags Details | Formatted Diff | Diff
Patch for landing (2.24 KB, patch)
2018-02-20 09:58 PST, Adrian Perez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adrian Perez 2018-02-19 11:07:24 PST
Today I noticed that Arch Linux has stopped shipping Enchant 1.x a
while ago, and that for building WebKitGTK+ the following patch is
applied:

  https://git.archlinux.org/svntogit/packages.git/tree/trunk/enchant-2.diff?h=packages/webkit2gtk

The patch is very simple, and it merely changes “FindEnchant.cmake”
so it checks for “enchant-2” instead of “enchant” using pkg-config.
The usage that WebKitGTK+ makes of Enchant does not seem to rely on
any API changed between the Enchant 1.x series and the 2.x ones, so
it should be safe to check whether any of them is available.

On a chat with Michael Catanzaro he pointed out that Enchant 2.x
will still take time to be adopted by all GNU/Linux distributions,
and in the meanwhile we should try to keep the build working fine
with both 1.x and 2.x — and making WebKitGTK+ buildable out of the
box with the newer releases is the first step.
Comment 1 Adrian Perez 2018-02-19 11:40:22 PST
Created attachment 334171 [details]
Patch
Comment 2 Adrian Perez 2018-02-19 11:42:48 PST
Comment on attachment 334171 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=334171&action=review

There's a small typo in the comment, I'll fix it before landing.

> Source/cmake/FindEnchant.cmake:48
> +    # cases in which e.g. both versions of the library are installed, but on

The “on” needs to be deleted in this phrase, it should read “but the
headers are usable/present only for one of them”.
Comment 3 Adrian Perez 2018-02-19 11:45:37 PST
Created attachment 334172 [details]
Patch

Rebased patch with typo in comment fixed
Comment 4 Adrian Perez 2018-02-20 09:32:18 PST
On a chat with Michael Catanzaro we have agreed that it's probably
better to check for Enchant 2.x first, so I'll re-upload the patch
with that change done.
Comment 5 Michael Catanzaro 2018-02-20 09:32:34 PST
Comment on attachment 334172 [details]
Patch

This looks good, just swap the order so enchant-2 is checked first, and please add a FIXME to remove the check for old enchant after a couple of years.
Comment 6 Adrian Perez 2018-02-20 09:34:06 PST
Created attachment 334275 [details]
Patch
Comment 7 Adrian Perez 2018-02-20 09:58:13 PST
Created attachment 334277 [details]
Patch for landing
Comment 8 WebKit Commit Bot 2018-02-20 10:46:14 PST
Comment on attachment 334277 [details]
Patch for landing

Clearing flags on attachment: 334277

Committed r228826: <https://trac.webkit.org/changeset/228826>
Comment 9 WebKit Commit Bot 2018-02-20 10:46:16 PST
All reviewed patches have been landed.  Closing bug.
Comment 10 Radar WebKit Bug Importer 2018-02-20 10:47:22 PST
<rdar://problem/37712970>