Bug 153211 - [CMake] Duplicate attempts to find software during cmake stage
Summary: [CMake] Duplicate attempts to find software during cmake stage
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: Other
Hardware: PC Linux
: P2 Normal
Assignee: Michael Catanzaro
URL:
Keywords:
Depends on:
Blocks: 153528
  Show dependency treegraph
 
Reported: 2016-01-18 13:20 PST by Michael Catanzaro
Modified: 2016-01-31 19:46 PST (History)
5 users (show)

See Also:


Attachments
Patch (6.24 KB, patch)
2016-01-18 13:46 PST, Michael Catanzaro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Catanzaro 2016-01-18 13:20:18 PST
Here is an excerpt of our cmake output:

-- Could NOT find Ruby (missing:  RUBY_INCLUDE_DIR RUBY_LIBRARY RUBY_CONFIG_INCLUDE_DIR) (found suitable version "1.9.3", minimum required is "1.9")
-- Checking for module 'gtk+-quartz-3.0'
--   Package 'gtk+-quartz-3.0' not found
-- Found HarfBuzz: /home/mcatanzaro/src/jhbuild/install/include/harfbuzz (Required is at least version "0.9.2") 
-- Found ICU header files in /usr/include
-- Found ICU libraries: /usr/lib64/libicuuc.so
-- Found GLIB: /home/mcatanzaro/src/jhbuild/install/include/glib-2.0;/home/mcatanzaro/src/jhbuild/install/lib/glib-2.0/include (found suitable version "2.47.4", minimum required is "2.36") 
-- Using platform-specific CMakeLists: /home/mcatanzaro/src/WebKit/Source/WTF/wtf/PlatformGTK.cmake
-- Could NOT find Ruby (missing:  RUBY_INCLUDE_DIR RUBY_LIBRARY RUBY_CONFIG_INCLUDE_DIR) (found suitable version "1.9.3", minimum required is "1.9")
-- Checking for module 'gtk+-quartz-3.0'
--   Package 'gtk+-quartz-3.0' not found
-- Found ICU header files in /usr/include
-- Found ICU libraries: /usr/lib64/libicuuc.so
-- Using platform-specific CMakeLists: /home/mcatanzaro/src/WebKit/Source/JavaScriptCore/PlatformGTK.cmake
-- Using platform-specific CMakeLists: /home/mcatanzaro/src/WebKit/Source/JavaScriptCore/shell/PlatformGTK.cmake
-- Could NOT find Ruby (missing:  RUBY_INCLUDE_DIR RUBY_LIBRARY RUBY_CONFIG_INCLUDE_DIR) (found suitable version "1.9.3", minimum required is "1.9")
-- Checking for module 'gtk+-quartz-3.0'
--   Package 'gtk+-quartz-3.0' not found
-- Found ICU header files in /usr/include
-- Found ICU libraries: /usr/lib64/libicuuc.so

We check for Ruby three times, for gtk+-quartz-3.0 three times, for ICU three times, and for GLIB twice (once higher up in the output). I don't know how to explain what's happening for GLib, but for the others, I think it must be because we call into WebKitCommon so many times:

JavaScriptCore/CMakeLists.txt:include(WebKitCommon)
WTF/CMakeLists.txt:include(WebKitCommon)
WebCore/CMakeLists.txt:include(WebKitCommon)
WebKit/CMakeLists.txt:include(WebKitCommon)

So that looks like an inefficiency to address....
Comment 1 Michael Catanzaro 2016-01-18 13:40:50 PST
It's probably due to r188540.
Comment 2 Michael Catanzaro 2016-01-18 13:46:24 PST
Created attachment 269235 [details]
Patch
Comment 3 WebKit Commit Bot 2016-01-18 17:23:20 PST
Comment on attachment 269235 [details]
Patch

Clearing flags on attachment: 269235

Committed r195242: <http://trac.webkit.org/changeset/195242>
Comment 4 WebKit Commit Bot 2016-01-18 17:23:24 PST
All reviewed patches have been landed.  Closing bug.
Comment 5 Alex Christensen 2016-01-22 13:36:53 PST
This caused the Windows builds to copy files to bin/JavaScriptCore.resources instead of bin64/JavaScriptCore.resources.  Not sure why.  CMAKE_RUNTIME_OUTPUT_DIRECTORY is set in OptionsWin.cmake and used in JavaScriptCore/PlatformWin.cmake
Comment 6 Michael Catanzaro 2016-01-22 14:08:31 PST
It's because they're also set in the toplevel CMakeLists.txt:

# -----------------------------------------------------------------------------
# Common configuration
#------------------------------------------------------------------------------
include(WebKitCommon)

# -----------------------------------------------------------------------------
# Output directories
#------------------------------------------------------------------------------
set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib)
set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib)
set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin)

Now WebKitCommon only executes once, which causes OptionsWin.cmake to execute (only once), then the settings are immediately overridden. Previously they were set in OptionsWin.cmake, overwritten by toplevel CMakeLists.txt, then overwritten again and again in OptionsWin.cmake when descending into the JavaScriptCore, WebKit, and WebCore directories, since OptionsWin.cmake was executed many times.

I don't have a strong opinion on how to fix this; the toplevel set statements could be put in conditionals, or moved into WebKitCommon.cmake, or even just moved up above the include (though that would be pretty fragile).
Comment 7 Michael Catanzaro 2016-01-31 19:46:15 PST
Note that we landed a fix for the Windows build in bug #153373.