Skip to contentSkip to conversation

core: don't build host-cmake if it is available on the build host

PATCH Superseded Yann E. MORIN 3 participants 5 messages Sep 6, 2016 – Sep 11, 2016

v6 of 8 Superseded Latest is v8 →

lore
Share & tools
lore

Yann E. MORIN

author
#0 lore raw

PATCH v6 1/1 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.

We then ensure, as a post-patch hook in the cmake infra, that no package requires a cmake version higher than what we do in Buildroot. If that's not the case, this is considered an error and the build is aborted. We use a post-patch hook rather a more obvious pre-configure hook because it is then easier to check the requirements of a package without having to build all its dependencies first.

However, the target cmake requires up to cmake 3.4 for running its tests; and they do not get built in our case. Bumping the required host cmake minimum version for the whole of Buildroot would almost always defeat the purpose of this change. So we just exclude the target cmake from the version check.

[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 use BR2_CMAKE_HOST_DEPENDENCY, drop USE_SYSTEM_CMAKE;
- bump to cmake 3.1 for grantlee and opencv;
- add the post-patch hook; exclude target cmake from the check.
]

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 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                     | 45 +++++++++++++++++++++++++++++---
 support/dependencies/check-host-cmake.mk | 19 ++++++++++++++
 support/dependencies/check-host-cmake.sh | 26 ++++++++++++++++++
 3 files changed, 87 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 +42 -3
index 4c6dc62..759b3e6 100644
@@ -35,6 +35,38 @@ 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.
+#
+define CMAKE_CHECK_MIN_VERSION
+	$(Q)v=$$( grep -hr -i 'cmake_minimum_required.*version' $(@D) \
+		|tr '[:upper:]' '[:lower:]' \
+		|sed -r -e '/.*\(version ([[:digit:]]+\.[[:digit:]]+).+/!d; s//\1/' \
+		|sort -t. -k 1,1nr -k2,2nr \
+		|head -n 1 \
+	 ); \
+	MAJOR=$${v%.*}; minor=$${v#*.}; \
+	if [ -z "$${v}" ]; then \
+		: Unknown, not much we can do, OK; \
+	elif [ $(BR2_CMAKE_MIN_VERSION_MAJOR) -gt $${MAJOR} ]; then \
+		: OK; \
+	elif [    $(BR2_CMAKE_MIN_VERSION_MAJOR) -eq $${MAJOR} \
+	       -a $(BR2_CMAKE_MIN_VERSION_MINOR) -ge $${minor} ]; then \
+		: OK; \
+	else \
+		printf "*** Error: incorrect minimum cmake version:\n"; \
+		printf "*** Error: Buildroot requires %s.%s\n" \
+			$(BR2_CMAKE_MIN_VERSION_MAJOR) $(BR2_CMAKE_MIN_VERSION_MINOR); \
+		printf "*** Error: %s needs at least %s.%s\n" \
+			$($(PKG)_NAME) $${MAJOR} $${minor}; \
+		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 +103,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

@@ -85,7 +124,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 +149,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 +185,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 +19 -0
new file mode 100644
index 0000000..8680ec9
@@ -0,0 +1,19 @@
+# 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_MIN_VERSION_MAJOR = 3
+BR2_CMAKE_MIN_VERSION_MINOR = 1
+
+BR2_CMAKE ?= cmake
+ifeq ($(call suitable-host-package,cmake,\
+	$(BR2_CMAKE) $(BR2_CMAKE_MIN_VERSION_MAJOR) $(BR2_CMAKE_MIN_VERSION_MINOR)),)
+BR2_CMAKE = $(HOST_DIR)/usr/bin/cmake
+BR2_CMAKE_HOST_DEPENDENCY = host-cmake
+endif
support/dependencies/check-host-cmake.sh new file +26 -0
new file mode 100755
index 0000000..4951a05
@@ -0,0 +1,26 @@
+#!/bin/sh
+
+candidate="${1}"
+major_min="${2}"
+minor_min="${3}"
+
+cmake=`which ${candidate}`
+if [ ! -x "${cmake}" ]; then
+	# echo nothing: no suitable cmake found
+	exit 1
+fi
+
+version=`${cmake} --version | head -n1 | cut -d\  -f3`
+major=`echo "${version}" | cut -d. -f1`
+minor=`echo "${version}" | cut -d. -f2`
+
+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
#1 lore raw

PATCH v6 1/1 core: don't build host-cmake if it is available on the build host

Dear Yann,

Quoted 68 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.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.

We then ensure, as a post-patch hook in the cmake infra, that no package requires a cmake version higher than what we do in Buildroot. If that's not the case, this is considered an error and the build is aborted. We use a post-patch hook rather a more obvious pre-configure hook because it is then easier to check the requirements of a package without having to build all its dependencies first.

However, the target cmake requires up to cmake 3.4 for running its tests; and they do not get built in our case. Bumping the required host cmake minimum version for the whole of Buildroot would almost always defeat the purpose of this change. So we just exclude the target cmake from the version check.

[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]>
Quoted 7 lines

[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;
- add the post-patch hook; exclude target cmake from the check.
]

Signed-off-by Yann E. MORIN <[email protected]>
Reviewed-by Luca Ceresoli <[email protected]>
Signature · 1 line
Luca
#2 lore raw

PATCH v6 1/1 core: don't build host-cmake if it is available on the build host

In reply to Yann E. MORIN
Quoted 35 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.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.

Quoted 5 lines

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

                                                             ^^^^^^^^^dupe
Quoted 6 lines

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.

Quoted 6 lines

We then ensure, as a post-patch hook in the cmake infra, that no package requires a cmake version higher than what we do in Buildroot. If that's not the case, this is considered an error and the build is aborted. We use a post-patch hook rather a more obvious pre-configure hook because it is then easier to check the requirements of a package without having to build all its dependencies first.

I still have some comments about this minimum version check. Perhaps it's
better to split that into a separate patch so the core functionality can already be committed.

Quoted 8 lines

However, the target cmake requires up to cmake 3.4 for running its tests; and they do not get built in our case. Bumping the required host cmake minimum version for the whole of Buildroot would almost always defeat the purpose of this change. So we just exclude the target cmake from the version check.

[0] https://cmake.org/gitweb?p=cmake.git;h=e8b8b37ef6fef094940d3384df5a1d421b9fa568

[snip]

Quoted 6 lines
package/pkg-cmake.mk +0 -0
index 4c6dc62..759b3e6 100644
@@ -35,6 +35,38 @@ ifneq ($(QUIET),)
 CMAKE_QUIET = -DCMAKE_RULE_MESSAGES=OFF -DCMAKE_INSTALL_MESSAGE=NEVER

package/pkg-cmake.mk ifneq ($(QUIET),) · line 45

Quoted 10 lines
package/pkg-cmake.mk +8 -0
 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.
+#
+define CMAKE_CHECK_MIN_VERSION

This would be perfect to do with a script instead - no make support is needed,
except for passing $(@D) $($(PKG)_NAME) and BR2_CMAKE_MIN_VERSION.

package/pkg-cmake.mk ifneq ($(QUIET),) · line 46

Quoted 1 line
package/pkg-cmake.mk +1 -0
+	$(Q)v=$$( grep -hr -i 'cmake_minimum_required.*version' $(@D) \

Shouldn't we search just in CMakeLists.txt files? Do you think it's likely that
a package declares the minimum required version in and included file? That said, the extra time spent on grepping through everything doesn't amount to much unless it's a large package, and in that case it's anyway going to be dominated by package build time.

package/pkg-cmake.mk ifneq ($(QUIET),) · line 48

Quoted 2 lines
package/pkg-cmake.mk +2 -0
+		|tr '[:upper:]' '[:lower:]' \
+		|sed -r -e '/.*\(version ([[:digit:]]+\.[[:digit:]]+).+/!d; s//\1/' \

Not exactly an easy to understand sed script...

package/pkg-cmake.mk ifneq ($(QUIET),) · line 49

Quoted 1 line
package/pkg-cmake.mk +1 -0
+		|sort -t. -k 1,1nr -k2,2nr \

Since you're anyway doing the version sort here already, you could just add
$(BR2_CMAKE_MIN_VERSION_MAJOR).$(BR2_CMAKE_MIN_VERSION_MINOR) into the mix and verify that the first line is equal to $(BR2_CMAKE_MIN_VERSION_MAJOR).$(BR2_CMAKE_MIN_VERSION_MINOR)
We already use that trick for checking the make version, for instance.

(and then you don't need to split anymore, just $(BR2_CMAKE_MIN_VERSION)).

package/pkg-cmake.mk ifneq ($(QUIET),) · line 59

Quoted 10 lines
package/pkg-cmake.mk +10 -0
+		|head -n 1 \
+	 ); \
+	MAJOR=$${v%.*}; minor=$${v#*.}; \
+	if [ -z "$${v}" ]; then \
+		: Unknown, not much we can do, OK; \
+	elif [ $(BR2_CMAKE_MIN_VERSION_MAJOR) -gt $${MAJOR} ]; then \
+		: OK; \
+	elif [    $(BR2_CMAKE_MIN_VERSION_MAJOR) -eq $${MAJOR} \
+	       -a $(BR2_CMAKE_MIN_VERSION_MINOR) -ge $${minor} ]; then \
+		: OK; \

Which removes the need for all these conditions.

package/pkg-cmake.mk ifneq ($(QUIET),) · line 65

Quoted 6 lines
package/pkg-cmake.mk +6 -0
+	else \
+		printf "*** Error: incorrect minimum cmake version:\n"; \
+		printf "*** Error: Buildroot requires %s.%s\n" \
+			$(BR2_CMAKE_MIN_VERSION_MAJOR) $(BR2_CMAKE_MIN_VERSION_MINOR); \
+		printf "*** Error: %s needs at least %s.%s\n" \
+			$($(PKG)_NAME) $${MAJOR} $${minor}; \

I don't think this error message is very clear. How about:

*** Error: package %s needs cmake version %s.%s\n

***        Buildroot's minimum version BR2_CMAKE_MIN_VERSION\n
***        should be set to at least that version. Currently\n
***        it is set to %s.\n
Quoted 56 lines
package/pkg-cmake.mk +14 -3
+		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 +103,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

@@ -85,7 +124,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 +149,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 +185,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 +2 -0
new file mode 100644
index 0000000..8680ec9
@@ -0,0 +1,19 @@
+# Versions before 3.0 are affected by the bug described in
+# https://git.busybox.net/buildroot/commit/?id=ef2c1970e4bff3be3992014070392b0e6bc28bd2

support/dependencies/check-host-cmake.mk line 12

Quoted 10 lines
support/dependencies/check-host-cmake.mk new file +10 -0
+# 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_MIN_VERSION_MAJOR = 3
+BR2_CMAKE_MIN_VERSION_MINOR = 1

It's just a personal preference, but I'd rather see a single
BR2_CMAKE_MIN_VERSION = 3.1, and where you need the major or minor use ${v%.*}.

Quoted 16 lines
support/dependencies/check-host-cmake.mk new file +7 -0
+
+BR2_CMAKE ?= cmake
+ifeq ($(call suitable-host-package,cmake,\
+	$(BR2_CMAKE) $(BR2_CMAKE_MIN_VERSION_MAJOR) $(BR2_CMAKE_MIN_VERSION_MINOR)),)
+BR2_CMAKE = $(HOST_DIR)/usr/bin/cmake
+BR2_CMAKE_HOST_DEPENDENCY = host-cmake
+endif
support/dependencies/check-host-cmake.sh new file +3 -0
new file mode 100755
index 0000000..4951a05
@@ -0,0 +1,26 @@
+#!/bin/sh
+
+candidate="${1}"

support/dependencies/check-host-cmake.sh line 13

Quoted 10 lines
support/dependencies/check-host-cmake.sh new file +10 -0
+major_min="${2}"
+minor_min="${3}"
+
+cmake=`which ${candidate}`
+if [ ! -x "${cmake}" ]; then
+	# echo nothing: no suitable cmake found
+	exit 1
+fi
+
+version=`${cmake} --version | head -n1 | cut -d\  -f3`

Again minor nit, but I think -d' ' is easier to understand than -d\ .

support/dependencies/check-host-cmake.sh line 15

Quoted 2 lines
support/dependencies/check-host-cmake.sh new file +2 -0
+major=`echo "${version}" | cut -d. -f1`
+minor=`echo "${version}" | cut -d. -f2`

I know this is just copy-paste from the other scripts, but ${version%.*} really
is better.

Regards,
Arnout

Quoted 12 lines
support/dependencies/check-host-cmake.sh new file +11 -0
+
+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 · 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
author
#3 lore raw

PATCH v6 1/1 core: don't build host-cmake if it is available on the build host

Arnout, All,

On 2016-09-10 00:26 +0200, Arnout Vandecappelle spake thusly:

Quoted 7 lines · Yann E. MORIN
Quoted 6 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 20 lines
Quoted 6 lines
package/pkg-cmake.mk +0 -0
index 4c6dc62..759b3e6 100644
@@ -35,6 +35,38 @@ ifneq ($(QUIET),)
 CMAKE_QUIET = -DCMAKE_RULE_MESSAGES=OFF -DCMAKE_INSTALL_MESSAGE=NEVER

package/pkg-cmake.mk ifneq ($(QUIET),) · line 45

Quoted 10 lines
package/pkg-cmake.mk +8 -0
 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.
+#
+define CMAKE_CHECK_MIN_VERSION

This would be perfect to do with a script instead - no make support is needed,
except for passing $(@D) $($(PKG)_NAME) and BR2_CMAKE_MIN_VERSION.

Quoted 7 lines

package/pkg-cmake.mk ifneq ($(QUIET),) · line 46

Quoted 1 line
package/pkg-cmake.mk +1 -0
+	$(Q)v=$$( grep -hr -i 'cmake_minimum_required.*version' $(@D) \

Shouldn't we search just in CMakeLists.txt files? Do you think it's likely that
a package declares the minimum required version in and included file? That said, the extra time spent on grepping through everything doesn't amount to much unless it's a large package, and in that case it's anyway going to be dominated by package build time.

Well, grepping in a source tree that has been freshly extracted (i.e. it would be cache-hot) would not be very costly...

Quoted 4 lines

package/pkg-cmake.mk ifneq ($(QUIET),) · line 48

Quoted 2 lines
package/pkg-cmake.mk +2 -0
+		|tr '[:upper:]' '[:lower:]' \
+		|sed -r -e '/.*\(version ([[:digit:]]+\.[[:digit:]]+).+/!d; s//\1/' \

Not exactly an easy to understand sed script...

I must be very weird, then, as I can read it pretty easily:
- any number of any character,
- followed by an opening paranthesis,
- followed by the literal string 'version' and then a space,
- start a matching group
- followed by one or more digits,
- followed by a literal dot
- followed by one or more digits,
- end the matching group
- followed by one or more of any character (implied: up to the end of the line),
- delete lines that don't match the above,
- on a line that matches, replace it with the matching group.

;-)

Quoted 6 lines

package/pkg-cmake.mk ifneq ($(QUIET),) · line 49

Quoted 1 line
package/pkg-cmake.mk +1 -0
+		|sort -t. -k 1,1nr -k2,2nr \

Since you're anyway doing the version sort here already, you could just add
$(BR2_CMAKE_MIN_VERSION_MAJOR).$(BR2_CMAKE_MIN_VERSION_MINOR) into the mix and verify that the first line is equal to $(BR2_CMAKE_MIN_VERSION_MAJOR).$(BR2_CMAKE_MIN_VERSION_MINOR)

orry, I'm not sure I understand what you meant. Something like:

(echo 3.1; grep blabla) |blabla

and then check that we get 3.1, right?

Quoted 1 line

We already use that trick for checking the make version, for instance.

I've looked at support/dependencies/dependencies.sh and we're not using that "trick" to check the make version;

   85 MAKE_MAJOR=$(echo $MAKE_VERSION | sed -e "s/\..*//g")
   86 MAKE_MINOR=$(echo $MAKE_VERSION | sed -e "s/^$MAKE_MAJOR\.//g" -e "s/\..*//g" -e "s/[a-zA-Z].*//g")
   87 if [ $MAKE_MAJOR -lt 3 ] || [ $MAKE_MAJOR -eq 3 -a $MAKE_MINOR -lt 81 ] ; then
   88 ?   echo
   89 ?   echo "You have make '$MAKE_VERSION' installed.  GNU make >=3.81 is required"
   90 ?   exit 1;
   91 fi;
Quoted 9 lines

package/pkg-cmake.mk ifneq ($(QUIET),) · line 65

Quoted 6 lines
package/pkg-cmake.mk +6 -0
+	else \
+		printf "*** Error: incorrect minimum cmake version:\n"; \
+		printf "*** Error: Buildroot requires %s.%s\n" \
+			$(BR2_CMAKE_MIN_VERSION_MAJOR) $(BR2_CMAKE_MIN_VERSION_MINOR); \
+		printf "*** Error: %s needs at least %s.%s\n" \
+			$($(PKG)_NAME) $${MAJOR} $${minor}; \

I don't think this error message is very clear. How about:

Quoted 4 lines

*** Error: package %s needs cmake version %s.%s\n

***        Buildroot's minimum version BR2_CMAKE_MIN_VERSION\n
***        should be set to at least that version. Currently\n
***        it is set to %s.\n

If you want...

[--SNIP--]

Quoted 13 lines
Quoted 13 lines
support/dependencies/check-host-cmake.mk new file +7 -0
new file mode 100644
index 0000000..8680ec9
@@ -0,0 +1,19 @@
+# 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
Quoted 8 lines

support/dependencies/check-host-cmake.mk line 12

Quoted 5 lines
support/dependencies/check-host-cmake.mk new file +5 -0
+# package is bumped or a new one added, and it requires a higher
+# version, our cmake infra will catch it and whine.
+#
+BR2_CMAKE_MIN_VERSION_MAJOR = 3
+BR2_CMAKE_MIN_VERSION_MINOR = 1

It's just a personal preference, but I'd rather see a single
BR2_CMAKE_MIN_VERSION = 3.1, and where you need the major or minor use ${v%.*}.

OK...

Quoted 19 lines
Quoted 19 lines
support/dependencies/check-host-cmake.mk new file +6 -0
+BR2_CMAKE ?= cmake
+ifeq ($(call suitable-host-package,cmake,\
+	$(BR2_CMAKE) $(BR2_CMAKE_MIN_VERSION_MAJOR) $(BR2_CMAKE_MIN_VERSION_MINOR)),)
+BR2_CMAKE = $(HOST_DIR)/usr/bin/cmake
+BR2_CMAKE_HOST_DEPENDENCY = host-cmake
+endif
support/dependencies/check-host-cmake.sh new file +7 -0
new file mode 100755
index 0000000..4951a05
@@ -0,0 +1,26 @@
+#!/bin/sh
+
+candidate="${1}"
+major_min="${2}"
+minor_min="${3}"
+
+cmake=`which ${candidate}`
Quoted 8 lines

support/dependencies/check-host-cmake.sh line 13

Quoted 6 lines
support/dependencies/check-host-cmake.sh new file +6 -0
+if [ ! -x "${cmake}" ]; then
+	# echo nothing: no suitable cmake found
+	exit 1
+fi
+
+version=`${cmake} --version | head -n1 | cut -d\  -f3`

Again minor nit, but I think -d' ' is easier to understand than -d\ .

OK...

Quoted 5 lines

support/dependencies/check-host-cmake.sh line 15

Quoted 2 lines
support/dependencies/check-host-cmake.sh new file +2 -0
+major=`echo "${version}" | cut -d. -f1`
+minor=`echo "${version}" | cut -d. -f2`

I know this is just copy-paste from the other scripts, but ${version%.*} really
is better.

Yep, I missed changing it...

Thanks!

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.  |
'------------------------------^-------^------------------^--------------------'
#4 lore raw

PATCH v6 1/1 core: don't build host-cmake if it is available on the build host

Quoted 4 lines · Yann E. MORIN

Arnout, All,

On 2016-09-10 00:26 +0200, Arnout Vandecappelle spake thusly:

Quoted 1 line · Yann E. MORIN

[snip]

Quoted 7 lines
Quoted 6 lines

package/pkg-cmake.mk ifneq ($(QUIET),) · line 49

Quoted 1 line
package/pkg-cmake.mk +1 -0
+		|sort -t. -k 1,1nr -k2,2nr \

Since you're anyway doing the version sort here already, you could just add
$(BR2_CMAKE_MIN_VERSION_MAJOR).$(BR2_CMAKE_MIN_VERSION_MINOR) into the mix and verify that the first line is equal to $(BR2_CMAKE_MIN_VERSION_MAJOR).$(BR2_CMAKE_MIN_VERSION_MINOR)

Quoted 5 lines

orry, I'm not sure I understand what you meant. Something like:

(echo 3.1; grep blabla) |blabla

and then check that we get 3.1, right?

Yep.

Quoted 6 lines
Quoted 2 lines

We already use that trick for checking the make version, for instance.

I've looked at support/dependencies/dependencies.sh and we're not using that "trick" to check the make version;

Quoted 7 lines
   85 MAKE_MAJOR=$(echo $MAKE_VERSION | sed -e "s/\..*//g")
   86 MAKE_MINOR=$(echo $MAKE_VERSION | sed -e "s/^$MAKE_MAJOR\.//g" -e "s/\..*//g" -e "s/[a-zA-Z].*//g")
   87 if [ $MAKE_MAJOR -lt 3 ] || [ $MAKE_MAJOR -eq 3 -a $MAKE_MINOR -lt 81 ] ; then
   88 ?   echo
   89 ?   echo "You have make '$MAKE_VERSION' installed.  GNU make >=3.81 is required"
   90 ?   exit 1;
   91 fi;

Ha, so we check for the make version twice then...

Top-level Makefile, line 51-59:
# Save running make version since it's clobbered by the make package RUNNING_MAKE_VERSION := $(MAKE_VERSION)

# Check for minimal make version (note: this check will break at make 10.x) MIN_MAKE_VERSION = 3.81
ifneq ($(firstword $(sort $(RUNNING_MAKE_VERSION) $(MIN_MAKE_VERSION))),$(MIN_MAKE_VERSION))
$(error You have make '$(RUNNING_MAKE_VERSION)' installed. GNU make >= $(MIN_MAKE_VERSION) is required)
endif

Regards,
Arnout

[snip]

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