Bug 124555 – [EFL] Use Config mode of find_package for EFL 1.8

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Ryuan Choi]

EFL 1.8 changed VERSION macro so it's difficult to use tricky approach which parses header file to know the VERSION.
Instead, EFL 1.8 supports Config mode of find_package using XXXConfig.cmake such as EinaConfig.cmake

So, we can use Config mode of find_package when we found Eo (which is introduced since EFL 1.8).

[Ryuan Choi]

Created attachment 217263 [details]
Patch

[Ryuan Choi]

Created attachment 217268 [details]
Patch

[Gyuyoung Kim]

Comment on attachment 217268 [details]
Patch

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

Looks fine except for some English nits.

> Source/cmake/OptionsEfl.cmake:130
> +    # EFL 1.8 provide XXXConfig.cmake

- provide -> provides ? 
- XXXConfig -> FooConfig.

> Tools/ChangeLog:8
> +        * MiniBrowser/efl/CMakeLists.txt: Introduced Config mode to find elementary.

Config mode -> a config mode ?

> ChangeLog:12
> +        This patch try to use Config mode if it is available after checked Eo.

try -> tries ? 
after checked -> after checking ?

[Ryuan Choi]

Created attachment 217285 [details]
Patch

[Ryuan Choi]

(In reply to comment #3)
> (From update of attachment 217268 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=217268&action=review
> 
> Looks fine except for some English nits.
> 
> > Source/cmake/OptionsEfl.cmake:130
> > +    # EFL 1.8 provide XXXConfig.cmake
> 
> - provide -> provides ? 
> - XXXConfig -> FooConfig.
> 
> > Tools/ChangeLog:8
> > +        * MiniBrowser/efl/CMakeLists.txt: Introduced Config mode to find elementary.
> 
> Config mode -> a config mode ?
> 
> > ChangeLog:12
> > +        This patch try to use Config mode if it is available after checked Eo.
> 
> try -> tries ? 
> after checked -> after checking ?

Thanks, I fixed.

[WebKit Commit Bot]

Comment on attachment 217285 [details]
Patch

Clearing flags on attachment: 217285

Committed r159496: <http://trac.webkit.org/changeset/159496>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Seokju Kwon]

It seems like 'CONFIG' option of find_package() is supported on cmake 3.8.8 or later.
But the minimum required version of cmake is 2.8.3.

Error : 
CMake Error at Source/cmake/OptionsEfl.cmake:126 (find_package):
  find_package called with invalid argument "CONFIG"

[WebKit Commit Bot]

Re-opened since this is blocked by bug 124641

[Csaba Osztrogonác]

Ubuntu 12.04 (the latest LTS) is shipped with cmake 2.8.7,
I don't think if we should drop supporting this release.

[Gyuyoung Kim]

(In reply to comment #10)
> Ubuntu 12.04 (the latest LTS) is shipped with cmake 2.8.7,
> I don't think if we should drop supporting this release.

Ryuan will find a better way to support this mode.

[Ryuan Choi]

Created attachment 217495 [details]
Patch

[Ryuan Choi]

(In reply to comment #8)
> It seems like 'CONFIG' option of find_package() is supported on cmake 3.8.8 or later.
> But the minimum required version of cmake is 2.8.3.
> 
> Error : 
> CMake Error at Source/cmake/OptionsEfl.cmake:126 (find_package):
>   find_package called with invalid argument "CONFIG"

Seokju, could you check with newer patch.
I believe that it will be solved.

(In reply to comment #11)
> (In reply to comment #10)
> > Ubuntu 12.04 (the latest LTS) is shipped with cmake 2.8.7,
> > I don't think if we should drop supporting this release.
> 
> Ryuan will find a better way to support this mode.

Sure, I don't want to drop them.

Instead, I changed little bit.

Eo will not be checked with config mode by removing legacy FindEo.cmake.
And added QUIET not to print warning.
To support EFL 1.8 with cmake lower than 2.8.8, I supposed to use legacy FindFoo.cmake without version requirement because Eo will guarantee the version.

[Seokju Kwon]

LGTM with EFL 1.7
Thanks.

[Gyuyoung Kim]

Comment on attachment 217495 [details]
Patch

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

Looks fine.

> Source/cmake/OptionsEfl.cmake:136
> +    if (NOT (CMAKE_VERSION VERSION_LESS 2.8.8))

Isn't there CMAKE_VERSION_VERSION_MORE or similar thing ? It looks this is double *not not* condition. If there is no similar thing, this is fine.

[Ryuan Choi]

(In reply to comment #15)
> (From update of attachment 217495 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=217495&action=review
> 
> Looks fine.
> 
> > Source/cmake/OptionsEfl.cmake:136
> > +    if (NOT (CMAKE_VERSION VERSION_LESS 2.8.8))
> 
> Isn't there CMAKE_VERSION_VERSION_MORE or similar thing ? It looks this is double *not not* condition. If there is no similar thing, this is fine.

Unfortunately cmake only have VERSION_LESS, VERSION_EQUAL, VERSION_GREATER.
So, below is another option but also complicated.
if ((CMAKE_VERSION VERSION_GREATER 2.8.8) OR (CMAKE_VERSION VERSION_EQUAL 2.8.8))

http://stackoverflow.com/questions/16667017/cmake-express-the-greater-or-equal-statement

[WebKit Commit Bot]

Comment on attachment 217495 [details]
Patch

Clearing flags on attachment: 217495

Committed r159776: <http://trac.webkit.org/changeset/159776>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.