core/pkg-cmake: do not build host-cmake if not needed (branch yem/host-cmake)
v8 cover core/pkg-cmake: do not build host-cmake if not needed (branch yem/host-cmake)
Hello All!
This series attempts at using the cmake already present on the build machine, if any, to avoid building our own version if the existing cmake is suitable.
Changes v7 -> V8:
- Chandle three-part version strings (Arnout)
Changes v6 -> v7:
- split out in two patches (Arnout)
Changes v7 -> v8:
-
Regards,
Yann E. MORIN.
The following changes since commit 0c6635e9d6a9b0f1d361f1363c4b647adcae6da2
package/libva-intel-driver: bump version to 1.7.2 (2016-09-05 22:17:54 +0200)
are available in the git repository at:
git://git.buildroot.org/~ymorin/git/buildroot.git
for you to fetch changes up to 51cc816673de51799d4f86a7045ef4f480fbab46
core/pkg-cmake: ensure no package needs a cmake newer than we do (2016-09-12 23:55:46 +0200)
----------------------------------------------------------------
Luca Ceresoli (1):
core: don't build host-cmake if it is available on the build host
Yann E. MORIN (1):
core/pkg-cmake: ensure no package needs a cmake newer than we do
package/pkg-cmake.mk | 46 +++++++++++++++++++++++++++++--- support/dependencies/check-host-cmake.mk | 18 +++++++++++++ support/dependencies/check-host-cmake.sh | 39 +++++++++++++++++++++++++++ 3 files changed, 100 insertions(+), 3 deletions(-) create mode 100644 support/dependencies/check-host-cmake.mk create mode 100755 support/dependencies/check-host-cmake.sh
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
PATCH v8 1/2 core: don't build host-cmake if it is available on the build host
From: Luca Ceresoli <[email protected]>
Currently all cmake packages depend on host-cmake. Unfortunately host-cmake takes a long time to configure and build: almost 7 minutes on a dual-core i5 with SSD. The time does not change even with ccache enabled.
Indeed, building host-cmake is avoidable if it is already installed on the build host: CMake is supposed to be quite portable, and the only patch in Buildroot for the CMake package seems to only affect target-cmake.
Thus we automatically skip building host-cmake and use the one on the system if:
- cmake is available on the system and
- it is recent enough.
First, we leverage the existing infrastructure in support/dependencies/dependencies.mk to find out whether there's a suitable cmake executable on the system. Its path can be passed in the BR2_CMAKE environment variable, otherwise it defaults to "cmake". If it is enabled, found and suitable then we set BR2_CMAKE_HOST_DEPENDENCY to empty; otherwise we set BR2_CMAKE_HOST_DEPENDENCY to 'host-cmake' and override BR2_CMAKE with "$(HOST_DIR)/usr/bin/cmake" to revert to using our own cmake (the old behaviour).
Then in pkg-cmake.mk we replace the hard-coded dependency on host-cmake to using the BR2_CMAKE_HOST_DEPENDENCY variable, and we use $(BR2_CMAKE) instead of $(HOST_DIR)/usr/bin/cmake.
Unlike what we do for host-tar and host-xzcat, for host-cmake we do not add host-cmake to DEPENDENCIES_HOST_PREREQ. If we did, host-cmake would be a dependency for _any_ package when it's not installed on the host, even when no cmake package is selected.
Cmake versions older than 3.0 are affected by the bug described and fixed in Buildroot in ef2c1970e4bf ("cmake: add patch to fix Qt mkspecs detection"). The bug was fixed in upstream CMake in version 3.0 [0].
Amongst all the cmake packages currently in Buildroot, the currently highest version mentioned in cmake_minimum_required() is 3.1 (grantlee and opencv3).
Thus we use 3.1 as the lowest required cmake for now, until a package is bumped, or a new package added, with a higher required version.
[0] https://cmake.org/gitweb?p=cmake.git;h=e8b8b37ef6fef094940d3384df5a1d421b9fa568
[yann.morin.1998 at free.fr:
- simplify logic in check-host-cmake.mk;
- set and use BR2_CMAKE_HOST_DEPENDENCY, drop USE_SYSTEM_CMAKE;
- bump to cmake 3.1 for grantlee and opencv;
]
---
Results (by Luca, before Yann's changes):
On my build server, this patch reduced the build time for a batch of 16 Buildroot configurations (all building cmake-based packages) from 2h44m to 1h54m. Woah, it's a 30% saving!
Tested on:
- Ubuntu 14.04 without CMake, with official CMake (2.8), PPA CMake (3.2)
- Ubuntu 15.10 without CMake, with official CMake (3.2)
- Ubuntu 16.04 without CMake, with official CMake (3.5)
Results (by Yann):
On my machine, this gains about 5min 30s per build, which is far from negligible.
Tested, by artificially bumping/lowering the minimum required version, on:
- Ubuntu 16.04 without CMake, with official CMake (3.5)
---
Changes v7 -> v8:
- handle versions in three parts (X.Y>Z) (Arnout)
Changes v6 -> v7:
- drop the post-patch hook, moved to a follow-up patch (Arnout)
- eye-candy in script (Arnout)
Changes v5 -> v6:
- rename variables in the version check in pkg-cmake (Luca)
- fix commit log (Luca)
Changes v4 -> v5:
- adopted by Yann;
- drop USE_SYSTEM_CMAKE in favour of BR2_CMAKE_HOST_DEPENDENCY which directly contains 'host-cmake' if it is needed;
- unconditionally add $(BR2_CMAKE_HOST_DEPENDENCY) to dependencies: it is empty if host-cmake is not needed;
- check our minimum version is enough in a post-patch hook.
Changes v3 -> v4:
- rename USE_SYSTEM_HOST_CMAKE -> USE_SYSTEM_CMAKE (Arnout)
- ditch BR2_TRY_SYSTEM_CMAKE and use system-cmake unconditionally if it is found and >= 3.0 (Arnout)
- rename BR2_HOST_CMAKE -> BR2_CMAKE since it can be either a host-cmake (built by Buildroot) or a system-cmake
Changes v2 -> v3:
- make this feature optional via the BR2_TRY_SYSTEM_CMAKE kconfig variable
- rename the CMAKE variable to BR2_HOST_CMAKE, so it's coherent with the naming of other variables overridable by the environment, e.g. BR2_DL_DIR
- invert the logic of the variable triggering the host-cmake
dependency: USE_SYSTEM_CMAKE instead of BUILD_HOST_CMAKE; needed to implement BR2_TRY_SYSTEM_CMAKE
Changes v1 -> v2:
- Require cmake >= 3.0. Fixes qjson failure (Luca, Samuel, Thomas)
- In check-host-cmake.sh only search $1, not "cmake" (Arnout)
- typo: host-ccache -> host-cmake (Arnout)
--- package/pkg-cmake.mk | 6 ++--- support/dependencies/check-host-cmake.mk | 18 +++++++++++++++ support/dependencies/check-host-cmake.sh | 39 ++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 3 deletions(-) create mode 100644 support/dependencies/check-host-cmake.mk create mode 100755 support/dependencies/check-host-cmake.sh
package/pkg-cmake.mk +3 -3
@@ -85,7 +85,7 @@ define $(2)_CONFIGURE_CMDS cd $$($$(PKG)_BUILDDIR) && \ rm -f CMakeCache.txt && \ PATH=$$(BR_PATH) \ - $$($$(PKG)_CONF_ENV) $$(HOST_DIR)/usr/bin/cmake $$($$(PKG)_SRCDIR) \ + $$($$(PKG)_CONF_ENV) $$(BR2_CMAKE) $$($$(PKG)_SRCDIR) \ -DCMAKE_TOOLCHAIN_FILE="$$(HOST_DIR)/usr/share/buildroot/toolchainfile.cmake" \ -DCMAKE_BUILD_TYPE=$$(if $$(BR2_ENABLE_DEBUG),RelWithDebInfo,Release) \ -DCMAKE_INSTALL_PREFIX="/usr" \
@@ -110,7 +110,7 @@ define $(2)_CONFIGURE_CMDS cd $$($$(PKG)_BUILDDIR) && \ rm -f CMakeCache.txt && \ PATH=$$(BR_PATH) \ - $$(HOST_DIR)/usr/bin/cmake $$($$(PKG)_SRCDIR) \ + $$(BR2_CMAKE) $$($$(PKG)_SRCDIR) \ -DCMAKE_INSTALL_SO_NO_EXE=0 \ -DCMAKE_FIND_ROOT_PATH="$$(HOST_DIR)" \ -DCMAKE_FIND_ROOT_PATH_MODE_PROGRAM="BOTH" \
@@ -146,7 +146,7 @@ endif # primitives to find {C,LD}FLAGS, add it to the dependency list. $(2)_DEPENDENCIES += host-pkgconf -$(2)_DEPENDENCIES += host-cmake +$(2)_DEPENDENCIES += $(BR2_CMAKE_HOST_DEPENDENCY) # # Build step. Only define it if not already defined by the package .mk
support/dependencies/check-host-cmake.mk new file +18 -0
@@ -0,0 +1,18 @@ +# Versions before 3.0 are affected by the bug described in +# https://git.busybox.net/buildroot/commit/?id=ef2c1970e4bff3be3992014070392b0e6bc28bd2 +# and fixed in upstream CMake in version 3.0: +# https://cmake.org/gitweb?p=cmake.git;h=e8b8b37ef6fef094940d3384df5a1d421b9fa568 +# +# Set this to either 3.0 or higher, depending on the highest minimum +# version required by any of the packages bundled in Buildroot. If a +# package is bumped or a new one added, and it requires a higher +# version, our cmake infra will catch it and whine. +# +BR2_CMAKE_VERSION_MIN = 3.1 + +BR2_CMAKE ?= cmake +ifeq ($(call suitable-host-package,cmake,\ + $(BR2_CMAKE) $(BR2_CMAKE_VERSION_MIN)),) +BR2_CMAKE = $(HOST_DIR)/usr/bin/cmake +BR2_CMAKE_HOST_DEPENDENCY = host-cmake +endif
support/dependencies/check-host-cmake.sh new file +39 -0
@@ -0,0 +1,39 @@ +#!/bin/sh + +candidate="${1}" +version_min="${2}" + +major_min="${version_min%.*}" +minor_min="${version_min#*.}" + +cmake=`which ${candidate}` +if [ ! -x "${cmake}" ]; then + # echo nothing: no suitable cmake found + exit 1 +fi + +# Extract version X.Y from versions in the form X.Y or X.Y.Z +# with X, Y and Z numbers with one or more digits each, e.g. +# 3.2 -> 3.2 +# 3.2.3 -> 3.2 +# 3.2.42 -> 3.2 +# 3.10 -> 3.10 +# 3.10.4 -> 3.10 +# 3.10.42 -> 3.10 +version="$(${cmake} --version \ + |sed -r -e '/.* ([[:digit:]]+\.[[:digit:]]+).*$/!d;' \ + -e 's//\1/' + )" +major="${version%.*}" +minor="${version#*.}" + +if [ ${major} -gt ${major_min} ]; then + echo "${cmake}" +else + if [ ${major} -eq ${major_min} -a ${minor} -ge ${minor_min} ]; then + echo "${cmake}" + else + # echo nothing: no suitable cmake found + exit 1 + fi +fi
Signature · 1 line
2.7.4
PATCH v8 1/2 core: don't build host-cmake if it is available on the build host
Dear Yann,
Quoted 212 lines · Yann E. MORIN
From: Luca Ceresoli <[email protected]>
Currently all cmake packages depend on host-cmake. Unfortunately host-cmake takes a long time to configure and build: almost 7 minutes on a dual-core i5 with SSD. The time does not change even with ccache enabled.
Indeed, building host-cmake is avoidable if it is already installed on the build host: CMake is supposed to be quite portable, and the only patch in Buildroot for the CMake package seems to only affect target-cmake.
Thus we automatically skip building host-cmake and use the one on the system if:
- cmake is available on the system and
- it is recent enough.First, we leverage the existing infrastructure in
support/dependencies/dependencies.mkto find out whether there's a suitable cmake executable on the system. Its path can be passed in theBR2_CMAKEenvironment variable, otherwise it defaults to "cmake". If it is enabled, found and suitable then we setBR2_CMAKE_HOST_DEPENDENCYto empty; otherwise we setBR2_CMAKE_HOST_DEPENDENCYto 'host-cmake' and overrideBR2_CMAKEwith "$(HOST_DIR)/usr/bin/cmake" to revert to using our own cmake (the old behaviour).Then in pkg-cmake.mk we replace the hard-coded dependency on host-cmake to using the
BR2_CMAKE_HOST_DEPENDENCYvariable, and we use $(BR2_CMAKE) instead of $(HOST_DIR)/usr/bin/cmake.Unlike what we do for host-tar and host-xzcat, for host-cmake we do not add host-cmake to
DEPENDENCIES_HOST_PREREQ. If we did, host-cmake would be a dependency for _any_ package when it's not installed on the host, even when no cmake package is selected.Cmake versions older than 3.0 are affected by the bug described and fixed in Buildroot in
ef2c1970e4bf("cmake: add patch to fix Qt mkspecs detection"). The bug was fixed in upstream CMake in version 3.0 [0].Amongst all the cmake packages currently in Buildroot, the currently highest version mentioned in
cmake_minimum_required()is 3.1 (grantlee and opencv3).Thus we use 3.1 as the lowest required cmake for now, until a package is bumped, or a new package added, with a higher required version.
[0] https://cmake.org/gitweb?p=cmake.git;h=e8b8b37ef6fef094940d3384df5a1d421b9fa568
Signed-off-by Luca Ceresoli <[email protected]>Cc Samuel Martin <[email protected]>Cc Davide Viti <[email protected]>Cc Arnout Vandecappelle <[email protected]>Cc Thomas Petazzoni <[email protected]>Reviewed-by Romain Naour <[email protected]>Tested-by Romain Naour <[email protected]>[yann.morin.1998 at free.fr:
- simplify logic in check-host-cmake.mk;
- set and useBR2_CMAKE_HOST_DEPENDENCY, dropUSE_SYSTEM_CMAKE;
- bump to cmake 3.1 for grantlee and opencv;
]Signed-off-by Yann E. MORIN <[email protected]>---
Results (by Luca, before Yann's changes):On my build server, this patch reduced the build time for a batch of 16 Buildroot configurations (all building cmake-based packages) from 2h44m to 1h54m. Woah, it's a 30% saving!
Tested on:
- Ubuntu 14.04 without CMake, with official CMake (2.8), PPA CMake (3.2)
- Ubuntu 15.10 without CMake, with official CMake (3.2)
- Ubuntu 16.04 without CMake, with official CMake (3.5)Results (by Yann):
On my machine, this gains about 5min 30s per build, which is far from negligible.
Tested, by artificially bumping/lowering the minimum required version, on:
- Ubuntu 16.04 without CMake, with official CMake (3.5)---
Changes v7 -> v8:
- handle versions in three parts (X.Y>Z) (Arnout)Changes v6 -> v7:
- drop the post-patch hook, moved to a follow-up patch (Arnout)
- eye-candy in script (Arnout)Changes v5 -> v6:
- rename variables in the version check in pkg-cmake (Luca)
- fix commit log (Luca)Changes v4 -> v5:
- adopted by Yann;
- dropUSE_SYSTEM_CMAKEin favour ofBR2_CMAKE_HOST_DEPENDENCYwhich directly contains 'host-cmake' if it is needed;
- unconditionally add $(BR2_CMAKE_HOST_DEPENDENCY) to dependencies: it is empty if host-cmake is not needed;
- check our minimum version is enough in a post-patch hook.Changes v3 -> v4:
- renameUSE_SYSTEM_HOST_CMAKE->USE_SYSTEM_CMAKE(Arnout)
- ditchBR2_TRY_SYSTEM_CMAKEand use system-cmake unconditionally if it is found and >= 3.0 (Arnout)
- renameBR2_HOST_CMAKE->BR2_CMAKEsince it can be either a host-cmake (built by Buildroot) or a system-cmakeChanges v2 -> v3:
- make this feature optional via theBR2_TRY_SYSTEM_CMAKEkconfig variable
- rename the CMAKE variable toBR2_HOST_CMAKE, so it's coherent with the naming of other variables overridable by the environment, e.g.BR2_DL_DIR
- invert the logic of the variable triggering the host-cmake
dependency:USE_SYSTEM_CMAKEinstead ofBUILD_HOST_CMAKE; needed to implementBR2_TRY_SYSTEM_CMAKEChanges v1 -> v2:
- Require cmake >= 3.0. Fixes qjson failure (Luca, Samuel, Thomas)
- Incheck-host-cmake.shonly search $1, not "cmake" (Arnout)
- typo: host-ccache -> host-cmake (Arnout)--- package/pkg-cmake.mk | 6 ++--- support/dependencies/check-host-cmake.mk | 18 +++++++++++++++ support/dependencies/check-host-cmake.sh | 39 ++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 3 deletions(-) create mode 100644 support/dependencies/check-host-cmake.mk create mode 100755 support/dependencies/check-host-cmake.shpackage/pkg-cmake.mk +3
-3@@ -85,7 +85,7 @@ define $(2)_CONFIGURE_CMDS cd $$($$(PKG)_BUILDDIR) && \ rm -f CMakeCache.txt && \ PATH=$$(BR_PATH) \ - $$($$(PKG)_CONF_ENV) $$(HOST_DIR)/usr/bin/cmake $$($$(PKG)_SRCDIR) \ + $$($$(PKG)_CONF_ENV) $$(BR2_CMAKE) $$($$(PKG)_SRCDIR) \ -DCMAKE_TOOLCHAIN_FILE="$$(HOST_DIR)/usr/share/buildroot/toolchainfile.cmake" \ -DCMAKE_BUILD_TYPE=$$(if $$(BR2_ENABLE_DEBUG),RelWithDebInfo,Release) \ -DCMAKE_INSTALL_PREFIX="/usr" \@@ -110,7 +110,7 @@ define $(2)_CONFIGURE_CMDS cd $$($$(PKG)_BUILDDIR) && \ rm -f CMakeCache.txt && \ PATH=$$(BR_PATH) \ - $$(HOST_DIR)/usr/bin/cmake $$($$(PKG)_SRCDIR) \ + $$(BR2_CMAKE) $$($$(PKG)_SRCDIR) \ -DCMAKE_INSTALL_SO_NO_EXE=0 \ -DCMAKE_FIND_ROOT_PATH="$$(HOST_DIR)" \ -DCMAKE_FIND_ROOT_PATH_MODE_PROGRAM="BOTH" \@@ -146,7 +146,7 @@ endif # primitives to find {C,LD}FLAGS, add it to the dependency list. $(2)_DEPENDENCIES += host-pkgconf -$(2)_DEPENDENCIES += host-cmake +$(2)_DEPENDENCIES += $(BR2_CMAKE_HOST_DEPENDENCY) # # Build step. Only define it if not already defined by the package .mksupport/dependencies/check-host-cmake.mk new file +18
-0@@ -0,0 +1,18 @@ +# Versions before 3.0 are affected by the bug described in +# https://git.busybox.net/buildroot/commit/?id=ef2c1970e4bff3be3992014070392b0e6bc28bd2 +# and fixed in upstream CMake in version 3.0: +# https://cmake.org/gitweb?p=cmake.git;h=e8b8b37ef6fef094940d3384df5a1d421b9fa568 +# +# Set this to either 3.0 or higher, depending on the highest minimum +# version required by any of the packages bundled in Buildroot. If a +# package is bumped or a new one added, and it requires a higher +# version, our cmake infra will catch it and whine. +# +BR2_CMAKE_VERSION_MIN = 3.1 + +BR2_CMAKE ?= cmake +ifeq ($(call suitable-host-package,cmake,\ + $(BR2_CMAKE) $(BR2_CMAKE_VERSION_MIN)),) +BR2_CMAKE = $(HOST_DIR)/usr/bin/cmake +BR2_CMAKE_HOST_DEPENDENCY = host-cmake +endifsupport/dependencies/check-host-cmake.sh new file +18
-0@@ -0,0 +1,39 @@ +#!/bin/sh + +candidate="${1}" +version_min="${2}" + +major_min="${version_min%.*}" +minor_min="${version_min#*.}" + +cmake=`which ${candidate}` +if [ ! -x "${cmake}" ]; then + # echo nothing: no suitable cmake found + exit 1 +fi + +# Extract version X.Y from versions in the form X.Y or X.Y.Z +# with X, Y and Z numbers with one or more digits each, e.g. +# 3.2 -> 3.2 +# 3.2.3 -> 3.2
support/dependencies/check-host-cmake.sh line 28
Quoted 10 lines
support/dependencies/check-host-cmake.sh new file +10
-0+# 3.2.42 -> 3.2 +# 3.10 -> 3.10 +# 3.10.4 -> 3.10 +# 3.10.42 -> 3.10 +version="$(${cmake} --version \ + |sed -r -e '/.* ([[:digit:]]+\.[[:digit:]]+).*$/!d;' \ + -e 's//\1/' + )" +major="${version%.*}" +minor="${version#*.}"
You removed the 'head -n1' in the pipe. It's not needed in the CMake versions I'm aware of, indeed. But still I'd like it to be there in case future/other cmake versions respond to 'cmake --version' with things like:
cmake version 3.5.1
Linked against libfoobar 7.6.5
In such a case, mayor and minor would be garbage. Maybe it will never happen, but look at 'tshark --version' as an example of what I'm thinking about.
I also find that the 8-lines comment above is not needed. The sed script you wrote is not _that_ unreadable. IMHO of course.
That said, I hate myself for the comments I wrote... This patch is floating around since more weeks than the lines it changes, and I really would like to see it committed, not delayed once again. Almost any of the versions sent so far are good enough to me. So consider the above comments are absolutely non-blocker and I'll be more than happy if this version of the patch were committed as is. But should you send v8, you might want to evaluate my proposals.
Signature · 4 lines
Luca -- Luca
PATCH v8 1/2 core: don't build host-cmake if it is available on the build host
Luca, All,
On 2016-09-13 23:05 +0200, Luca Ceresoli spake thusly:
Quoted 6 lines · Yann E. MORIN
Quoted 5 lines
From: Luca Ceresoli <[email protected]>
Currently all cmake packages depend on host-cmake. Unfortunately host-cmake takes a long time to configure and build: almost 7 minutes on a dual-core i5 with SSD. The time does not change even with ccache enabled.
[--SNIP--]
Quoted 28 lines
Quoted 28 lines
support/dependencies/check-host-cmake.sh new file +22
-0@@ -0,0 +1,39 @@ +#!/bin/sh + +candidate="${1}" +version_min="${2}" + +major_min="${version_min%.*}" +minor_min="${version_min#*.}" + +cmake=`which ${candidate}` +if [ ! -x "${cmake}" ]; then + # echo nothing: no suitable cmake found + exit 1 +fi + +# Extract version X.Y from versions in the form X.Y or X.Y.Z +# with X, Y and Z numbers with one or more digits each, e.g. +# 3.2 -> 3.2 +# 3.2.3 -> 3.2 +# 3.2.42 -> 3.2 +# 3.10 -> 3.10 +# 3.10.4 -> 3.10 +# 3.10.42 -> 3.10
Quoted 8 lines
support/dependencies/check-host-cmake.sh line 28
Quoted 6 lines
support/dependencies/check-host-cmake.sh new file +6
-0+version="$(${cmake} --version \ + |sed -r -e '/.* ([[:digit:]]+\.[[:digit:]]+).*$/!d;' \ + -e 's//\1/' + )" +major="${version%.*}" +minor="${version#*.}"You removed the 'head -n1' in the pipe.
Indeed, but look at the sed script (which is not that complicated): all lines that do not match the pattern are ignored.
|sed -r -e '/.* ([[:digit:]]+\.[[:digit:]]+).*$/!d;' \
Here ----^"!d" means "if no match, delete line".
Then the next expression is only aplied on matching lines, and "\1" refers to the ()-match, which is the version string.
Regards,
Yann E. MORIN.
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
PATCH v8 1/2 core: don't build host-cmake if it is available on the build host
Quoted 50 lines · Yann E. MORIN
Luca, All,
On 2016-09-13 23:05 +0200, Luca Ceresoli spake thusly:
Quoted 6 lines · Yann E. MORIN
Quoted 5 lines
From: Luca Ceresoli <[email protected]>
Currently all cmake packages depend on host-cmake. Unfortunately host-cmake takes a long time to configure and build: almost 7 minutes on a dual-core i5 with SSD. The time does not change even with ccache enabled.[--SNIP--]
Quoted 28 lines
Quoted 28 lines
support/dependencies/check-host-cmake.sh new file +22
-0@@ -0,0 +1,39 @@ +#!/bin/sh + +candidate="${1}" +version_min="${2}" + +major_min="${version_min%.*}" +minor_min="${version_min#*.}" + +cmake=`which ${candidate}` +if [ ! -x "${cmake}" ]; then + # echo nothing: no suitable cmake found + exit 1 +fi + +# Extract version X.Y from versions in the form X.Y or X.Y.Z +# with X, Y and Z numbers with one or more digits each, e.g. +# 3.2 -> 3.2 +# 3.2.3 -> 3.2 +# 3.2.42 -> 3.2 +# 3.10 -> 3.10 +# 3.10.4 -> 3.10 +# 3.10.42 -> 3.10Quoted 8 lines
support/dependencies/check-host-cmake.sh line 28
Quoted 6 lines
support/dependencies/check-host-cmake.sh new file +6
-0+version="$(${cmake} --version \ + |sed -r -e '/.* ([[:digit:]]+\.[[:digit:]]+).*$/!d;' \ + -e 's//\1/' + )" +major="${version%.*}" +minor="${version#*.}"You removed the 'head -n1' in the pipe.
Indeed, but look at the sed script (which is not that complicated): all lines that do not match the pattern are ignored.
Quoted 7 lines
|sed -r -e '/.* ([[:digit:]]+\.[[:digit:]]+).*$/!d;' \ Here ----^"!d" means "if no match, delete line".
Then the next expression is only aplied on matching lines, and "\1" refers to the ()-match, which is the version string.
You snipped away Luca's example: if there are two lines that contain something
that looks like a version number, then you get two versions out of this script. So repeating Luca's example:
cmake version 3.5.1
Linked against libfoobar 7.6.5
will give you
version="3.5
7.6"
Regards,
Arnout
Quoted 4 lines
Regards,
Yann E. MORIN.
Signature · 6 lines
Arnout Vandecappelle arnout at mind be Senior Embedded Software Architect +32-16-286500 Essensium/Mind http://www.mind.be G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle GPG fingerprint: 7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
PATCH v8 1/2 core: don't build host-cmake if it is available on the build host
Dear Arnout, Yann,
Quoted 65 lines · Arnout Vandecappelle
Quoted 50 lines · Yann E. MORIN
Luca, All,
On 2016-09-13 23:05 +0200, Luca Ceresoli spake thusly:
Quoted 6 lines · Yann E. MORIN
Quoted 5 lines
From: Luca Ceresoli <[email protected]>
Currently all cmake packages depend on host-cmake. Unfortunately host-cmake takes a long time to configure and build: almost 7 minutes on a dual-core i5 with SSD. The time does not change even with ccache enabled.[--SNIP--]
Quoted 28 lines
Quoted 28 lines
support/dependencies/check-host-cmake.sh new file +22
-0@@ -0,0 +1,39 @@ +#!/bin/sh + +candidate="${1}" +version_min="${2}" + +major_min="${version_min%.*}" +minor_min="${version_min#*.}" + +cmake=`which ${candidate}` +if [ ! -x "${cmake}" ]; then + # echo nothing: no suitable cmake found + exit 1 +fi + +# Extract version X.Y from versions in the form X.Y or X.Y.Z +# with X, Y and Z numbers with one or more digits each, e.g. +# 3.2 -> 3.2 +# 3.2.3 -> 3.2 +# 3.2.42 -> 3.2 +# 3.10 -> 3.10 +# 3.10.4 -> 3.10 +# 3.10.42 -> 3.10Quoted 8 lines
support/dependencies/check-host-cmake.sh line 28
Quoted 6 lines
support/dependencies/check-host-cmake.sh new file +6
-0+version="$(${cmake} --version \ + |sed -r -e '/.* ([[:digit:]]+\.[[:digit:]]+).*$/!d;' \ + -e 's//\1/' + )" +major="${version%.*}" +minor="${version#*.}"You removed the 'head -n1' in the pipe.
Indeed, but look at the sed script (which is not that complicated): all lines that do not match the pattern are ignored.
Quoted 7 lines
|sed -r -e '/.* ([[:digit:]]+\.[[:digit:]]+).*$/!d;' \ Here ----^"!d" means "if no match, delete line".
Then the next expression is only aplied on matching lines, and "\1" refers to the ()-match, which is the version string.
You snipped away Luca's example: if there are two lines that contain something
that looks like a version number, then you get two versions out of this script. So repeating Luca's example:
Quoted 8 lines
cmake version 3.5.1
Linked against libfoobar 7.6.5
will give you
version="3.5
7.6"
You're right. Yann and I clarified this yesterday night on IRC.
OTOH we also agreed that:
1. the script in v8 works with all cmake versions we are aware of,
including 2.8.* and 3.5.1
2. adding 'head -n1', or any other change to the script, can guard against any possible future change in the output of 'cmake --version' that we just can't predict.
Thus you can consider my comment as "if I wrote it, I'd have put a head -n1", which is a matter of personal taste and not a request to Yann to change the code. Should cmake change its output in the future, we'll handle it when it happens.
Signature · 1 line
Luca
Thomas Petazzoni
AppliedPATCH v8 1/2 core: don't build host-cmake if it is available on the build host
In reply to Yann E. MORINHello,
Quoted 60 lines · Yann E. MORIN
From: Luca Ceresoli <[email protected]>
Currently all cmake packages depend on host-cmake. Unfortunately host-cmake takes a long time to configure and build: almost 7 minutes on a dual-core i5 with SSD. The time does not change even with ccache enabled.
Indeed, building host-cmake is avoidable if it is already installed on the build host: CMake is supposed to be quite portable, and the only patch in Buildroot for the CMake package seems to only affect target-cmake.
Thus we automatically skip building host-cmake and use the one on the system if:
- cmake is available on the system and
- it is recent enough.First, we leverage the existing infrastructure in
support/dependencies/dependencies.mkto find out whether there's a suitable cmake executable on the system. Its path can be passed in theBR2_CMAKEenvironment variable, otherwise it defaults to "cmake". If it is enabled, found and suitable then we setBR2_CMAKE_HOST_DEPENDENCYto empty; otherwise we setBR2_CMAKE_HOST_DEPENDENCYto 'host-cmake' and overrideBR2_CMAKEwith "$(HOST_DIR)/usr/bin/cmake" to revert to using our own cmake (the old behaviour).Then in pkg-cmake.mk we replace the hard-coded dependency on host-cmake to using the
BR2_CMAKE_HOST_DEPENDENCYvariable, and we use $(BR2_CMAKE) instead of $(HOST_DIR)/usr/bin/cmake.Unlike what we do for host-tar and host-xzcat, for host-cmake we do not add host-cmake to
DEPENDENCIES_HOST_PREREQ. If we did, host-cmake would be a dependency for _any_ package when it's not installed on the host, even when no cmake package is selected.Cmake versions older than 3.0 are affected by the bug described and fixed in Buildroot in
ef2c1970e4bf("cmake: add patch to fix Qt mkspecs detection"). The bug was fixed in upstream CMake in version 3.0 [0].Amongst all the cmake packages currently in Buildroot, the currently highest version mentioned in
cmake_minimum_required()is 3.1 (grantlee and opencv3).Thus we use 3.1 as the lowest required cmake for now, until a package is bumped, or a new package added, with a higher required version.
[0] https://cmake.org/gitweb?p=cmake.git;h=e8b8b37ef6fef094940d3384df5a1d421b9fa568
Signed-off-by Luca Ceresoli <[email protected]>Cc Samuel Martin <[email protected]>Cc Davide Viti <[email protected]>Cc Arnout Vandecappelle <[email protected]>Cc Thomas Petazzoni <[email protected]>Reviewed-by Romain Naour <[email protected]>Tested-by Romain Naour <[email protected]>[yann.morin.1998 at free.fr:
- simplify logic in check-host-cmake.mk;
- set and useBR2_CMAKE_HOST_DEPENDENCY, dropUSE_SYSTEM_CMAKE;
- bump to cmake 3.1 for grantlee and opencv;
]
Quoted 4 lines
Signed-off-by Yann E. MORIN <[email protected]>---
Results (by Luca, before Yann's changes):
Applied to master, thanks.
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
PATCH v8 2/2 core/pkg-cmake: ensure no package needs a cmake newer than we do
In reply to Yann E. MORINIf the existing host cmake is deemed usable, we now skip building our own variant.
However, in the following conditions, the build would fail:
- host cmake: 3.2
- Buildroot requirement: 3.1
- package requirement: 3.4
- cmake package in Buildroot: 3.5
This is because Buildroot would consider that cmake-3.2 is usable (and it technically is for Buildroot, which requires 3.0), but the package will fail at configure time.
To catch this situation and fix Buildroot, we add a post-patch hook that checks that, amongst all cmake versions required by the package, the version required by Buildroot is at least equal or higher.
We do use a post-patch hook rather than the more expected pre-configure hook, to avoid having to build all the package dependencies just to check the cmake version.
Exclude the cmake (target variant) from that check, since it requires cmake >= 3.4. However, that requirement is only for the tests, and we're not building them; all the other parts of the build have no requirement besides a cmake >= 2.8). Bumping the version Buildroot requires to 3.4 would almost always defeat the purpose of this change, as a lot of not-so-old distributions are still using cmake < 3.4; only very recent ones released since the end of 2015 would stand a chance of having such a cmake version. So we just exclude cmake from that check.
--- package/pkg-cmake.mk | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+)
package/pkg-cmake.mk +40 -0
@@ -35,6 +35,39 @@ ifneq ($(QUIET),) CMAKE_QUIET = -DCMAKE_RULE_MESSAGES=OFF -DCMAKE_INSTALL_MESSAGE=NEVER endif +# +# Check the package does not require a cmake version more recent than we do. +# +# Some packages may use a variable to set the minimum required version. In +# this case, there is not much we can do, so we just accept it; the configure +# would fail later anyway in this case. +# +# We sort all the minimum versions required by the package (there may be more +# than one, if the package has cmake "sub-modules"), to which we add our own +# version. We then sort everything to get the highest version first, and expect +# our version to win. +# +# If not, then it means the package has a higher requirement than we do. +# +define CMAKE_CHECK_MIN_VERSION + $(Q)v=$$( \ + { grep -hr -i 'cmake_minimum_required.*version' $(@D); \ + echo $(BR2_CMAKE_VERSION_MIN); \ + } \ + |tr '[:upper:]' '[:lower:]' \ + |sed -r -e '/.*\(version ([[:digit:]]+\.[[:digit:]]+).+/!d; s//\1/' \ + |sort -t. -k 1,1nr -k2,2nr \ + |head -n 1 \ + ); \ + if [ "$${v}" != "$(BR2_CMAKE_VERSION_MIN)" ]; then \ + printf "*** Error: package %s needs cmake version >= %s\n" $($(PKG)_NAME) $${v}; \ + printf "*** Buildroot's minimum version BR2_CMAKE_VERSION_MIN\n"; \ + printf "*** should be set to at least that version, but is\n" \ + printf "*** currently set to %s."; $(BR2_CMAKE_VERSION_MIN); \ + exit 1; \ + fi +endef + ################################################################################ # inner-cmake-package -- defines how the configuration, compilation and # installation of a CMake package should be done, implements a few hooks to
@@ -71,6 +104,13 @@ else $(2)_BUILDDIR = $$($(2)_SRCDIR)/buildroot-build endif +# Special exception for cmake, which requires cmake up to 3.4, but +# only to run its tests; all other equirements are on at most 3.0. +# Just skip the version check for cmake, and only for cmake. +ifneq ($(1),cmake) +$(2)_POST_PATCH_HOOKS += CMAKE_CHECK_MIN_VERSION +endif + # # Configure step. Only define it if not already defined by the package # .mk file. And take care of the differences between host and target
Signature · 1 line
2.7.4
PATCH v8 2/2 core/pkg-cmake: ensure no package needs a cmake newer than we do
Hello,
Quoted 31 lines · Yann E. MORIN
If the existing host cmake is deemed usable, we now skip building our own variant.
However, in the following conditions, the build would fail:
- host cmake: 3.2
- Buildroot requirement: 3.1
- package requirement: 3.4
- cmake package in Buildroot: 3.5This is because Buildroot would consider that cmake-3.2 is usable (and it technically is for Buildroot, which requires 3.0), but the package will fail at configure time.
To catch this situation and fix Buildroot, we add a post-patch hook that checks that, amongst all cmake versions required by the package, the version required by Buildroot is at least equal or higher.
We do use a post-patch hook rather than the more expected pre-configure hook, to avoid having to build all the package dependencies just to check the cmake version.
Exclude the cmake (target variant) from that check, since it requires cmake >= 3.4. However, that requirement is only for the tests, and we're not building them; all the other parts of the build have no requirement besides a cmake >= 2.8). Bumping the version Buildroot requires to 3.4 would almost always defeat the purpose of this change, as a lot of not-so-old distributions are still using cmake < 3.4; only very recent ones released since the end of 2015 would stand a chance of having such a cmake version. So we just exclude cmake from that check.
Quoted 7 lines
Signed-off-by "Yann E. MORIN" <[email protected]>Cc Luca Ceresoli <[email protected]>Cc Arnout Vandecappelle <[email protected]>Cc Samuel Martin <[email protected]>Cc Thomas Petazzoni <[email protected]>Cc Davide Viti <[email protected]>
As was said by others, I don't really like this solution. It's complicated, it doesn't work in all cases, and covers a situation where the error message is already going to be fairly explicit.
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com