Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 0/3] Skip host-cmake dependency to speedup builds
@ 2016-07-01 15:53 Luca Ceresoli
  2016-07-01 15:53 ` [Buildroot] [PATCH 1/3] Move the host-pkgconf dependency from host-cmake to pkg-cmake Luca Ceresoli
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Luca Ceresoli @ 2016-07-01 15:53 UTC (permalink / raw)
  To: buildroot

Hi,

this is a respin of my old patch series (February 2016) to allow
skipping host-cmake and speedup builds.

Patch 1 is new, and it fixes a problem raised by Samuel. Well, it
doesn't fix it at its root, but it improves the workaround letting it
work even after my host-cmake patch.

Patch 2 simply adds some docs that can be useful in cmake.mk.

Patch 3 is the one doing the interesting stuff. It has a very detailed
commit message, and a discussion that needs comments from as many
reviewers as possible. Thanks in advance!


Luca Ceresoli (3): Move the host-pkgconf dependency from host-cmake to
  pkg-cmake cmake: add documentation about how it is built Don't build
  host-cmake if it is available on the build host

 Config.in                                | 19 +++++++++++++++++++
 package/cmake/cmake.mk                   | 14 +++++++++++++-
 package/pkg-cmake.mk                     | 10 ++++++++--
 support/dependencies/check-host-cmake.mk | 11 +++++++++++
 support/dependencies/check-host-cmake.sh | 30 ++++++++++++++++++++++++++++++
 5 files changed, 81 insertions(+), 3 deletions(-)
 create mode 100644 support/dependencies/check-host-cmake.mk
 create mode 100755 support/dependencies/check-host-cmake.sh

-- 
2.7.4

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Buildroot] [PATCH 1/3] Move the host-pkgconf dependency from host-cmake to pkg-cmake
  2016-07-01 15:53 [Buildroot] [PATCH 0/3] Skip host-cmake dependency to speedup builds Luca Ceresoli
@ 2016-07-01 15:53 ` Luca Ceresoli
  2016-07-02 13:56   ` Arnout Vandecappelle
  2016-07-01 15:53 ` [Buildroot] [PATCH 2/3] cmake: add documentation about how it is built Luca Ceresoli
  2016-07-01 15:53 ` [Buildroot] [PATCH 3/3] Don't build host-cmake if it is available on the build host Luca Ceresoli
  2 siblings, 1 reply; 15+ messages in thread
From: Luca Ceresoli @ 2016-07-01 15:53 UTC (permalink / raw)
  To: buildroot

In 3d475ee0ba4d6eea6aca25594cfe5bb153ac804f a dependency on
host-pkgconf was added to host-cmake. It is a workaround to fix build
failures for packages that use pkgconf through a cmake module, but do
not depend on host-pkgconf as they should.

Since it is the package that needs host-pkgconf and not host-cmake
itself, move the dependency to the proper place, in pkg-cmake.mk.

Also copy the explanation on the mentioned commit as a comment in
order to clarify why we do this.

Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
Cc: Samuel Martin <s.martin49@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Davide Viti <zinosat@tiscali.it>
Cc: Arnout Vandecappelle <arnout@mind.be>

---

This is done as a preliminary cleanup step for the following patch
that allows to skip building host-cmake in some cases. The dependency
where it is now would be missed when we skip building host-cmake, so I
moved it where it will be catched in all cases. Without the following
patch this can be considered just a cleanup without any visible
effect.
---
 package/cmake/cmake.mk | 2 +-
 package/pkg-cmake.mk   | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/package/cmake/cmake.mk b/package/cmake/cmake.mk
index a776b14..33fb4aa 100644
--- a/package/cmake/cmake.mk
+++ b/package/cmake/cmake.mk
@@ -10,7 +10,7 @@ CMAKE_SITE = https://cmake.org/files/v$(CMAKE_VERSION_MAJOR)
 CMAKE_LICENSE = BSD-3c
 CMAKE_LICENSE_FILES = Copyright.txt
 
-HOST_CMAKE_DEPENDENCIES = host-pkgconf
+HOST_CMAKE_DEPENDENCIES =
 CMAKE_DEPENDENCIES = zlib jsoncpp libcurl libarchive expat bzip2 xz
 
 CMAKE_CONF_OPTS = \
diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk
index 81dcfcc..8d7d265 100644
--- a/package/pkg-cmake.mk
+++ b/package/pkg-cmake.mk
@@ -149,6 +149,10 @@ $(2)_DEPENDENCIES ?= $$(filter-out host-skeleton host-toolchain $(1),\
 	$$(patsubst host-host-%,host-%,$$(addprefix host-,$$($(3)_DEPENDENCIES))))
 endif
 
+# Since some CMake modules (even upstream ones) use pgk_check_modules
+# primitives to find {C,LD}FLAGS, add it to the dependency list.
+$(2)_DEPENDENCIES += host-pkgconf
+
 $(2)_DEPENDENCIES += host-cmake
 
 #
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Buildroot] [PATCH 2/3] cmake: add documentation about how it is built
  2016-07-01 15:53 [Buildroot] [PATCH 0/3] Skip host-cmake dependency to speedup builds Luca Ceresoli
  2016-07-01 15:53 ` [Buildroot] [PATCH 1/3] Move the host-pkgconf dependency from host-cmake to pkg-cmake Luca Ceresoli
@ 2016-07-01 15:53 ` Luca Ceresoli
  2016-07-02 13:57   ` Arnout Vandecappelle
  2016-07-02 14:48   ` Yann E. MORIN
  2016-07-01 15:53 ` [Buildroot] [PATCH 3/3] Don't build host-cmake if it is available on the build host Luca Ceresoli
  2 siblings, 2 replies; 15+ messages in thread
From: Luca Ceresoli @ 2016-07-01 15:53 UTC (permalink / raw)
  To: buildroot

Commit 7b17bafc5d7948aff3059e058ada80ad1fc50500 by Davide Viti has a
detailed explanation of some unusual techniques used for building
host-cmake and (target-)cmake. This is useful information for whoever
starts hacking on it, so copy it in the makefile, where it will be
easily noticed.

Also remove the sentence about host-cmake having a runtime dependency
on host-pkgconfig (not true anymore: it's the specific cmake-packages
that depend on it) and fix typos.

Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
Cc: Samuel Martin <s.martin49@gmail.com>
Cc: Davide Viti <zinosat@tiscali.it>
Cc: Arnout Vandecappelle <arnout@mind.be>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 package/cmake/cmake.mk | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/package/cmake/cmake.mk b/package/cmake/cmake.mk
index 33fb4aa..d734e03 100644
--- a/package/cmake/cmake.mk
+++ b/package/cmake/cmake.mk
@@ -10,6 +10,18 @@ CMAKE_SITE = https://cmake.org/files/v$(CMAKE_VERSION_MAJOR)
 CMAKE_LICENSE = BSD-3c
 CMAKE_LICENSE_FILES = Copyright.txt
 
+# CMake is a particular package:
+# * CMake can be built using the generic infrastructure or the cmake one.
+#   Since Buildroot has no requirement regarding the host system cmake
+#   program presence, it uses the generic infrastructure to build the
+#   host-cmake package, then the (target-)cmake package can be built
+#   using the cmake infrastructure;
+# * CMake bundles its dependencies within its sources. This is the
+#   reason why the host-cmake package has no dependencies:, whereas
+#   the (target-)cmake package has a lot of dependencies, using only
+#   the system-wide libraries instead of rebuilding and statically
+#   linking with the ones bundled into the CMake sources.
+
 HOST_CMAKE_DEPENDENCIES =
 CMAKE_DEPENDENCIES = zlib jsoncpp libcurl libarchive expat bzip2 xz
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Buildroot] [PATCH 3/3] Don't build host-cmake if it is available on the build host
  2016-07-01 15:53 [Buildroot] [PATCH 0/3] Skip host-cmake dependency to speedup builds Luca Ceresoli
  2016-07-01 15:53 ` [Buildroot] [PATCH 1/3] Move the host-pkgconf dependency from host-cmake to pkg-cmake Luca Ceresoli
  2016-07-01 15:53 ` [Buildroot] [PATCH 2/3] cmake: add documentation about how it is built Luca Ceresoli
@ 2016-07-01 15:53 ` Luca Ceresoli
  2016-07-02 14:18   ` Arnout Vandecappelle
  2 siblings, 1 reply; 15+ messages in thread
From: Luca Ceresoli @ 2016-07-01 15:53 UTC (permalink / raw)
  To: buildroot

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 introduce the possibility to skip entirely building host-cmake
and use the one on the system. However this feature has to be enabled
with a kconfig knob because it introduces problems in some
cases, described below.

In detail, we avoid building host-cmake if:
 - the knob is enabled and
 - 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_HOST_CMAKE environment variable, otherwise it defaults to
"cmake". If it is enabled, found and suitable then we set
USE_SYSTEM_HOST_CMAKE. Otherwise we override BR2_HOST_CMAKE with
"$(HOST_DIR)/usr/bin/cmake" to revert to the old behaviour.

Then in pkg-cmake.mk we launch $(BR2_HOST_CMAKE) instead of
$(HOST_DIR)/usr/bin/cmake.

Finally, we skip adding the dependency on host-cmake for all cmake
packages when $(USE_SYSTEM_HOST_CMAKE) = YES.

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.

check-host-cmake.sh requires CMake to be at least 3.0 to consider it
suitable. This is because older versions 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].

Besides, among all the cmake packages currently in Buildroot, the
highest version mentioned in cmake_minimum_required() is 3.0 (the
grantlee package). Thus 3.0 should be enough to build all current
packages. Of course, with the addition or bump of packages, the
minimum required version will raise.

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)

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

Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
Cc: Samuel Martin <s.martin49@gmail.com>
Cc: Davide Viti <zinosat@tiscali.it>
Cc: Arnout Vandecappelle <arnout@mind.be>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

---

Discussion:

There's an open point about this patch. Currently check-host-cmake.sh
only selects a cmake executable if it is >= 3.0, which is the highest
requirement among all Buildroot packages on current master. In the
future when bumping cmake-packages we should check whether they need a
more recent CMake and bump the required version. Doing that we will
silently disable the speedup for all users who upgrade Buildroot, even
if they do not use the specific package requiring a more recent CMake.

This issue can be handled in several ways:
 1. remove the version check entirely
 2. let the check as it is in this patch
 3. add a "minimum required version" kconfig parameter

Option 1 makes the Buildroot code simpler but breaks builds for people
using one of the mentioned packages. They can address the issue:
 - installing a more recent cmake (maybe in their $HOME, if they
   cannot do any better)
 - disabling BR2_TRY_SYSTEM_CMAKE

Option 2 is similar, except it silently disables the "speedup" feature
for known-too-old versions of cmake. This is bad because it does it
silently as discussed above. On the other hand this option handles the
use case where exactly the same configuration is built on several
machines with different distros, e.g. developer PC and CI server. The
system cmake will be used where suitable, built otherwise.

Option 3 means adding another parameter (and implement a proper
version number comparison). It would also force the user to take care
of setting it properly, which is easy however: if a package fails at
cmake_minimum_required(), raise the parameter. The advantage is that
users not using a specific package can keep the number lower, thus
enabling the speedup on more build hosts.

None of these options is totally satisfying. For this reason I added
the configration knob and kept it off by default. This forces the user
to think before enabling it, and to check whether he has a recent
enough cmake.

Any comments about the three possible alternatives is very welcome.

Results:

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!

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_HOST_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)
---
 Config.in                                | 19 +++++++++++++++++++
 package/pkg-cmake.mk                     |  6 ++++--
 support/dependencies/check-host-cmake.mk | 11 +++++++++++
 support/dependencies/check-host-cmake.sh | 30 ++++++++++++++++++++++++++++++
 4 files changed, 64 insertions(+), 2 deletions(-)
 create mode 100644 support/dependencies/check-host-cmake.mk
 create mode 100755 support/dependencies/check-host-cmake.sh

diff --git a/Config.in b/Config.in
index 820b1f0..58eaa79 100644
--- a/Config.in
+++ b/Config.in
@@ -357,6 +357,25 @@ config BR2_CCACHE_USE_BASEDIR
 
 endif
 
+config BR2_TRY_SYSTEM_CMAKE
+	bool "Try to use a CMake installed on the system"
+	help
+	  By default Buildroot builds CMake to run on the host
+	  (host-cmake) when any cmake-package is enabled. However
+	  building host-cmake takes a long time. If you have a
+	  suitable cmake on your build host you can use it by enabling
+	  this options: Buildroot will skip building host-cmake and
+	  use the system-provided one.
+
+	  By default Buildroot looks for an executable named "cmake"
+	  in your $PATH. You can specify another one by setting its
+	  path in the BR2_HOST_CMAKE environment variable.
+
+	  Note the system-provided CMake must be recent enough to
+	  support all enabled packages. Make sure it is no lower than
+	  the value passed to cmake_minimum_required() in the
+	  cmake-packages you use.
+
 config BR2_DEPRECATED
 	bool "Show options and packages that are deprecated or obsolete"
 	help
diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk
index 8d7d265..a96b9cd 100644
--- a/package/pkg-cmake.mk
+++ b/package/pkg-cmake.mk
@@ -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_HOST_CMAKE) $$($$(PKG)_SRCDIR) \
 		-DCMAKE_TOOLCHAIN_FILE="$$(HOST_DIR)/usr/share/buildroot/toolchainfile.cmake" \
 		-DCMAKE_BUILD_TYPE=$$(if $$(BR2_ENABLE_DEBUG),Debug,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_HOST_CMAKE) $$($$(PKG)_SRCDIR) \
 		-DCMAKE_INSTALL_SO_NO_EXE=0 \
 		-DCMAKE_FIND_ROOT_PATH="$$(HOST_DIR)" \
 		-DCMAKE_FIND_ROOT_PATH_MODE_PROGRAM="BOTH" \
@@ -153,7 +153,9 @@ endif
 # primitives to find {C,LD}FLAGS, add it to the dependency list.
 $(2)_DEPENDENCIES += host-pkgconf
 
+ifneq ($$(USE_SYSTEM_HOST_CMAKE),YES)
 $(2)_DEPENDENCIES += host-cmake
+endif
 
 #
 # Build step. Only define it if not already defined by the package .mk
diff --git a/support/dependencies/check-host-cmake.mk b/support/dependencies/check-host-cmake.mk
new file mode 100644
index 0000000..20ef81f
--- /dev/null
+++ b/support/dependencies/check-host-cmake.mk
@@ -0,0 +1,11 @@
+BR2_HOST_CMAKE ?= cmake
+
+ifeq ($(BR2_TRY_SYSTEM_CMAKE),y)
+ifneq (,$(call suitable-host-package,cmake,$(BR2_HOST_CMAKE)))
+USE_SYSTEM_HOST_CMAKE = YES
+endif
+endif
+
+ifneq ($(USE_SYSTEM_HOST_CMAKE),YES)
+BR2_HOST_CMAKE = $(HOST_DIR)/usr/bin/cmake
+endif
diff --git a/support/dependencies/check-host-cmake.sh b/support/dependencies/check-host-cmake.sh
new file mode 100755
index 0000000..08de60c
--- /dev/null
+++ b/support/dependencies/check-host-cmake.sh
@@ -0,0 +1,30 @@
+#!/bin/sh
+
+candidate="$1"
+
+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`
+
+# 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
+major_min=3
+minor_min=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
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Buildroot] [PATCH 1/3] Move the host-pkgconf dependency from host-cmake to pkg-cmake
  2016-07-01 15:53 ` [Buildroot] [PATCH 1/3] Move the host-pkgconf dependency from host-cmake to pkg-cmake Luca Ceresoli
@ 2016-07-02 13:56   ` Arnout Vandecappelle
  2016-07-02 14:44     ` Yann E. MORIN
  0 siblings, 1 reply; 15+ messages in thread
From: Arnout Vandecappelle @ 2016-07-02 13:56 UTC (permalink / raw)
  To: buildroot

On 01-07-16 17:53, Luca Ceresoli wrote:
> In 3d475ee0ba4d6eea6aca25594cfe5bb153ac804f a dependency on
> host-pkgconf was added to host-cmake. It is a workaround to fix build
> failures for packages that use pkgconf through a cmake module, but do
> not depend on host-pkgconf as they should.

 Wouldn't this be a great time to just bite the bullet, remove the host-pkgconf
dependency, and fixes packages as they break in the autobuilders?

> 
> Since it is the package that needs host-pkgconf and not host-cmake
> itself, move the dependency to the proper place, in pkg-cmake.mk.
> 
> Also copy the explanation on the mentioned commit as a comment in
> order to clarify why we do this.
> 
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>

 If it is decided to keep the current hack around:

Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Regards,
 Arnout

> Cc: Samuel Martin <s.martin49@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Davide Viti <zinosat@tiscali.it>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> 
> ---
> 
> This is done as a preliminary cleanup step for the following patch
> that allows to skip building host-cmake in some cases. The dependency
> where it is now would be missed when we skip building host-cmake, so I
> moved it where it will be catched in all cases. Without the following
> patch this can be considered just a cleanup without any visible
> effect.
> ---
>  package/cmake/cmake.mk | 2 +-
>  package/pkg-cmake.mk   | 4 ++++
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/package/cmake/cmake.mk b/package/cmake/cmake.mk
> index a776b14..33fb4aa 100644
> --- a/package/cmake/cmake.mk
> +++ b/package/cmake/cmake.mk
> @@ -10,7 +10,7 @@ CMAKE_SITE = https://cmake.org/files/v$(CMAKE_VERSION_MAJOR)
>  CMAKE_LICENSE = BSD-3c
>  CMAKE_LICENSE_FILES = Copyright.txt
>  
> -HOST_CMAKE_DEPENDENCIES = host-pkgconf
> +HOST_CMAKE_DEPENDENCIES =
>  CMAKE_DEPENDENCIES = zlib jsoncpp libcurl libarchive expat bzip2 xz
>  
>  CMAKE_CONF_OPTS = \
> diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk
> index 81dcfcc..8d7d265 100644
> --- a/package/pkg-cmake.mk
> +++ b/package/pkg-cmake.mk
> @@ -149,6 +149,10 @@ $(2)_DEPENDENCIES ?= $$(filter-out host-skeleton host-toolchain $(1),\
>  	$$(patsubst host-host-%,host-%,$$(addprefix host-,$$($(3)_DEPENDENCIES))))
>  endif
>  
> +# Since some CMake modules (even upstream ones) use pgk_check_modules
> +# primitives to find {C,LD}FLAGS, add it to the dependency list.
> +$(2)_DEPENDENCIES += host-pkgconf
> +
>  $(2)_DEPENDENCIES += host-cmake
>  
>  #
> 


-- 
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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Buildroot] [PATCH 2/3] cmake: add documentation about how it is built
  2016-07-01 15:53 ` [Buildroot] [PATCH 2/3] cmake: add documentation about how it is built Luca Ceresoli
@ 2016-07-02 13:57   ` Arnout Vandecappelle
  2016-07-02 14:48   ` Yann E. MORIN
  1 sibling, 0 replies; 15+ messages in thread
From: Arnout Vandecappelle @ 2016-07-02 13:57 UTC (permalink / raw)
  To: buildroot

On 01-07-16 17:53, Luca Ceresoli wrote:
> Commit 7b17bafc5d7948aff3059e058ada80ad1fc50500 by Davide Viti has a
> detailed explanation of some unusual techniques used for building
> host-cmake and (target-)cmake. This is useful information for whoever
> starts hacking on it, so copy it in the makefile, where it will be
> easily noticed.
> 
> Also remove the sentence about host-cmake having a runtime dependency
> on host-pkgconfig (not true anymore: it's the specific cmake-packages
> that depend on it) and fix typos.
> 
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>

Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Regards,
 Arnout

> Cc: Samuel Martin <s.martin49@gmail.com>
> Cc: Davide Viti <zinosat@tiscali.it>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  package/cmake/cmake.mk | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/package/cmake/cmake.mk b/package/cmake/cmake.mk
> index 33fb4aa..d734e03 100644
> --- a/package/cmake/cmake.mk
> +++ b/package/cmake/cmake.mk
> @@ -10,6 +10,18 @@ CMAKE_SITE = https://cmake.org/files/v$(CMAKE_VERSION_MAJOR)
>  CMAKE_LICENSE = BSD-3c
>  CMAKE_LICENSE_FILES = Copyright.txt
>  
> +# CMake is a particular package:
> +# * CMake can be built using the generic infrastructure or the cmake one.
> +#   Since Buildroot has no requirement regarding the host system cmake
> +#   program presence, it uses the generic infrastructure to build the
> +#   host-cmake package, then the (target-)cmake package can be built
> +#   using the cmake infrastructure;
> +# * CMake bundles its dependencies within its sources. This is the
> +#   reason why the host-cmake package has no dependencies:, whereas
> +#   the (target-)cmake package has a lot of dependencies, using only
> +#   the system-wide libraries instead of rebuilding and statically
> +#   linking with the ones bundled into the CMake sources.
> +
>  HOST_CMAKE_DEPENDENCIES =
>  CMAKE_DEPENDENCIES = zlib jsoncpp libcurl libarchive expat bzip2 xz
>  
> 


-- 
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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Buildroot] [PATCH 3/3] Don't build host-cmake if it is available on the build host
  2016-07-01 15:53 ` [Buildroot] [PATCH 3/3] Don't build host-cmake if it is available on the build host Luca Ceresoli
@ 2016-07-02 14:18   ` Arnout Vandecappelle
  2016-07-04 10:36     ` Luca Ceresoli
  0 siblings, 1 reply; 15+ messages in thread
From: Arnout Vandecappelle @ 2016-07-02 14:18 UTC (permalink / raw)
  To: buildroot

On 01-07-16 17:53, Luca Ceresoli wrote:
> 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 introduce the possibility to skip entirely building host-cmake
> and use the one on the system. However this feature has to be enabled
> with a kconfig knob because it introduces problems in some
> cases, described below.

 I don't find the description of those problems...

> 
> In detail, we avoid building host-cmake if:
>  - the knob is enabled and
>  - 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_HOST_CMAKE environment variable, otherwise it defaults to
> "cmake". If it is enabled, found and suitable then we set
> USE_SYSTEM_HOST_CMAKE. Otherwise we override BR2_HOST_CMAKE with
> "$(HOST_DIR)/usr/bin/cmake" to revert to the old behaviour.
> 
> Then in pkg-cmake.mk we launch $(BR2_HOST_CMAKE) instead of
> $(HOST_DIR)/usr/bin/cmake.
> 
> Finally, we skip adding the dependency on host-cmake for all cmake
> packages when $(USE_SYSTEM_HOST_CMAKE) = YES.
> 
> 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.
> 
> check-host-cmake.sh requires CMake to be at least 3.0 to consider it
> suitable. This is because older versions 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].
> 
> Besides, among all the cmake packages currently in Buildroot, the
> highest version mentioned in cmake_minimum_required() is 3.0 (the
> grantlee package). Thus 3.0 should be enough to build all current
> packages. Of course, with the addition or bump of packages, the
> minimum required version will raise.
> 
> 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)
> 
> [0] https://cmake.org/gitweb?p=cmake.git;h=e8b8b37ef6fef094940d3384df5a1d421b9fa568
> 
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> Cc: Samuel Martin <s.martin49@gmail.com>
> Cc: Davide Viti <zinosat@tiscali.it>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> 
> ---
> 
> Discussion:

 IMHO this discussion text should be part of the commit log. We typically look
at the commit log when we wonder why something is done in a particular way, and
this discussion gives exactly that kind of information.

> 
> There's an open point about this patch. Currently check-host-cmake.sh
> only selects a cmake executable if it is >= 3.0, which is the highest
> requirement among all Buildroot packages on current master. In the
> future when bumping cmake-packages we should check whether they need a
> more recent CMake and bump the required version. Doing that we will
> silently disable the speedup for all users who upgrade Buildroot, even
> if they do not use the specific package requiring a more recent CMake.
> 
> This issue can be handled in several ways:
>  1. remove the version check entirely
>  2. let the check as it is in this patch
>  3. add a "minimum required version" kconfig parameter
> 
> Option 1 makes the Buildroot code simpler but breaks builds for people
> using one of the mentioned packages. They can address the issue:
>  - installing a more recent cmake (maybe in their $HOME, if they
>    cannot do any better)
>  - disabling BR2_TRY_SYSTEM_CMAKE

 Ah, this is the problem you are talking about? So basically,
BR2_TRY_SYSTEM_CMAKE is only relevant if the version check is removed?

> 
> Option 2 is similar, except it silently disables the "speedup" feature
> for known-too-old versions of cmake. This is bad because it does it
> silently as discussed above. On the other hand this option handles the
> use case where exactly the same configuration is built on several
> machines with different distros, e.g. developer PC and CI server. The
> system cmake will be used where suitable, built otherwise.
> 
> Option 3 means adding another parameter (and implement a proper
> version number comparison). It would also force the user to take care
> of setting it properly, which is easy however: if a package fails at
> cmake_minimum_required(), raise the parameter. The advantage is that
> users not using a specific package can keep the number lower, thus
> enabling the speedup on more build hosts.

 There could also be a BR2_PACKAGE_HOST_CMAKE_MIN_VERSION_X_Y that could be
selected by individual packages, and used in the check-host-cmake script to
evaluate the installed cmake version. However, it is not entirely trivial to
check the correctness of this in the autobuilders.

 For me, option 2 is sufficient. There may indeed be a build speed regression
when bumping buildroot with the same config. But since bumping buildroot also
means that package versions have been bumped, this could also be due to the new
package version requiring a more recent cmake. In addition, a build speed
regression I don't seriously consider a regression. Actually, for me, almost
every buildroot commit is a speed regression because bash-completion becomes
slower with each additional package :-)

 So, in my opinion: ditch BR2_TRY_SYSTEM_CMAKE.

> 
> None of these options is totally satisfying. For this reason I added
> the configration knob and kept it off by default. This forces the user
> to think before enabling it, and to check whether he has a recent
> enough cmake.
> 
> Any comments about the three possible alternatives is very welcome.
[snip]
> diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk
> index 8d7d265..a96b9cd 100644
> --- a/package/pkg-cmake.mk
> +++ b/package/pkg-cmake.mk
> @@ -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_HOST_CMAKE) $$($$(PKG)_SRCDIR) \
>  		-DCMAKE_TOOLCHAIN_FILE="$$(HOST_DIR)/usr/share/buildroot/toolchainfile.cmake" \
>  		-DCMAKE_BUILD_TYPE=$$(if $$(BR2_ENABLE_DEBUG),Debug,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_HOST_CMAKE) $$($$(PKG)_SRCDIR) \
>  		-DCMAKE_INSTALL_SO_NO_EXE=0 \
>  		-DCMAKE_FIND_ROOT_PATH="$$(HOST_DIR)" \
>  		-DCMAKE_FIND_ROOT_PATH_MODE_PROGRAM="BOTH" \
> @@ -153,7 +153,9 @@ endif
>  # primitives to find {C,LD}FLAGS, add it to the dependency list.
>  $(2)_DEPENDENCIES += host-pkgconf
>  
> +ifneq ($$(USE_SYSTEM_HOST_CMAKE),YES)

 We typically talk about host-foo and system-foo, so this should be
USE_SYSTEM_CMAKE.

>  $(2)_DEPENDENCIES += host-cmake
> +endif
>  
>  #
>  # Build step. Only define it if not already defined by the package .mk
> diff --git a/support/dependencies/check-host-cmake.mk b/support/dependencies/check-host-cmake.mk
> new file mode 100644
> index 0000000..20ef81f
> --- /dev/null
> +++ b/support/dependencies/check-host-cmake.mk
> @@ -0,0 +1,11 @@
> +BR2_HOST_CMAKE ?= cmake
> +
> +ifeq ($(BR2_TRY_SYSTEM_CMAKE),y)
> +ifneq (,$(call suitable-host-package,cmake,$(BR2_HOST_CMAKE)))
> +USE_SYSTEM_HOST_CMAKE = YES
> +endif
> +endif
> +
> +ifneq ($(USE_SYSTEM_HOST_CMAKE),YES)

 If BR2_TRY_SYSTEM_CMAKE is ditched, this can just be the else branch.


 Regards,
 Arnout

> +BR2_HOST_CMAKE = $(HOST_DIR)/usr/bin/cmake
> +endif
> diff --git a/support/dependencies/check-host-cmake.sh b/support/dependencies/check-host-cmake.sh
> new file mode 100755
> index 0000000..08de60c
> --- /dev/null
> +++ b/support/dependencies/check-host-cmake.sh
> @@ -0,0 +1,30 @@
> +#!/bin/sh
> +
> +candidate="$1"
> +
> +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`
> +
> +# 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
> +major_min=3
> +minor_min=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
> 


-- 
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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Buildroot] [PATCH 1/3] Move the host-pkgconf dependency from host-cmake to pkg-cmake
  2016-07-02 13:56   ` Arnout Vandecappelle
@ 2016-07-02 14:44     ` Yann E. MORIN
  2016-07-02 14:52       ` Arnout Vandecappelle
  0 siblings, 1 reply; 15+ messages in thread
From: Yann E. MORIN @ 2016-07-02 14:44 UTC (permalink / raw)
  To: buildroot

Arnout, All,

On 2016-07-02 15:56 +0200, Arnout Vandecappelle spake thusly:
> On 01-07-16 17:53, Luca Ceresoli wrote:
> > In 3d475ee0ba4d6eea6aca25594cfe5bb153ac804f a dependency on
> > host-pkgconf was added to host-cmake. It is a workaround to fix build
> > failures for packages that use pkgconf through a cmake module, but do
> > not depend on host-pkgconf as they should.
> 
>  Wouldn't this be a great time to just bite the bullet, remove the host-pkgconf
> dependency, and fixes packages as they break in the autobuilders?

We've jsut discussed it live here in TLS, and we've concluded that this
is a topic unrelated to the work by Luca.

So even if we wanted to remove this "hack", this should not block this
patch from going in.

However, because the culprit for causing the dependency to host-pkgconf
is cmake itslef, and not a cmake-based package, I would suggest that we
do not consider this a hack, but the reality.

Anyway, for this patch:

Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Regards,
Yann E. MORIN.

> > Since it is the package that needs host-pkgconf and not host-cmake
> > itself, move the dependency to the proper place, in pkg-cmake.mk.
> > 
> > Also copy the explanation on the mentioned commit as a comment in
> > order to clarify why we do this.
> > 
> > Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> 
>  If it is decided to keep the current hack around:
> 
> Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> 
>  Regards,
>  Arnout
> 
> > Cc: Samuel Martin <s.martin49@gmail.com>
> > Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> > Cc: Davide Viti <zinosat@tiscali.it>
> > Cc: Arnout Vandecappelle <arnout@mind.be>
> > 
> > ---
> > 
> > This is done as a preliminary cleanup step for the following patch
> > that allows to skip building host-cmake in some cases. The dependency
> > where it is now would be missed when we skip building host-cmake, so I
> > moved it where it will be catched in all cases. Without the following
> > patch this can be considered just a cleanup without any visible
> > effect.
> > ---
> >  package/cmake/cmake.mk | 2 +-
> >  package/pkg-cmake.mk   | 4 ++++
> >  2 files changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/package/cmake/cmake.mk b/package/cmake/cmake.mk
> > index a776b14..33fb4aa 100644
> > --- a/package/cmake/cmake.mk
> > +++ b/package/cmake/cmake.mk
> > @@ -10,7 +10,7 @@ CMAKE_SITE = https://cmake.org/files/v$(CMAKE_VERSION_MAJOR)
> >  CMAKE_LICENSE = BSD-3c
> >  CMAKE_LICENSE_FILES = Copyright.txt
> >  
> > -HOST_CMAKE_DEPENDENCIES = host-pkgconf
> > +HOST_CMAKE_DEPENDENCIES =
> >  CMAKE_DEPENDENCIES = zlib jsoncpp libcurl libarchive expat bzip2 xz
> >  
> >  CMAKE_CONF_OPTS = \
> > diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk
> > index 81dcfcc..8d7d265 100644
> > --- a/package/pkg-cmake.mk
> > +++ b/package/pkg-cmake.mk
> > @@ -149,6 +149,10 @@ $(2)_DEPENDENCIES ?= $$(filter-out host-skeleton host-toolchain $(1),\
> >  	$$(patsubst host-host-%,host-%,$$(addprefix host-,$$($(3)_DEPENDENCIES))))
> >  endif
> >  
> > +# Since some CMake modules (even upstream ones) use pgk_check_modules
> > +# primitives to find {C,LD}FLAGS, add it to the dependency list.
> > +$(2)_DEPENDENCIES += host-pkgconf
> > +
> >  $(2)_DEPENDENCIES += host-cmake
> >  
> >  #
> > 
> 
> 
> -- 
> 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
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Buildroot] [PATCH 2/3] cmake: add documentation about how it is built
  2016-07-01 15:53 ` [Buildroot] [PATCH 2/3] cmake: add documentation about how it is built Luca Ceresoli
  2016-07-02 13:57   ` Arnout Vandecappelle
@ 2016-07-02 14:48   ` Yann E. MORIN
  1 sibling, 0 replies; 15+ messages in thread
From: Yann E. MORIN @ 2016-07-02 14:48 UTC (permalink / raw)
  To: buildroot

Luca, All,

On 2016-07-01 17:53 +0200, Luca Ceresoli spake thusly:
> Commit 7b17bafc5d7948aff3059e058ada80ad1fc50500 by Davide Viti has a
> detailed explanation of some unusual techniques used for building
> host-cmake and (target-)cmake. This is useful information for whoever
> starts hacking on it, so copy it in the makefile, where it will be
> easily noticed.
> 
> Also remove the sentence about host-cmake having a runtime dependency
> on host-pkgconfig (not true anymore: it's the specific cmake-packages
> that depend on it) and fix typos.
> 
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> Cc: Samuel Martin <s.martin49@gmail.com>
> Cc: Davide Viti <zinosat@tiscali.it>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Regards,
Yann E. MORIN.

> ---
>  package/cmake/cmake.mk | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/package/cmake/cmake.mk b/package/cmake/cmake.mk
> index 33fb4aa..d734e03 100644
> --- a/package/cmake/cmake.mk
> +++ b/package/cmake/cmake.mk
> @@ -10,6 +10,18 @@ CMAKE_SITE = https://cmake.org/files/v$(CMAKE_VERSION_MAJOR)
>  CMAKE_LICENSE = BSD-3c
>  CMAKE_LICENSE_FILES = Copyright.txt
>  
> +# CMake is a particular package:
> +# * CMake can be built using the generic infrastructure or the cmake one.
> +#   Since Buildroot has no requirement regarding the host system cmake
> +#   program presence, it uses the generic infrastructure to build the
> +#   host-cmake package, then the (target-)cmake package can be built
> +#   using the cmake infrastructure;
> +# * CMake bundles its dependencies within its sources. This is the
> +#   reason why the host-cmake package has no dependencies:, whereas
> +#   the (target-)cmake package has a lot of dependencies, using only
> +#   the system-wide libraries instead of rebuilding and statically
> +#   linking with the ones bundled into the CMake sources.
> +
>  HOST_CMAKE_DEPENDENCIES =
>  CMAKE_DEPENDENCIES = zlib jsoncpp libcurl libarchive expat bzip2 xz
>  
> -- 
> 2.7.4
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Buildroot] [PATCH 1/3] Move the host-pkgconf dependency from host-cmake to pkg-cmake
  2016-07-02 14:44     ` Yann E. MORIN
@ 2016-07-02 14:52       ` Arnout Vandecappelle
  2016-07-02 15:43         ` Yann E. MORIN
  0 siblings, 1 reply; 15+ messages in thread
From: Arnout Vandecappelle @ 2016-07-02 14:52 UTC (permalink / raw)
  To: buildroot



On 02-07-16 16:44, Yann E. MORIN wrote:
> However, because the culprit for causing the dependency to host-pkgconf
> is cmake itslef, and not a cmake-based package, I would suggest that we
> do not consider this a hack, but the reality.

 Isn't the problem that package foo uses FindBar from cmake, which uses
pkg-config to find the package? For me, the burden of depending on host-pkgconf
is on package foo in that case.

 That said, this hack really simplifies things so let's indeed keep it.

 Actually, why don't we just always build host-pkgconf? On my machine 'make
host-pkgconf' takes 6.5 seconds, of which 4.7 seconds are parsing the makefiles.
Can we afford the cost of 1.5 seconds of build time and 844K of build size?

 Regards,
 Arnout

-- 
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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Buildroot] [PATCH 1/3] Move the host-pkgconf dependency from host-cmake to pkg-cmake
  2016-07-02 14:52       ` Arnout Vandecappelle
@ 2016-07-02 15:43         ` Yann E. MORIN
  2016-07-04  9:23           ` Luca Ceresoli
  0 siblings, 1 reply; 15+ messages in thread
From: Yann E. MORIN @ 2016-07-02 15:43 UTC (permalink / raw)
  To: buildroot

Arnout, All,

On 2016-07-02 16:52 +0200, Arnout Vandecappelle spake thusly:
> On 02-07-16 16:44, Yann E. MORIN wrote:
> > However, because the culprit for causing the dependency to host-pkgconf
> > is cmake itslef, and not a cmake-based package, I would suggest that we
> > do not consider this a hack, but the reality.
> 
>  Isn't the problem that package foo uses FindBar from cmake, which uses
> pkg-config to find the package? For me, the burden of depending on host-pkgconf
> is on package foo in that case.

I don't agree. ;-) The use of pkg-config is an internal detail of the
CMake modules, of which the package should have no knowledge.

>  That said, this hack really simplifies things so let's indeed keep it.

On which I'll close the discussion. Thanks! :-)

>  Actually, why don't we just always build host-pkgconf? On my machine 'make
> host-pkgconf' takes 6.5 seconds, of which 4.7 seconds are parsing the makefiles.
> Can we afford the cost of 1.5 seconds of build time and 844K of build size?

If we decide so, then we should move this non-hack into the
generic-package infra, so that it benefits all packages at once, not
only cmake-based packages.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Buildroot] [PATCH 1/3] Move the host-pkgconf dependency from host-cmake to pkg-cmake
  2016-07-02 15:43         ` Yann E. MORIN
@ 2016-07-04  9:23           ` Luca Ceresoli
  0 siblings, 0 replies; 15+ messages in thread
From: Luca Ceresoli @ 2016-07-04  9:23 UTC (permalink / raw)
  To: buildroot

Yann, Arnout,

On 02/07/2016 17:43, Yann E. MORIN wrote:
> Arnout, All,
> 
> On 2016-07-02 16:52 +0200, Arnout Vandecappelle spake thusly:
>> On 02-07-16 16:44, Yann E. MORIN wrote:
>>> However, because the culprit for causing the dependency to host-pkgconf
>>> is cmake itslef, and not a cmake-based package, I would suggest that we
>>> do not consider this a hack, but the reality.
>>
>>  Isn't the problem that package foo uses FindBar from cmake, which uses
>> pkg-config to find the package? For me, the burden of depending on host-pkgconf
>> is on package foo in that case.
> 
> I don't agree. ;-) The use of pkg-config is an internal detail of the
> CMake modules, of which the package should have no knowledge.

Good point, you're right. FindBar might even be not using pkgconf in the
current CMake release, and start using it in a future release. As long
as it's provided among the CMake built-in modules, this should be
considered a CMake implementation detail.

Thus my comment to the patch should be changed, but the patch content is
still needed in case we skip building host-cmake. Either in the current
form...

> 
>>  That said, this hack really simplifies things so let's indeed keep it.
> 
> On which I'll close the discussion. Thanks! :-)
> 
>>  Actually, why don't we just always build host-pkgconf? On my machine 'make
>> host-pkgconf' takes 6.5 seconds, of which 4.7 seconds are parsing the makefiles.
>> Can we afford the cost of 1.5 seconds of build time and 844K of build size?

...or in the variant suggested by Arnout.

> If we decide so, then we should move this non-hack into the
> generic-package infra, so that it benefits all packages at once, not
> only cmake-based packages.

This might add an unneeded host-pkgconf build in a some corner cases,
thus it would be once again a "hack". But its cost is so negligible that
I would definitely consider it.

-- 
Luca

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Buildroot] [PATCH 3/3] Don't build host-cmake if it is available on the build host
  2016-07-02 14:18   ` Arnout Vandecappelle
@ 2016-07-04 10:36     ` Luca Ceresoli
  2016-07-05  9:21       ` Arnout Vandecappelle
  0 siblings, 1 reply; 15+ messages in thread
From: Luca Ceresoli @ 2016-07-04 10:36 UTC (permalink / raw)
  To: buildroot

Arnout,

thanks for your comments! Se my replies below.

On 02/07/2016 16:18, Arnout Vandecappelle wrote:
> On 01-07-16 17:53, Luca Ceresoli wrote:
[...]
>> Discussion:
> 
>  IMHO this discussion text should be part of the commit log. We typically look
> at the commit log when we wonder why something is done in a particular way, and
> this discussion gives exactly that kind of information.
> 
>>
>> There's an open point about this patch. Currently check-host-cmake.sh
>> only selects a cmake executable if it is >= 3.0, which is the highest
>> requirement among all Buildroot packages on current master. In the
>> future when bumping cmake-packages we should check whether they need a
>> more recent CMake and bump the required version. Doing that we will
>> silently disable the speedup for all users who upgrade Buildroot, even
>> if they do not use the specific package requiring a more recent CMake.
>>
>> This issue can be handled in several ways:
>>  1. remove the version check entirely
>>  2. let the check as it is in this patch
>>  3. add a "minimum required version" kconfig parameter
>>
>> Option 1 makes the Buildroot code simpler but breaks builds for people
>> using one of the mentioned packages. They can address the issue:
>>  - installing a more recent cmake (maybe in their $HOME, if they
>>    cannot do any better)
>>  - disabling BR2_TRY_SYSTEM_CMAKE
> 
>  Ah, this is the problem you are talking about? So basically,
> BR2_TRY_SYSTEM_CMAKE is only relevant if the version check is removed?

Technically you're right. But for a goor "user experience", I'd like
BR2_TRY_SYSTEM_CMAKE to be there at least to hold docs about this
feature. In case 2 the user should be aware that the speedup might not
exist and might disappear bumping Buildroot, why, and how to re-enable it.

I also would give the users the option to opt out, in case there's any
suspect problems with the system cmake.

>> Option 2 is similar, except it silently disables the "speedup" feature
>> for known-too-old versions of cmake. This is bad because it does it
>> silently as discussed above. On the other hand this option handles the
>> use case where exactly the same configuration is built on several
>> machines with different distros, e.g. developer PC and CI server. The
>> system cmake will be used where suitable, built otherwise.
>>
>> Option 3 means adding another parameter (and implement a proper
>> version number comparison). It would also force the user to take care
>> of setting it properly, which is easy however: if a package fails at
>> cmake_minimum_required(), raise the parameter. The advantage is that
>> users not using a specific package can keep the number lower, thus
>> enabling the speedup on more build hosts.
> 
>  There could also be a BR2_PACKAGE_HOST_CMAKE_MIN_VERSION_X_Y that could be
> selected by individual packages, and used in the check-host-cmake script to
> evaluate the installed cmake version. However, it is not entirely trivial to
> check the correctness of this in the autobuilders.

This would be the most complete solution. However, no matter how complex
it is to implement and test, it still adds a burden on the shoulders of
contributors wanting to "simply" bump a package version. I'm trying to
avoid that.

>  For me, option 2 is sufficient. There may indeed be a build speed regression
> when bumping buildroot with the same config. But since bumping buildroot also
> means that package versions have been bumped, this could also be due to the new
> package version requiring a more recent cmake. In addition, a build speed
> regression I don't seriously consider a regression. Actually, for me, almost
> every buildroot commit is a speed regression because bash-completion becomes
> slower with each additional package :-)

Thanks for your vote! :)

I just want to make sure you understand this can disable the speedup
_without_ any real need. Take this use case:

- in BR 2016.08 we bump package foo to 9.9, which needs CMake 4.0
- the user bumps to BR 2016.08 when it's released
- the user uses Ubuntu 16.04 LTS on his PC, which ships CMake 3.5
- the user does not use package foo

This user's builds will fail in finding a suitable system-cmake and will
build host-cmake, which is not strictly needed because all enabled
packages are happy with CMake <= 3.5.

I'm trying to support users not willing do modify the Buildroot code
(e.g. using only BR2_EXTERNAL to add their own salt). With the current
patch, these users could not touch the minimum required version. They
would have no way to re-enable the speedup from Buildroot, they'd need
to act on the "environment", by installing a newer CMake.

[...]
>> +ifneq ($$(USE_SYSTEM_HOST_CMAKE),YES)
> 
>  We typically talk about host-foo and system-foo, so this should be
> USE_SYSTEM_CMAKE.

Right, will fix!

-- 
Luca

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Buildroot] [PATCH 3/3] Don't build host-cmake if it is available on the build host
  2016-07-04 10:36     ` Luca Ceresoli
@ 2016-07-05  9:21       ` Arnout Vandecappelle
  2016-07-16 20:32         ` Luca Ceresoli
  0 siblings, 1 reply; 15+ messages in thread
From: Arnout Vandecappelle @ 2016-07-05  9:21 UTC (permalink / raw)
  To: buildroot



On 04-07-16 12:36, Luca Ceresoli wrote:
> Arnout,
> 
> thanks for your comments! Se my replies below.
> 
> On 02/07/2016 16:18, Arnout Vandecappelle wrote:
>> On 01-07-16 17:53, Luca Ceresoli wrote:
[snip]
>>> Option 3 means adding another parameter (and implement a proper
>>> version number comparison). It would also force the user to take care
>>> of setting it properly, which is easy however: if a package fails at
>>> cmake_minimum_required(), raise the parameter. The advantage is that
>>> users not using a specific package can keep the number lower, thus
>>> enabling the speedup on more build hosts.
>>
>>  There could also be a BR2_PACKAGE_HOST_CMAKE_MIN_VERSION_X_Y that could be
>> selected by individual packages, and used in the check-host-cmake script to
>> evaluate the installed cmake version. However, it is not entirely trivial to
>> check the correctness of this in the autobuilders.
> 
> This would be the most complete solution. However, no matter how complex
> it is to implement and test, it still adds a burden on the shoulders of
> contributors wanting to "simply" bump a package version. I'm trying to
> avoid that.

 I didn't make this clear enough: I absolutely don't like the idea of using
BR2_PACKAGE_HOST_CMAKE_MIN_VERSION_X_Y, because it adds a lot of complexity and
maintainance burden with limited gains.

> 
>>  For me, option 2 is sufficient. There may indeed be a build speed regression
>> when bumping buildroot with the same config. But since bumping buildroot also
>> means that package versions have been bumped, this could also be due to the new
>> package version requiring a more recent cmake. In addition, a build speed
>> regression I don't seriously consider a regression. Actually, for me, almost
>> every buildroot commit is a speed regression because bash-completion becomes
>> slower with each additional package :-)
> 
> Thanks for your vote! :)
> 
> I just want to make sure you understand this can disable the speedup
> _without_ any real need. Take this use case:
> 
> - in BR 2016.08 we bump package foo to 9.9, which needs CMake 4.0
> - the user bumps to BR 2016.08 when it's released
> - the user uses Ubuntu 16.04 LTS on his PC, which ships CMake 3.5
> - the user does not use package foo
> 
> This user's builds will fail in finding a suitable system-cmake and will
> build host-cmake, which is not strictly needed because all enabled
> packages are happy with CMake <= 3.5.

 Yes, I understand that this is possible. And my response is: I don't care. We
did the same (though with lower impact) when we started requiring tar >= 1.17. I
don't think a simple speedup is important enough to warrant additional
user-visible complexity. Given that it is just a speed-up, I don't think
complicated help text is needed either.

 So I really would like to remove the BR2_TRY_SYSTEM_CMAKE option. First of all,
it obviously doesn't help for the speed regression issue: if you set that to y,
you will still see the speed regression. Supposedly you would have read the help
text and know that this is possible, but typically a buildroot bump will happen
months or years after the configuration was first done, so in all likelihood the
user has forgotten all about it.

 The only real reason to keep the option is to work around a problem where the
system cmake seems to be OK but isn't. Since we have various cmake versions in
the autobuilders, the risk should be limited there. And if a user really has a
broken system cmake, you can still set BR2_HOST_CMAKE=missing to override it. It
does make sense to put that in the troubleshooting section of the manual
(together with xzcat and tar).

 Regards,
 Arnout


> I'm trying to support users not willing do modify the Buildroot code
> (e.g. using only BR2_EXTERNAL to add their own salt). With the current
> patch, these users could not touch the minimum required version. They
> would have no way to re-enable the speedup from Buildroot, they'd need
> to act on the "environment", by installing a newer CMake.
> 
> [...]
>>> +ifneq ($$(USE_SYSTEM_HOST_CMAKE),YES)
>>
>>  We typically talk about host-foo and system-foo, so this should be
>> USE_SYSTEM_CMAKE.
> 
> Right, will fix!
> 

-- 
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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Buildroot] [PATCH 3/3] Don't build host-cmake if it is available on the build host
  2016-07-05  9:21       ` Arnout Vandecappelle
@ 2016-07-16 20:32         ` Luca Ceresoli
  0 siblings, 0 replies; 15+ messages in thread
From: Luca Ceresoli @ 2016-07-16 20:32 UTC (permalink / raw)
  To: buildroot

Dear Arnout,

On 05/07/2016 11:21, Arnout Vandecappelle wrote:
[snip]
>>>> Option 3 means adding another parameter (and implement a proper
>>>> version number comparison). It would also force the user to take care
>>>> of setting it properly, which is easy however: if a package fails at
>>>> cmake_minimum_required(), raise the parameter. The advantage is that
>>>> users not using a specific package can keep the number lower, thus
>>>> enabling the speedup on more build hosts.
>>>
>>>  There could also be a BR2_PACKAGE_HOST_CMAKE_MIN_VERSION_X_Y that could be
>>> selected by individual packages, and used in the check-host-cmake script to
>>> evaluate the installed cmake version. However, it is not entirely trivial to
>>> check the correctness of this in the autobuilders.
>>
>> This would be the most complete solution. However, no matter how complex
>> it is to implement and test, it still adds a burden on the shoulders of
>> contributors wanting to "simply" bump a package version. I'm trying to
>> avoid that.
> 
>  I didn't make this clear enough: I absolutely don't like the idea of using
> BR2_PACKAGE_HOST_CMAKE_MIN_VERSION_X_Y, because it adds a lot of complexity and
> maintainance burden with limited gains.

I couldn't agree more.

>>>  For me, option 2 is sufficient. There may indeed be a build speed regression
>>> when bumping buildroot with the same config. But since bumping buildroot also
>>> means that package versions have been bumped, this could also be due to the new
>>> package version requiring a more recent cmake. In addition, a build speed
>>> regression I don't seriously consider a regression. Actually, for me, almost
>>> every buildroot commit is a speed regression because bash-completion becomes
>>> slower with each additional package :-)
>>
>> Thanks for your vote! :)

Guess what, you won the poll! v4 is coming with option 2 implemented.
Which means it will be equivalent to v2, except for cleanups.

>> I just want to make sure you understand this can disable the speedup
>> _without_ any real need. Take this use case:
>>
>> - in BR 2016.08 we bump package foo to 9.9, which needs CMake 4.0
>> - the user bumps to BR 2016.08 when it's released
>> - the user uses Ubuntu 16.04 LTS on his PC, which ships CMake 3.5
>> - the user does not use package foo
>>
>> This user's builds will fail in finding a suitable system-cmake and will
>> build host-cmake, which is not strictly needed because all enabled
>> packages are happy with CMake <= 3.5.
> 
>  Yes, I understand that this is possible. And my response is: I don't care. We
> did the same (though with lower impact) when we started requiring tar >= 1.17. I
> don't think a simple speedup is important enough to warrant additional
> user-visible complexity. Given that it is just a speed-up, I don't think
> complicated help text is needed either.
> 
>  So I really would like to remove the BR2_TRY_SYSTEM_CMAKE option. First of all,
> it obviously doesn't help for the speed regression issue: if you set that to y,
> you will still see the speed regression. Supposedly you would have read the help
> text and know that this is possible, but typically a buildroot bump will happen
> months or years after the configuration was first done, so in all likelihood the
> user has forgotten all about it.

Although I don't 100% agree with you on the theoretical side, I have
come to think your approach is the best to all practical effects. And it
looks like most packages are fine with a relatively old version of
CMake, which makes my concerns much less relevant. Finally, we can add
more complexity later, should we realize it's needed instead.

-- 
Luca

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2016-07-16 20:32 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-07-01 15:53 [Buildroot] [PATCH 0/3] Skip host-cmake dependency to speedup builds Luca Ceresoli
2016-07-01 15:53 ` [Buildroot] [PATCH 1/3] Move the host-pkgconf dependency from host-cmake to pkg-cmake Luca Ceresoli
2016-07-02 13:56   ` Arnout Vandecappelle
2016-07-02 14:44     ` Yann E. MORIN
2016-07-02 14:52       ` Arnout Vandecappelle
2016-07-02 15:43         ` Yann E. MORIN
2016-07-04  9:23           ` Luca Ceresoli
2016-07-01 15:53 ` [Buildroot] [PATCH 2/3] cmake: add documentation about how it is built Luca Ceresoli
2016-07-02 13:57   ` Arnout Vandecappelle
2016-07-02 14:48   ` Yann E. MORIN
2016-07-01 15:53 ` [Buildroot] [PATCH 3/3] Don't build host-cmake if it is available on the build host Luca Ceresoli
2016-07-02 14:18   ` Arnout Vandecappelle
2016-07-04 10:36     ` Luca Ceresoli
2016-07-05  9:21       ` Arnout Vandecappelle
2016-07-16 20:32         ` Luca Ceresoli

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox