Bug 130150 – [EFL][GTK] Get CMake to find Freetype2 properly

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Thiago de Barros Lacerda]

Newer versions of CMake are not able to find Freetype2 correctly.

[Thiago de Barros Lacerda]

Created attachment 226542 [details]
Patch

[Martin Robinson]

*** Bug 130107 has been marked as a duplicate of this bug. ***

[Martin Robinson]

Comment on attachment 226542 [details]
Patch

It should be possible to properly use the version information with pkgconfig. See FindGeoclue.cmake for how we have done this in WebKitGTK+.

[Thiago de Barros Lacerda]

Created attachment 227113 [details]
Patch

[Martin Robinson]

Comment on attachment 227113 [details]
Patch

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

> Source/cmake/FindFreetype2.cmake:2
> +# Copyright (C) 2013 Nokia Corporation and/or its subsidiary(-ies).
> +#

For the purposes of GTK+, it is just sufficient to look at the pkgconfig file. I don't know about EFL.

> Source/cmake/OptionsGTK.cmake:173
> -find_package(Freetype 2.4.2 REQUIRED)
> +find_package(Freetype2 2.4.11 REQUIRED)

I'm not sure I understand this change. You are raising the freetype dependency requirements. We cannot do that for GTK+ at least. My understanding of the situation is that the FreeType pkgconfig file lists the libtool version triple in it instead of the actual version. So here we need to figure out what the version listed in the FreeType 2.4.11 release is and then put that version here. It would also be good to put a comment explaining the situation.

[Thiago de Barros Lacerda]

Comment on attachment 227113 [details]
Patch

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

>> Source/cmake/FindFreetype2.cmake:2
>> +#
> 
> For the purposes of GTK+, it is just sufficient to look at the pkgconfig file. I don't know about EFL.

Huum... maybe you commented in the wrong line?

>> Source/cmake/OptionsGTK.cmake:173
>> +find_package(Freetype2 2.4.11 REQUIRED)
> 
> I'm not sure I understand this change. You are raising the freetype dependency requirements. We cannot do that for GTK+ at least. My understanding of the situation is that the FreeType pkgconfig file lists the libtool version triple in it instead of the actual version. So here we need to figure out what the version listed in the FreeType 2.4.11 release is and then put that version here. It would also be good to put a comment explaining the situation.

2.4.11 is the version that GTK (and also EFL) is using in jhbuild.modules. Looking at the original FindFreetype.cmake (usually in /usr/share/cmake<version>/Modules) I noticed that it searches for the version in the freetype.h header, which is the same of the package in jhbuild.modules, not that number that is in freetype2.pc file.
So we can use the version that is present in jhbuild.modules

[Martin Robinson]

(In reply to comment #6)

> 2.4.11 is the version that GTK (and also EFL) is using in jhbuild.modules. Looking at the original FindFreetype.cmake (usually in /usr/share/cmake<version>/Modules) I noticed that it searches for the version in the freetype.h header, which is the same of the package in jhbuild.modules, not that number that is in freetype2.pc file.
> So we can use the version that is present in jhbuild.modules


Okay. That seems better. In this case we don't need to raise the dependency at all.

[Gyuyoung Kim]

Comment on attachment 227113 [details]
Patch

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

LGTM on EFL port side.

>>> Source/cmake/FindFreetype2.cmake:2
>>> +#
>> 
>> For the purposes of GTK+, it is just sufficient to look at the pkgconfig file. I don't know about EFL.
> 
> Huum... maybe you commented in the wrong line?

I'm not sure I understand what Martin said. It seems to me that Martin think we don't need to keep FindFreetype2.cmake. But, I think Changelog explains why FindFreetype2.cmake is needed, isn't it ?

[Gyuyoung Kim]

Comment on attachment 227113 [details]
Patch

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

LGTM on EFL port side.

>>>> Source/cmake/FindFreetype2.cmake:2
>>>> +#
>>> 
>>> For the purposes of GTK+, it is just sufficient to look at the pkgconfig file. I don't know about EFL.
>> 
>> Huum... maybe you commented in the wrong line?
> 
> I'm not sure I understand what Martin said. It seems to me that Martin think we don't need to keep FindFreetype2.cmake. But, I think Changelog explains why FindFreetype2.cmake is needed, isn't it ?

I'm not sure I understand what Martin said. It seems to me that Martin think we don't need to keep FindFreetype2.cmake. But, I think Changelog explains why FindFreetype2.cmake is needed, isn't it ?

[Gyuyoung Kim]

Comment on attachment 227113 [details]
Patch

LGTM on EFL port side.

[Gyuyoung Kim]

(In reply to comment #10)
> (From update of attachment 227113 [details])
> LGTM on EFL port side.

Oops, sorry for duplicated comments. My internet connection was weird for a while. :(

[Thiago de Barros Lacerda]

(In reply to comment #11)
> (In reply to comment #10)
> > (From update of attachment 227113 [details] [details])
> > LGTM on EFL port side.
> 
> Oops, sorry for duplicated comments. My internet connection was weird for a while. :(

Looks like it is OK for both GTK and EFL. Who is going to r+? :)

[Csaba Osztrogonác]

Comment on attachment 227113 [details]
Patch

r=me based on previous comments.

[WebKit Commit Bot]

Comment on attachment 227113 [details]
Patch

Clearing flags on attachment: 227113

Committed r165962: <http://trac.webkit.org/changeset/165962>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Martin Robinson]

This raised the Freetype2 dependency for GTK+ which is against our port policy. Why did it increase the version requirement?

[Csaba Osztrogonác]

(In reply to comment #16)
> This raised the Freetype2 dependency for GTK+ which is against our port policy. Why did it increase the version requirement?

It didn' raise, GTK already uses 2.4.11 via jhbuild.modules
as you agreed previously in comment #7 .

[Alberto Garcia]

The build is broken here since this patch:

CMake Error at /usr/share/cmake-2.8/Modules/FindPackageHandleStandardArgs.cmake:108 (message):
  Could NOT find Freetype2 (missing: FREETYPE2_INCLUDE_DIRS) (Required is at
  least version "2.4.11")

[Alberto Garcia]

The problem seems to be this one:

find_path(FREETYPE2_ROOT_INCLUDE_DIR
    NAMES freetype/freetype.h
    HINTS ${PC_FREETYPE2_INCLUDE_DIRS}
          ${PC_FREETYPE2_INCLUDEDIR}
)


There's no freetype/freetype.h since FreeType 2.5.1

See bug 125074

[Martin Robinson]

(In reply to comment #17)

> It didn' raise, GTK already uses 2.4.11 via jhbuild.modules
> as you agreed previously in comment #7 .

This patch causes everyone compiling WebKitGTK+ to have a newer version of FreeType. The version of FreeType in the JHBuild module is the one that developers and the bots use for testing.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 130512

[Martin Robinson]

(In reply to comment #17)
> (In reply to comment #16)
> > This raised the Freetype2 dependency for GTK+ which is against our port policy. Why did it increase the version requirement?
> 
> It didn' raise, GTK already uses 2.4.11 via jhbuild.modules
> as you agreed previously in comment #7 .

Ah, apologies about comment 7. I think we managed to talk past on another there. My conclusion was that we didn't need to raise the dependency.

[Martin Robinson]

For the purposes of EFL and GTK+ can't we just accept the pkgconfig results? We are doing this for other packages. The only thing we need to do is to translate the versions to ones that are in the pkgconfig files.

[Csaba Osztrogonác]

Sorry, I misundertood it, I thought you agreead about it. And I thought 
the version in jhbuild is the minimum required version. Good to know 
if it isn't. I rolled it out to unbreak the build.

[Thiago de Barros Lacerda]

Created attachment 227323 [details]
Fix for Fretype version>=2.5.1

[Thiago de Barros Lacerda]

As Ossy said, I did not raised the version of freetype, I just used the one that is present on jhbuild.
Anyway, I uploaded a new patch that is supposed to work with other versions. Can you guys check?

[Martin Robinson]

Comment on attachment 227323 [details]
Fix for Fretype version>=2.5.1

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

> Source/cmake/OptionsGTK.cmake:173
> -find_package(Freetype 2.4.2 REQUIRED)
> +find_package(Freetype2 2.4.11 REQUIRED)

This is the line where you raise the freetype dependency.

[Thiago de Barros Lacerda]

(In reply to comment #27)
> (From update of attachment 227323 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=227323&action=review
> 
> > Source/cmake/OptionsGTK.cmake:173
> > -find_package(Freetype 2.4.2 REQUIRED)
> > +find_package(Freetype2 2.4.11 REQUIRED)
> 
> This is the line where you raise the freetype dependency.

OK, I will put it back to 2.4.2. Is that OK?

[Martin Robinson]

(In reply to comment #28)
> (In reply to comment #27)
> > (From update of attachment 227323 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=227323&action=review
> > 
> > > Source/cmake/OptionsGTK.cmake:173
> > > -find_package(Freetype 2.4.2 REQUIRED)
> > > +find_package(Freetype2 2.4.11 REQUIRED)
> > 
> > This is the line where you raise the freetype dependency.
> 
> OK, I will put it back to 2.4.2. Is that OK?

Thanks. :)

[Thiago de Barros Lacerda]

Created attachment 227325 [details]
Fix for Fretype version>=2.5.1 and putting version back to 2.4.2

[Thiago de Barros Lacerda]

Martin, Gyuyoung and Alberto, is that OK now?
If so, Martin or Gyuyoung, could you r+?
Thanks

[WebKit Commit Bot]

Comment on attachment 227325 [details]
Fix for Fretype version>=2.5.1 and putting version back to 2.4.2

Clearing flags on attachment: 227325

Committed r166037: <http://trac.webkit.org/changeset/166037>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.