Bug 179971 - [CMake] Values of CMAKE_BUILD_TYPE from toolchain file are ignored
Summary: [CMake] Values of CMAKE_BUILD_TYPE from toolchain file are ignored
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Adrian Perez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-11-23 02:23 PST by Adrian Perez
Modified: 2017-11-27 07:35 PST (History)
9 users (show)

See Also:


Attachments
Example toolchainfile.cmake generated by Buildroot (3.50 KB, text/plain)
2017-11-23 02:24 PST, Adrian Perez
no flags Details
Patch (1.44 KB, patch)
2017-11-23 21:23 PST, Adrian Perez
no flags Details | Formatted Diff | Diff
Patch for landing (1.86 KB, patch)
2017-11-27 05:02 PST, Adrian Perez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adrian Perez 2017-11-23 02:23:26 PST
I have noticed this one while working with Buildroot. For CMake-based
packages it generates a “Toolchain file” [1] which defines the paths
to the cross-compilation tools, build options, and so on. This toolchain
file also specifies the build type with:

  # Build type from the Buildroot configuration                                    
  set(CMAKE_BUILD_TYPE Release CACHE STRING "Buildroot build configuration")       

Nevertheless, when WebKit is configured with the above setting, it will
notify that “RelWithDebInfo” is being used instead (output re-formatted
by me for clarity) — the value from the toolchain file is ignored:

  >>> wpewebkit custom Configuring
  (mkdir -p /home/aperez/devel/wpe/buildroot-wpe-overlay/buildroot-2017.08.1/output/build/wpewebkit-custom/ && \
   cd /home/aperez/devel/wpe/buildroot-wpe-overlay/buildroot-2017.08.1/output/build/wpewebkit-custom/ && \
   rm -f CMakeCache.txt && \
   PATH="/home/aperez/devel/wpe/buildroot-wpe-overlay/buildroot-2017.08.1/output/host/bin:/home/aperez/devel/wpe/buildroot-wpe-overlay/buildroot-2017.08.1/output/host/sbin:/usr/local/bin:/usr/local/sbin" \
     /usr/bin/cmake /home/aperez/devel/wpe/buildroot-wpe-overlay/buildroot-2017.08.1/output/build/wpewebkit-custom/ \
     -DCMAKE_TOOLCHAIN_FILE="/home/aperez/devel/wpe/buildroot-wpe-overlay/buildroot-2017.08.1/output/host/share/buildroot/toolchainfile.cmake" \
     -DCMAKE_INSTALL_PREFIX="/usr" \
     -DCMAKE_COLOR_MAKEFILE=OFF \
     -DBUILD_DOC=OFF \
     -DBUILD_DOCS=OFF \
     -DBUILD_EXAMPLE=OFF \
     -DBUILD_EXAMPLES=OFF \
     -DBUILD_TEST=OFF \
     -DBUILD_TESTS=OFF \
     -DBUILD_TESTING=OFF \
     -DBUILD_SHARED_LIBS=ON \
     -DENABLE_API_TESTS=OFF \
     -DENABLE_GEOLOCATION=OFF \
     -DENABLE_GTKDOC=OFF \
     -DENABLE_INTROSPECTION=OFF \
     -DENABLE_MINIBROWSER=OFF \
     -DENABLE_SPELLCHECK=OFF \
     -DPORT=WPE \
     -DENABLE_JIT=ON \
     -DENABLE_VIDEO=OFF \
     -DENABLE_WEB_AUDIO=OFF \
     -DENABLE_MEDIA_STREAM=OFF \
     -DENABLE_WEB_CRYPTO=OFF \
     -DENABLE_WEBDRIVER=OFF \
     -DENABLE_XSLT=OFF )
  CMake Warning at CMakeLists.txt:7 (message):
    No CMAKE_BUILD_TYPE value specified, defaulting to RelWithDebInfo.

Probably this has to do with how we are trying to detect whether “CMAKE_BUILD_TYPE”
was set (or not) in the command line. In this case it is not coming directly from
the command line, and it is being reset to “RelWithDebInfo”.

This is definitely having an impact on the build results: If I patch the Buildroot
recipe to pass “-DCMAKE_BUILD_TYPE=Release” in the command line, instead of letting
the default “RelWithDebInfo” overwrite the setting from the toolchain file, then
the resulting binaries take ~17 MiB less of disk space.


---
[1] https://cmake.org/cmake/help/v3.3/variable/CMAKE_TOOLCHAIN_FILE.html
Comment 1 Adrian Perez 2017-11-23 02:24:53 PST
Created attachment 327489 [details]
Example toolchainfile.cmake generated by Buildroot
Comment 2 Michael Catanzaro 2017-11-23 07:58:14 PST
The code at the top of our toplevel CMakeLists.txt is this:

if (NOT CMAKE_BUILD_TYPE)
    message(WARNING "No CMAKE_BUILD_TYPE value specified, defaulting to RelWithDebInfo.")
    set(CMAKE_BUILD_TYPE "RelWithDebInfo" CACHE STRING "Choose the type of build." FORCE)
else ()
    message(STATUS "The CMake build type is: ${CMAKE_BUILD_TYPE}")
endif ()

So... that's fairly straightforward.

Are the other variables in the toolchain file already set before that point?
Comment 3 Adrian Perez 2017-11-23 08:48:19 PST
(In reply to Michael Catanzaro from comment #2)
> The code at the top of our toplevel CMakeLists.txt is this:
> 
> if (NOT CMAKE_BUILD_TYPE)
>     message(WARNING "No CMAKE_BUILD_TYPE value specified, defaulting to
> RelWithDebInfo.")
>     set(CMAKE_BUILD_TYPE "RelWithDebInfo" CACHE STRING "Choose the type of
> build." FORCE)
> else ()
>     message(STATUS "The CMake build type is: ${CMAKE_BUILD_TYPE}")
> endif ()
> 
> So... that's fairly straightforward.

It is, indeed! It's puzzling, and I have no idea why CMake is so
bizarre. I suspect that the toolchain file is NOT loaded directly
on startup, but included by some file from “/usrshare/cmake/*”,
and those get pulled in by one of the top-level commands used to
configure.

> Are the other variables in the toolchain file already set before that point?

Will check. Probably not if my hypothesis as outlined above is
correct.
Comment 4 Adrian Perez 2017-11-23 21:23:41 PST
Created attachment 327524 [details]
Patch
Comment 5 Adrian Perez 2017-11-23 21:25:10 PST
(In reply to Adrian Perez from comment #3)
> (In reply to Michael Catanzaro from comment #2)
> > The code at the top of our toplevel CMakeLists.txt is this:
> > 
> > if (NOT CMAKE_BUILD_TYPE)
> >     message(WARNING "No CMAKE_BUILD_TYPE value specified, defaulting to
> > RelWithDebInfo.")
> >     set(CMAKE_BUILD_TYPE "RelWithDebInfo" CACHE STRING "Choose the type of
> > build." FORCE)
> > else ()
> >     message(STATUS "The CMake build type is: ${CMAKE_BUILD_TYPE}")
> > endif ()
> > 
> > So... that's fairly straightforward.
> 
> It is, indeed! It's puzzling, and I have no idea why CMake is so
> bizarre. I suspect that the toolchain file is NOT loaded directly
> on startup, but included by some file from “/usrshare/cmake/*”,
> and those get pulled in by one of the top-level commands used to
> configure.

It turns out that the call to “project()” in the top-level CMakeLists.txt
is what checks “CMAKE_TOOLCHAIN_FILE” and loads it. Therefore we need to
call “project()” first before checking anything.
Comment 6 Carlos Alberto Lopez Perez 2017-11-24 01:19:01 PST
Comment on attachment 327524 [details]
Patch

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

wow, this is certainly tricky.

> CMakeLists.txt:10
>  
> +project(WebKit)
> +
>  if (NOT CMAKE_BUILD_TYPE)
>      message(WARNING "No CMAKE_BUILD_TYPE value specified, defaulting to RelWithDebInfo.")
>      set(CMAKE_BUILD_TYPE "RelWithDebInfo" CACHE STRING "Choose the type of build." FORCE)

I think this deserves a comment line saying that calling project first is needed to automatically define CMAKE_BUILD_TYPE from the toolchain file
Comment 7 Adrian Perez 2017-11-27 04:56:40 PST
(In reply to Carlos Alberto Lopez Perez from comment #6)
> Comment on attachment 327524 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=327524&action=review
> 
> wow, this is certainly tricky.
> 
> > CMakeLists.txt:10
> >  
> > +project(WebKit)
> > +
> >  if (NOT CMAKE_BUILD_TYPE)
> >      message(WARNING "No CMAKE_BUILD_TYPE value specified, defaulting to RelWithDebInfo.")
> >      set(CMAKE_BUILD_TYPE "RelWithDebInfo" CACHE STRING "Choose the type of build." FORCE)
> 
> I think this deserves a comment line saying that calling project first is
> needed to automatically define CMAKE_BUILD_TYPE from the toolchain file

I'll add the comment before landing the change, with a link to

  https://cmake.org/cmake/help/v3.3/command/project.html

where it says that “project()” must be the first command (it does not explain
very well why, but then again not being particularly clear is something the
CMake documentation tends to do).
Comment 8 Adrian Perez 2017-11-27 05:02:58 PST
Created attachment 327629 [details]
Patch for landing
Comment 9 WebKit Commit Bot 2017-11-27 07:34:52 PST
Comment on attachment 327629 [details]
Patch for landing

Clearing flags on attachment: 327629

Committed r225168: <https://trac.webkit.org/changeset/225168>
Comment 10 WebKit Commit Bot 2017-11-27 07:34:54 PST
All reviewed patches have been landed.  Closing bug.
Comment 11 Radar WebKit Bug Importer 2017-11-27 07:35:25 PST
<rdar://problem/35697658>