Message ID | 1454363063-21105-1-git-send-email-luca@lucaceresoli.net |
---|---|
State | Changes Requested |
Headers | show |
On 01-02-16 22:44, 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-ccache is not very useful if it is already host-ccache -> host-cmake > installed on the build host: it is supposed to be quite portable, and > the only patch we have for cmake seems to only affect target-cmake. > > We avoid building host-cmake if cmake is already available on the host > using a technique similar to the one used for host-tar and host-xzcat. > > First, we leverage the existing infrastructure in > support/dependencies/dependencies.mk to set CMAKE to either "cmake" or > "$(HOST_DIR)/usr/bin/cmake". In the latter case we also set > BUILD_HOST_CMAKE = YES meaning we need to build host-cmake. > > Then in pkg-cmake.mk we launch $(CMAKE) instead of > $(HOST_DIR)/usr/bin/cmake. > > Finally, we do skip adding the dependency on host-cmake for all cmake > packages when $(BUILD_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. Otherwise host-cmake > would be a dependency for _any_ package when it's not installed on the > host, even if no cmake package is selected. Nice and long commit message, thanks! > > Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net> > Cc: Samuel Martin <s.martin49@gmail.com> > Cc: Davide Viti <zinosat@tiscali.it> > --- > package/pkg-cmake.mk | 6 ++++-- > support/dependencies/check-host-cmake.mk | 6 ++++++ > support/dependencies/check-host-cmake.sh | 14 ++++++++++++++ > 3 files changed, 24 insertions(+), 2 deletions(-) > create mode 100644 support/dependencies/check-host-cmake.mk > create mode 100755 support/dependencies/check-host-cmake.sh > > diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk > index 81dcfcc..e3bd603 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) $$(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) \ > + $$(CMAKE) $$($$(PKG)_SRCDIR) \ > -DCMAKE_INSTALL_SO_NO_EXE=0 \ > -DCMAKE_FIND_ROOT_PATH="$$(HOST_DIR)" \ > -DCMAKE_FIND_ROOT_PATH_MODE_PROGRAM="BOTH" \ > @@ -149,7 +149,9 @@ $(2)_DEPENDENCIES ?= $$(filter-out host-skeleton host-toolchain $(1),\ > $$(patsubst host-host-%,host-%,$$(addprefix host-,$$($(3)_DEPENDENCIES)))) > endif > > +ifeq ($$(BUILD_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..fe16322 > --- /dev/null > +++ b/support/dependencies/check-host-cmake.mk > @@ -0,0 +1,6 @@ > +CMAKE ?= cmake > + > +ifeq (,$(call suitable-host-package,cmake,$(CMAKE))) > +BUILD_HOST_CMAKE = YES > +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..76a81e0 > --- /dev/null > +++ b/support/dependencies/check-host-cmake.sh > @@ -0,0 +1,14 @@ > +#!/bin/sh > + > +candidate="$1" > + > +cmake=`which $candidate` $candidate could be an absolute path, in which case which returns nothing. > +if [ ! -x "$cmake" ]; then > + cmake=`which cmake` Since there is already CMAKE ?= cmake in the caller, this is pointless. How about a for cmake in "$cmake" "$( which "$cmake" )"; do ... instead? > + if [ ! -x "$cmake" ]; then > + # echo nothing: no suitable cmake found > + exit 1 > + fi > +fi I have the feeling that there should also be a version check here, though I don't know which version we should use. Regards, Arnout > + > +echo $cmake >
Hi Arnout, thanks for the review. Unfortunately this patch is not working, at least for qjson with qt4. qjson fails with: CMake Error in src/CMakeLists.txt: Imported target "Qt4::QtCore" includes non-existent path "QT_MKSPECS_DIR-NOTFOUND/default" in its INTERFACE_INCLUDE_DIRECTORIES. Possible reasons include: * The path was deleted, renamed, or moved to another location. * An install or uninstall procedure did not complete successfully. * The installation package was faulty and references files it does not provide. I am investigating right now. Below my replies to your comments. Arnout Vandecappelle wrote: > On 01-02-16 22:44, 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-ccache is not very useful if it is already > > host-ccache -> host-cmake Oops. :) > >> installed on the build host: it is supposed to be quite portable, and >> the only patch we have for cmake seems to only affect target-cmake. >> >> We avoid building host-cmake if cmake is already available on the host >> using a technique similar to the one used for host-tar and host-xzcat. >> >> First, we leverage the existing infrastructure in >> support/dependencies/dependencies.mk to set CMAKE to either "cmake" or >> "$(HOST_DIR)/usr/bin/cmake". In the latter case we also set >> BUILD_HOST_CMAKE = YES meaning we need to build host-cmake. >> >> Then in pkg-cmake.mk we launch $(CMAKE) instead of >> $(HOST_DIR)/usr/bin/cmake. >> >> Finally, we do skip adding the dependency on host-cmake for all cmake >> packages when $(BUILD_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. Otherwise host-cmake >> would be a dependency for _any_ package when it's not installed on the >> host, even if no cmake package is selected. > > Nice and long commit message, thanks! > >> >> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net> >> Cc: Samuel Martin <s.martin49@gmail.com> >> Cc: Davide Viti <zinosat@tiscali.it> >> --- >> package/pkg-cmake.mk | 6 ++++-- >> support/dependencies/check-host-cmake.mk | 6 ++++++ >> support/dependencies/check-host-cmake.sh | 14 ++++++++++++++ >> 3 files changed, 24 insertions(+), 2 deletions(-) >> create mode 100644 support/dependencies/check-host-cmake.mk >> create mode 100755 support/dependencies/check-host-cmake.sh >> >> diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk >> index 81dcfcc..e3bd603 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) $$(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) \ >> + $$(CMAKE) $$($$(PKG)_SRCDIR) \ >> -DCMAKE_INSTALL_SO_NO_EXE=0 \ >> -DCMAKE_FIND_ROOT_PATH="$$(HOST_DIR)" \ >> -DCMAKE_FIND_ROOT_PATH_MODE_PROGRAM="BOTH" \ >> @@ -149,7 +149,9 @@ $(2)_DEPENDENCIES ?= $$(filter-out host-skeleton host-toolchain $(1),\ >> $$(patsubst host-host-%,host-%,$$(addprefix host-,$$($(3)_DEPENDENCIES)))) >> endif >> >> +ifeq ($$(BUILD_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..fe16322 >> --- /dev/null >> +++ b/support/dependencies/check-host-cmake.mk >> @@ -0,0 +1,6 @@ >> +CMAKE ?= cmake >> + >> +ifeq (,$(call suitable-host-package,cmake,$(CMAKE))) >> +BUILD_HOST_CMAKE = YES >> +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..76a81e0 >> --- /dev/null >> +++ b/support/dependencies/check-host-cmake.sh >> @@ -0,0 +1,14 @@ >> +#!/bin/sh >> + >> +candidate="$1" >> + >> +cmake=`which $candidate` > > $candidate could be an absolute path, in which case which returns nothing. > >> +if [ ! -x "$cmake" ]; then >> + cmake=`which cmake` > > Since there is already CMAKE ?= cmake in the caller, this is pointless. > > How about a > > for cmake in "$cmake" "$( which "$cmake" )"; do > ... > > instead? Your suggestions are absolutely correct. I shamelessly copied other check-host-*.sh scripts and just concentrated on having the thing working. Seems like thos other script need a cleanup now. I'll have a look (if/after I can make this thing working). > >> + if [ ! -x "$cmake" ]; then >> + # echo nothing: no suitable cmake found >> + exit 1 >> + fi >> +fi > > I have the feeling that there should also be a version check here, though I > don't know which version we should use. Right. My idea is to download all the cmake packages that are currently in Buildroot, find the highest version mentioned in their CMAKE_MINIMUM_REQUIRED() statement and check for that version in the script.
Hi Arnout, Luca Ceresoli wrote: > Hi Arnout, > > thanks for the review. > > Unfortunately this patch is not working, at least for qjson with qt4. It's fixed now (thanks to Samuel Martin for the help). So it's now time for the refinements. [...] >>> new file mode 100644 >>> index 0000000..fe16322 >>> --- /dev/null >>> +++ b/support/dependencies/check-host-cmake.mk >>> @@ -0,0 +1,6 @@ >>> +CMAKE ?= cmake >>> + >>> +ifeq (,$(call suitable-host-package,cmake,$(CMAKE))) >>> +BUILD_HOST_CMAKE = YES >>> +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..76a81e0 >>> --- /dev/null >>> +++ b/support/dependencies/check-host-cmake.sh >>> @@ -0,0 +1,14 @@ >>> +#!/bin/sh >>> + >>> +candidate="$1" >>> + >>> +cmake=`which $candidate` >> >> $candidate could be an absolute path, in which case which returns >> nothing. Not sure I got what you mean, sorry. which + absolute path returns the absolute path itself, if it exists: $ which cmake /usr/bin/cmake $ which /usr/bin/cmake /usr/bin/cmake $ which /not/quite/cmake $ And the which(1) manpage confirms this is correct. >>> +if [ ! -x "$cmake" ]; then >>> + cmake=`which cmake` >> >> Since there is already CMAKE ?= cmake in the caller, this is pointless. Aah, yes. Removing the second if will still allow to override CMAKE ('make CMAKE=/my/custom/cmake qjson') but if it does not exist it will not search for one in the path. Sounds good. >> How about a >> >> for cmake in "$cmake" "$( which "$cmake" )"; do >> ... >> >> instead? Given the above discussion about which(1), I think this is useless. >>> + if [ ! -x "$cmake" ]; then >>> + # echo nothing: no suitable cmake found >>> + exit 1 >>> + fi >>> +fi >> >> I have the feeling that there should also be a version check here, >> though I >> don't know which version we should use. > > Right. My idea is to download all the cmake packages that are currently > in Buildroot, find the highest version mentioned in their > CMAKE_MINIMUM_REQUIRED() statement and check for that version in the > script. I added it, will be in v2. This was absolutely needed to make it work on a Ubuntu 14.04 LTS host. More details will be in the commit log (which will be even longer, ooh yeah).
On 04-02-16 14:37, Luca Ceresoli wrote: > Hi Arnout, > > Luca Ceresoli wrote: >>>> +cmake=`which $candidate` >>> >>> $candidate could be an absolute path, in which case which returns >>> nothing. > > Not sure I got what you mean, sorry. which + absolute path returns the > absolute path itself, if it exists: > > $ which cmake > /usr/bin/cmake > $ which /usr/bin/cmake > /usr/bin/cmake > $ which /not/quite/cmake > $ > > And the which(1) manpage confirms this is correct. > Oops, my bad, I tested it with a file that wasn't executable... Regards, Arnout [snip]
diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk index 81dcfcc..e3bd603 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) $$(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) \ + $$(CMAKE) $$($$(PKG)_SRCDIR) \ -DCMAKE_INSTALL_SO_NO_EXE=0 \ -DCMAKE_FIND_ROOT_PATH="$$(HOST_DIR)" \ -DCMAKE_FIND_ROOT_PATH_MODE_PROGRAM="BOTH" \ @@ -149,7 +149,9 @@ $(2)_DEPENDENCIES ?= $$(filter-out host-skeleton host-toolchain $(1),\ $$(patsubst host-host-%,host-%,$$(addprefix host-,$$($(3)_DEPENDENCIES)))) endif +ifeq ($$(BUILD_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..fe16322 --- /dev/null +++ b/support/dependencies/check-host-cmake.mk @@ -0,0 +1,6 @@ +CMAKE ?= cmake + +ifeq (,$(call suitable-host-package,cmake,$(CMAKE))) +BUILD_HOST_CMAKE = YES +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..76a81e0 --- /dev/null +++ b/support/dependencies/check-host-cmake.sh @@ -0,0 +1,14 @@ +#!/bin/sh + +candidate="$1" + +cmake=`which $candidate` +if [ ! -x "$cmake" ]; then + cmake=`which cmake` + if [ ! -x "$cmake" ]; then + # echo nothing: no suitable cmake found + exit 1 + fi +fi + +echo $cmake
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-ccache is not very useful if it is already installed on the build host: it is supposed to be quite portable, and the only patch we have for cmake seems to only affect target-cmake. We avoid building host-cmake if cmake is already available on the host using a technique similar to the one used for host-tar and host-xzcat. First, we leverage the existing infrastructure in support/dependencies/dependencies.mk to set CMAKE to either "cmake" or "$(HOST_DIR)/usr/bin/cmake". In the latter case we also set BUILD_HOST_CMAKE = YES meaning we need to build host-cmake. Then in pkg-cmake.mk we launch $(CMAKE) instead of $(HOST_DIR)/usr/bin/cmake. Finally, we do skip adding the dependency on host-cmake for all cmake packages when $(BUILD_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. Otherwise host-cmake would be a dependency for _any_ package when it's not installed on the host, even if no cmake package is selected. Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net> Cc: Samuel Martin <s.martin49@gmail.com> Cc: Davide Viti <zinosat@tiscali.it> --- package/pkg-cmake.mk | 6 ++++-- support/dependencies/check-host-cmake.mk | 6 ++++++ support/dependencies/check-host-cmake.sh | 14 ++++++++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) create mode 100644 support/dependencies/check-host-cmake.mk create mode 100755 support/dependencies/check-host-cmake.sh