Bug 111028 - [EFL] Build break with latest EFL libraries
Summary: [EFL] Build break with latest EFL libraries
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryuan Choi
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-27 18:19 PST by Ryuan Choi
Modified: 2013-03-05 16:17 PST (History)
11 users (show)

See Also:


Attachments
Patch (8.02 KB, patch)
2013-02-27 18:32 PST, Ryuan Choi
no flags Details | Formatted Diff | Diff
Patch (8.88 KB, patch)
2013-03-03 16:50 PST, Ryuan Choi
no flags Details | Formatted Diff | Diff
Patch (9.74 KB, patch)
2013-03-05 00:01 PST, Ryuan Choi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryuan Choi 2013-02-27 18:19:05 PST
In latest EFL trunk, include path of ecore sub modules are changed from ecore-1/ to ecore-XXX-1/
Comment 1 Ryuan Choi 2013-02-27 18:32:30 PST
Created attachment 190634 [details]
Patch
Comment 2 Benjamin Poulain 2013-02-28 00:08:07 PST
Just in case: no need for a WebKit2 reviewer here. Someone working on EFL should review the patch.
Comment 3 Gyuyoung Kim 2013-02-28 02:01:48 PST
Ryuan, do you need to build WebKit EFL based on latest EFL ?  Could you let me know which EFL version you want to build ?
Comment 4 Ryuan Choi 2013-02-28 02:48:06 PST
(In reply to comment #3)
> Ryuan, do you need to build WebKit EFL based on latest EFL ?  Could you let me know which EFL version you want to build ?

I used TOT in Efl trunk.

I am setting my new PC.
Comment 5 Raphael Kubo da Costa (:rakuco) 2013-02-28 08:40:04 PST
For the record, this is also covered by bug 109819.

It's probably a good idea to mention this is about the changes in EFL 1.8 instead of trunk so people in the future know when the directories changed.

I did not test the patch myself; if it still works with the current 1.7 series, LGTM.
Comment 6 Dominik Röttsches (drott) 2013-03-01 05:00:49 PST
Comment on attachment 190634 [details]
Patch

As Kubo mentions, this basically equivalent to my previous patch in the other bug. Looks okay to me except IMO one missing line in 
+ find_package(Ecore 1.6 COMPONENTS Evas File Input Imf Imf_Evas Con ${ECORE_ADDITIONAL_COMPONENTS})
OptionsEfl.cmake - Imf_Evas and Con should be added there, if I am not mistaken.
Comment 7 Dominik Röttsches (drott) 2013-03-01 05:01:27 PST
And it should be tested whether it works with 1.7.5 jhbuild and ToT SVN.
Comment 8 Ryuan Choi 2013-03-03 16:10:17 PST
(In reply to comment #5)
> For the record, this is also covered by bug 109819.

Sorry, I didn't know it.
I just searched build break issues.

> 
> It's probably a good idea to mention this is about the changes in EFL 1.8 instead of trunk so people in the future know when the directories changed.

Absolutely agree, I will add some comment in FindEcore.cmake.

(In reply to comment #6)
> (From update of attachment 190634 [details])
> As Kubo mentions, this basically equivalent to my previous patch in the other bug. Looks okay to me except IMO one missing line in 
> + find_package(Ecore 1.6 COMPONENTS Evas File Input Imf Imf_Evas Con ${ECORE_ADDITIONAL_COMPONENTS})
> OptionsEfl.cmake - Imf_Evas and Con should be added there, if I am not mistaken.

Right, Imf_Evas is needed. I will upload patch again.

But Ecore_Con, it is only required by Elementary.h.
I think that it can be an optional.

(In reply to comment #7)
> And it should be tested whether it works with 1.7.5 jhbuild and ToT SVN.

I tested whether it works with jhbuild and ToT SVN.
Comment 9 Ryuan Choi 2013-03-03 16:50:35 PST
Created attachment 191152 [details]
Patch
Comment 10 Laszlo Gombos 2013-03-04 06:47:45 PST
(In reply to comment #8)
> (In reply to comment #5)
> > For the record, this is also covered by bug 109819.

> But Ecore_Con, it is only required by Elementary.h.
> I think that it can be an optional.

Would this mean that that Minibrowser needs Ecore_Con ? Should we add this dependency than to Tools/MiniBrowser/efl/CMakeLists.txt (but not for the entire project) ?
Comment 11 Ryuan Choi 2013-03-05 00:01:03 PST
Created attachment 191418 [details]
Patch
Comment 12 Ryuan Choi 2013-03-05 00:03:50 PST
(In reply to comment #10)
> (In reply to comment #8)
> > (In reply to comment #5)
> > > For the record, this is also covered by bug 109819.
> 
> > But Ecore_Con, it is only required by Elementary.h.
> > I think that it can be an optional.
> 
> Would this mean that that Minibrowser needs Ecore_Con ? Should we add this dependency than to Tools/MiniBrowser/efl/CMakeLists.txt (but not for the entire project) ?

Right, MiniBrowser requires Ecore_Con as mandatory package.

I tried to move Ecore_Con check routine from FindEcore.cmake to FindElementary.cmake so that Ecore_Con dependency is tested only for MiniBrowser.
Comment 13 Dominik Röttsches (drott) 2013-03-05 06:55:09 PST
LGTM - Rob or Dirk, could you review this?
Comment 14 Dirk Pranke 2013-03-05 12:49:51 PST
Comment on attachment 191418 [details]
Patch

rs=me
Comment 15 Ryuan Choi 2013-03-05 16:04:29 PST
Comment on attachment 191418 [details]
Patch

Thank you.
Comment 16 WebKit Review Bot 2013-03-05 16:17:44 PST
Comment on attachment 191418 [details]
Patch

Clearing flags on attachment: 191418

Committed r144842: <http://trac.webkit.org/changeset/144842>
Comment 17 WebKit Review Bot 2013-03-05 16:17:50 PST
All reviewed patches have been landed.  Closing bug.