Bug 45247

Summary: [EFL] Need to add custom dependencies.
Product: WebKit Reporter: Ryuan Choi <ryuan.choi>
Component: WebKit EFLAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: antognolli+webkit, leandro, lucas.de.marchi, tonikitoo
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Ryuan Choi 2010-09-05 21:28:52 PDT
EFL require default.edj,
but It looks not compiled because there are no dependency.

so, I can't install ewebkit because of missing default.edj.

below is error message.

CMake Error at WebKit/cmake_install.cmake:57 (FILE):
  file INSTALL cannot find
  "/workspace/webkit/build/WebKit/efl/DefaultTheme/default.edj".
Call Stack (most recent call first):
  cmake_install.cmake:39 (INCLUDE)
Comment 1 Ryuan Choi 2010-09-05 21:50:27 PDT
Created attachment 66606 [details]
Patch
Comment 2 Ryuan Choi 2010-09-05 23:02:31 PDT
Created attachment 66612 [details]
Patch
Comment 3 Lucas De Marchi 2010-09-06 23:09:22 PDT
Comment on attachment 66612 [details]
Patch

> Index: WebKit/ChangeLog
> ===================================================================
> --- WebKit/ChangeLog	(revision 66809)
> +++ WebKit/ChangeLog	(working copy)
> @@ -1,3 +1,14 @@
> +2010-09-05  Ryuan Choi  <ryuan.choi@samsung.com>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        [EFL] PORT_DEPENDENCY to add custom target in CMake system
What did you mean?

[EFL] Add theme as dependency to 
> +        https://bugs.webkit.org/show_bug.cgi?id=45247
> +
> +        Add WEBKIT_PORT_DEPENDENCIES to include custom target for each port.
I'd reword to something like:

Add WEBKIT_PORT_DEPENDENCIES to include custom dependencies for each port.


Otherwise, it looks good.
Comment 4 Ryuan Choi 2010-09-07 01:06:38 PDT
Created attachment 66690 [details]
Patch
Comment 5 Ryuan Choi 2010-09-07 01:07:26 PDT
(In reply to comment #3)
> (From update of attachment 66612 [details])
> > Index: WebKit/ChangeLog
> > ===================================================================
> > --- WebKit/ChangeLog	(revision 66809)
> > +++ WebKit/ChangeLog	(working copy)
> > @@ -1,3 +1,14 @@
> > +2010-09-05  Ryuan Choi  <ryuan.choi@samsung.com>
> > +
> > +        Reviewed by NOBODY (OOPS!).
> > +
> > +        [EFL] PORT_DEPENDENCY to add custom target in CMake system
> What did you mean?
> 
> [EFL] Add theme as dependency to 
> > +        https://bugs.webkit.org/show_bug.cgi?id=45247
> > +
> > +        Add WEBKIT_PORT_DEPENDENCIES to include custom target for each port.
> I'd reword to something like:
> 
> Add WEBKIT_PORT_DEPENDENCIES to include custom dependencies for each port.
> 
> 
> Otherwise, it looks good.

thank you.
I created patch like you mentioned.
Comment 6 Lucas De Marchi 2010-09-08 05:10:45 PDT
(In reply to comment #5)
> 
> thank you.
> I created patch like you mentioned.


Humn, sorry. Looking again at this issue, I think a much cleaner approach would be to apply the following patch. It just adds the theme to WebKit's source list.

Leandro, Ryuan, what do you think?


diff --git a/WebKit/efl/CMakeListsEfl.txt b/WebKit/efl/CMakeListsEfl.txt
index cff1822..57cacae 100644
--- a/WebKit/efl/CMakeListsEfl.txt
+++ b/WebKit/efl/CMakeListsEfl.txt
@@ -159,6 +159,10 @@ ADD_CUSTOM_COMMAND(
   VERBATIM
 )
 
+LIST(APPEND WebKit_SOURCES
+     ${WebKit_THEME}
+)
+
 IF (SHARED_CORE)
     SET(LIBS_PRIVATE "-l${WTF_LIBRARY_NAME} -l${JavaScriptCore_LIBRARY_NAME} -l${WebCore_LIBRARY_NAME}")
 ELSE ()
Comment 7 Ryuan Choi 2010-09-08 07:36:16 PDT
Created attachment 66897 [details]
Patch
Comment 8 Ryuan Choi 2010-09-08 07:40:24 PDT
(In reply to comment #7)
> Created an attachment (id=66897) [details]
> Patch

(In reply to comment #6)
> (In reply to comment #5)
> > 
> > thank you.
> > I created patch like you mentioned.
> 
> 
> Humn, sorry. Looking again at this issue, I think a much cleaner approach would be to apply the following patch. It just adds the theme to WebKit's source list.
> 
> Leandro, Ryuan, what do you think?
> 
> 
> diff --git a/WebKit/efl/CMakeListsEfl.txt b/WebKit/efl/CMakeListsEfl.txt
> index cff1822..57cacae 100644
> --- a/WebKit/efl/CMakeListsEfl.txt
> +++ b/WebKit/efl/CMakeListsEfl.txt
> @@ -159,6 +159,10 @@ ADD_CUSTOM_COMMAND(
>    VERBATIM
>  )
> 
> +LIST(APPEND WebKit_SOURCES
> +     ${WebKit_THEME}
> +)
> +
>  IF (SHARED_CORE)
>      SET(LIBS_PRIVATE "-l${WTF_LIBRARY_NAME} -l${JavaScriptCore_LIBRARY_NAME} -l${WebCore_LIBRARY_NAME}")
>  ELSE ()

LGTM,
Conceptually, it's not a source. but it's simple than before.
I tested and upload patch you mentioned.
Comment 9 Ryuan Choi 2010-09-08 07:58:15 PDT
Created attachment 66899 [details]
Patch
Comment 10 Lucas De Marchi 2010-09-08 13:53:02 PDT
Committed r67011: <http://trac.webkit.org/changeset/67011>
Comment 11 Lucas De Marchi 2010-09-08 13:54:27 PDT
Comment on attachment 66899 [details]
Patch

Clearing flags