WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
72685
[CMake] Move the top-level logic to the top-level directory.
https://bugs.webkit.org/show_bug.cgi?id=72685
Summary
[CMake] Move the top-level logic to the top-level directory.
Raphael Kubo da Costa (:rakuco)
Reported
2011-11-17 18:35:56 PST
[CMake] Move the top-level logic to the top-level directory.
Attachments
Patch
(25.16 KB, patch)
2011-11-17 18:51 PST
,
Raphael Kubo da Costa (:rakuco)
no flags
Details
Formatted Diff
Diff
Same patch, fix CMAKE_SOURCE_DIR in WebKit2 too
(26.29 KB, patch)
2011-11-17 19:33 PST
,
Raphael Kubo da Costa (:rakuco)
no flags
Details
Formatted Diff
Diff
Follow Patrick's suggestions
(26.08 KB, patch)
2011-11-18 05:35 PST
,
Raphael Kubo da Costa (:rakuco)
no flags
Details
Formatted Diff
Diff
Rebase on top of Patrick's recent CMake commits
(26.11 KB, patch)
2011-11-18 10:18 PST
,
Raphael Kubo da Costa (:rakuco)
no flags
Details
Formatted Diff
Diff
Mark bfulgham as reviewer, rebase and adjust the blackberry files
(28.72 KB, patch)
2011-11-22 05:30 PST
,
Raphael Kubo da Costa (:rakuco)
no flags
Details
Formatted Diff
Diff
Show Obsolete
(4)
View All
Add attachment
proposed patch, testcase, etc.
Raphael Kubo da Costa (:rakuco)
Comment 1
2011-11-17 18:51:01 PST
Created
attachment 115724
[details]
Patch
Raphael Kubo da Costa (:rakuco)
Comment 2
2011-11-17 18:57:41 PST
Rationale: This is related to
bug 72684
-- the build started to break on my ArchLinux box after Valgrind was upgraded to 3.7.0, andthis version started shipping /usr/include/valgrind/config.h.
Bug 72684
fixes the actual include order in EFL's DumpRenderTree, however Tools/CMakeListsEfl.txt has all the code to build EWebLauncher (including adding the required includes) and then uses INCLUDE() to add Tools/DumpRenderTree/efl/CMakeLists.txt, so the external include paths from EWebLauncher will always come before the ones added by DRT and the build will still break. Since I was already at this, I found it would be better to fix it in a more general way.
Raphael Kubo da Costa (:rakuco)
Comment 3
2011-11-17 19:33:31 PST
Created
attachment 115726
[details]
Same patch, fix CMAKE_SOURCE_DIR in WebKit2 too
Patrick R. Gansterer
Comment 4
2011-11-18 00:20:38 PST
Comment on
attachment 115726
[details]
Same patch, fix CMAKE_SOURCE_DIR in WebKit2 too View in context:
https://bugs.webkit.org/attachment.cgi?id=115726&action=review
> Tools/CMakeLists.txt:1 > +INCLUDE_IF_EXISTS(${TOOLS_DIR}/CMakeLists${PORT}.txt)
What about changing this to a simple IF($PORT EQUALS EFL) ADD_SUBDIRECTORY(...) ELSEIF ($PORT EQUALS WinCE) ADD_SUBDIRECTORY ENDIF () IMHO that will be simpler, uses only CMakeLists.txt (without port) and does not pollute the Tools dir with a many 1-3 line files.
> Tools/DumpRenderTree/efl/CMakeLists.txt:126 > +SET_TARGET_PROPERTIES(Programs/DumpRenderTree PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}")
i don't see how this change is related to the bug title
> Tools/DumpRenderTree/efl/CMakeLists.txt:131 > +SET_TARGET_PROPERTIES(Programs/ImageDiff PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}")
ditto
> Tools/EWebLauncher/CMakeLists.txt:35 > + LIST(APPEND EWebLauncher_LIBRARIES
please use 4 space ident
> Tools/EWebLauncher/CMakeLists.txt:44 > + LIST(APPEND EWebLauncher_LIBRARIES ${LIBSOUP24_LIBRARIES}) > + LIST(APPEND EWebLauncher_LINK_FLAGS ${LIBSOUP24_LDFLAGS})
4 space ident
> Tools/EWebLauncher/CMakeLists.txt:49 > + LIST(APPEND EWebLauncher_LIBRARIES ${CURL_LIBRARIES}) > + LIST(APPEND EWebLauncher_LINK_FLAGS ${CURL_LDFLAGS})
4 space ident
Raphael Kubo da Costa (:rakuco)
Comment 5
2011-11-18 05:00:46 PST
(In reply to
comment #4
)
> > Tools/DumpRenderTree/efl/CMakeLists.txt:126 > > +SET_TARGET_PROPERTIES(Programs/DumpRenderTree PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}") > > i don't see how this change is related to the bug title > > > Tools/DumpRenderTree/efl/CMakeLists.txt:131 > > +SET_TARGET_PROPERTIES(Programs/ImageDiff PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}") > > ditto
Now that these directories are being added via ADD_SUBDIRECTORY() instead of being indirectly INCLUDE()d all the way from Source/, by default the compiled binaries were being put in ${CMAKE_BINARY_DIR}/Tools/DumpRenderTree/efl/Programs/{DumpRenderTree,ImageDiff} (the same happened to EWebLauncher and WinCELauncher with their respective paths). Since the current DumpRenderTree code expects these programs to be in ${CMAKE_BINARY_DIR}/Programs (and it is much easier to find them there while working on layout stuff), I had to set RUNTIME_OUTPUT_DIRECTORY.
Raphael Kubo da Costa (:rakuco)
Comment 6
2011-11-18 05:35:04 PST
Created
attachment 115788
[details]
Follow Patrick's suggestions
Gyuyoung Kim
Comment 7
2011-11-18 05:38:26 PST
Comment on
attachment 115788
[details]
Follow Patrick's suggestions
Attachment 115788
[details]
did not pass efl-ews (efl): Output:
http://queues.webkit.org/results/10517089
Raphael Kubo da Costa (:rakuco)
Comment 8
2011-11-18 05:43:50 PST
(In reply to
comment #7
)
> (From update of
attachment 115788
[details]
) >
Attachment 115788
[details]
did not pass efl-ews (efl): > Output:
http://queues.webkit.org/results/10517089
This part of the patch has not changed, it looks like the buildbot is not cleaning the build directory and is thus trying to reuse Source/CMakeLists.txt as the top-level build file.
Raphael Kubo da Costa (:rakuco)
Comment 9
2011-11-18 10:18:56 PST
Created
attachment 115828
[details]
Rebase on top of Patrick's recent CMake commits
Gyuyoung Kim
Comment 10
2011-11-18 10:21:41 PST
Comment on
attachment 115828
[details]
Rebase on top of Patrick's recent CMake commits
Attachment 115828
[details]
did not pass efl-ews (efl): Output:
http://queues.webkit.org/results/10495160
Brent Fulgham
Comment 11
2011-11-18 10:46:31 PST
Comment on
attachment 115828
[details]
Rebase on top of Patrick's recent CMake commits View in context:
https://bugs.webkit.org/attachment.cgi?id=115828&action=review
Looks good. Holding cq+ until buildbot maintainer can update build configuration.
> Source/JavaScriptCore/CMakeLists.txt:20 > + "${CMAKE_SOURCE_DIR}/Source"
It seems funny that CMAKE_SOURCE_DIR doesn't have Source as part of its path. I guess it's really more a "CMAKE_ROOT_BUILD_DIR"?
Raphael Kubo da Costa (:rakuco)
Comment 12
2011-11-21 04:41:40 PST
(In reply to
comment #11
)
> (From update of
attachment 115828
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=115828&action=review
> > Looks good. Holding cq+ until buildbot maintainer can update build configuration.
Patrick, do you know how the issue was solved when Source/ was created? Did someone just clean the bot's build dir after the commit?
> > Source/JavaScriptCore/CMakeLists.txt:20 > > + "${CMAKE_SOURCE_DIR}/Source" > > It seems funny that CMAKE_SOURCE_DIR doesn't have Source as part of its path. I guess it's really more a "CMAKE_ROOT_BUILD_DIR"?
That's a variable defined by CMake itself and points to the root of the source directories (in our case, the top-level directory where the main CMakeLists.txt is).
Patrick R. Gansterer
Comment 13
2011-11-21 07:23:02 PST
(In reply to
comment #12
)
> (In reply to
comment #11
) > > (From update of
attachment 115828
[details]
[details]) > > View in context:
https://bugs.webkit.org/attachment.cgi?id=115828&action=review
> > > > Looks good. Holding cq+ until buildbot maintainer can update build configuration. > > Patrick, do you know how the issue was solved when Source/ was created? Did someone just clean the bot's build dir after the commit?
Yes, both WinCE and EFL build dir have been clean up before.
Raphael Kubo da Costa (:rakuco)
Comment 14
2011-11-21 08:51:00 PST
(In reply to
comment #13
)
> (In reply to
comment #12
) > > (In reply to
comment #11
) > > > (From update of
attachment 115828
[details]
[details] [details]) > > > View in context:
https://bugs.webkit.org/attachment.cgi?id=115828&action=review
> > > > > > Looks good. Holding cq+ until buildbot maintainer can update build configuration. > > > > Patrick, do you know how the issue was solved when Source/ was created? Did someone just clean the bot's build dir after the commit? > > Yes, both WinCE and EFL build dir have been clean up before.
Alright, so is it OK to rebase the patch, have it committed and have the bot clean up afterwards?
Raphael Kubo da Costa (:rakuco)
Comment 15
2011-11-22 05:30:37 PST
Created
attachment 116210
[details]
Mark bfulgham as reviewer, rebase and adjust the blackberry files
Gyuyoung Kim
Comment 16
2011-11-22 05:58:28 PST
Comment on
attachment 116210
[details]
Mark bfulgham as reviewer, rebase and adjust the blackberry files
Attachment 116210
[details]
did not pass efl-ews (efl): Output:
http://queues.webkit.org/results/10548174
Patrick R. Gansterer
Comment 17
2011-11-23 01:08:39 PST
Comment on
attachment 116210
[details]
Mark bfulgham as reviewer, rebase and adjust the blackberry files Clearing flags on attachment: 116210 Committed
r101052
: <
http://trac.webkit.org/changeset/101052
>
Patrick R. Gansterer
Comment 18
2011-11-23 01:08:49 PST
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug