| Message ID | 1454670476-20635-1-git-send-email-luca@lucaceresoli.net |
|---|---|
| State | Changes Requested |
| Headers | show |
Luca, All, On 2016-02-05 12:07 +0100, Luca Ceresoli spake thusly: > 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. > > 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. [--SNIP--] > 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. Would it make sense to have the cmake-package infra check for that? > --- > Note: there is still a pending clarification with Arnout about this > patch. Which is? ;-) [--SNIP--] > 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 At first, I was a bit worried that we use just 'CMAKE' as the variable name. But in retrospect, it seems you want to allow the user to specify his own locally-installed cmake, right? Is so, maybe you could add that to the manual (chapter 8.6, Environment variables). > +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..08de60c > --- /dev/null > +++ b/support/dependencies/check-host-cmake.sh > @@ -0,0 +1,30 @@ > +#!/bin/sh > + > +candidate="$1" > + > +cmake=`which $candidate` For good measure, redirect stderr to /dev/null. 'which' is not supposed to spit out anything on stderrm even when the program is not found, but still, there might be rogue implenentations of which in the wild... > +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 Damn, I would have suggested you make it a single condition: if [ ${major} -gt ${major_min} -o \ ${major} -eq ${major_min} -a ${minor} -ge ${minor_min} ]; then But since what you did is what is already done for asciidoc and tar, I guess that's OK. Yet, I prefer we use ${..} to expand variables, it is /cleaner/... Regards, Yann E. MORIN. > + echo $cmake > + else > + # echo nothing: no suitable cmake found > + exit 1 > + fi > +fi > -- > 1.9.1 > > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
Dear Yann, Yann E. MORIN wrote: > Luca, All, > > On 2016-02-05 12:07 +0100, Luca Ceresoli spake thusly: >> 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. >> >> 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. > [--SNIP--] >> 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. > > Would it make sense to have the cmake-package infra check for that? Sure it would. But I don't know how to do it properly. Simply grepping CMakeLists.txt is not reliable: some packages do cmake_minimum_required(VERSION ${SOME_COMPUTED_VARIABLE}) Probably the only solid way to obtain the minimum required version would be to launch cmake and let it tell us. But I had a very quick look and I didn't find any way to ask it such information. >> Note: there is still a pending clarification with Arnout about this >> patch. > > Which is? ;-) ...which is now solved. :) It was only a misunderstanding: http://lists.busybox.net/pipermail/buildroot/2016-February/151981.html > [--SNIP--] >> 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 > > At first, I was a bit worried that we use just 'CMAKE' as the variable > name. > > But in retrospect, it seems you want to allow the user to specify his > own locally-installed cmake, right? Is so, maybe you could add that to > the manual (chapter 8.6, Environment variables). "you want to allow" is an overstatement. What happened is that I lazily copied check-host-tar.sh, which happens to have the same construct. I don't know whether it was done on purpose there, but I realized being able to do 'make CMAKE=/my/dirty/cmake' might be useful, e.g. if one is hacking cmake itself and wants to use a specific version without tweaking PATH. >> +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..08de60c >> --- /dev/null >> +++ b/support/dependencies/check-host-cmake.sh >> @@ -0,0 +1,30 @@ >> +#!/bin/sh >> + >> +candidate="$1" >> + >> +cmake=`which $candidate` > > For good measure, redirect stderr to /dev/null. 'which' is not supposed > to spit out anything on stderrm even when the program is not found, but > still, there might be rogue implenentations of which in the wild... Yeah, there might be a 'witch' out there in the wild. :) Enlightened by your shell wisdom, I will add the redirect. >> +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 > > Damn, I would have suggested you make it a single condition: > > if [ ${major} -gt ${major_min} -o \ > ${major} -eq ${major_min} -a ${minor} -ge ${minor_min} ]; then It's nicer and cleaner. However it would produce fairly long lines when comparing three or more numbers. E.g. for asciidoc we want at least version >= 8.6.3. I wonder whether it would make sense to extract a shell function to compare a version number and reuse it in all check-host-*.sh, and possibly elsewhere. But that's for another day. > But since what you did is what is already done for asciidoc and tar, I > guess that's OK. > > Yet, I prefer we use ${..} to expand variables, it is /cleaner/... Sure, will do.
On 09-02-16 00:48, Luca Ceresoli wrote: > Dear Yann, > > Yann E. MORIN wrote: >> Luca, All, >> >> On 2016-02-05 12:07 +0100, Luca Ceresoli spake thusly: [snip] >>> 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. >> >> Would it make sense to have the cmake-package infra check for that? > > Sure it would. But I don't know how to do it properly. > > Simply grepping CMakeLists.txt is not reliable: some packages do > > cmake_minimum_required(VERSION ${SOME_COMPUTED_VARIABLE}) > > Probably the only solid way to obtain the minimum required version would > be to launch cmake and let it tell us. But I had a very quick look and > I didn't find any way to ask it such information. I think the best way to check is to make sure there is an autobuilder with cmake 3.0. Of course, when we bump the minimum required version, that autobuilder should be bumped as well. Thomas, do you see this feasible? [snip] >>> 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 >> >> At first, I was a bit worried that we use just 'CMAKE' as the variable >> name. >> >> But in retrospect, it seems you want to allow the user to specify his >> own locally-installed cmake, right? Is so, maybe you could add that to >> the manual (chapter 8.6, Environment variables). > > "you want to allow" is an overstatement. What happened is that I lazily > copied check-host-tar.sh, which happens to have the same construct. I > don't know whether it was done on purpose there, but I realized being > able to do 'make CMAKE=/my/dirty/cmake' might be useful, e.g. if one > is hacking cmake itself and wants to use a specific version without > tweaking PATH. Like, for example, on an autobuilder with cmake 3.0 :-) [snip] >>> +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 >> >> Damn, I would have suggested you make it a single condition: >> >> if [ ${major} -gt ${major_min} -o \ >> ${major} -eq ${major_min} -a ${minor} -ge ${minor_min} ]; then > > It's nicer and cleaner. However it would produce fairly long lines when > comparing three or more numbers. E.g. for asciidoc we want at least > version >= 8.6.3. > > I wonder whether it would make sense to extract a shell function to > compare a version number and reuse it in all check-host-*.sh, and > possibly elsewhere. But that's for another day. It's something that would fit nicely in the concept of 'shell modules' that Samuel introduces as part of the fix-rpath series [1]. Regards, Arnout [1] http://patchwork.ozlabs.org/patch/576597/ > >> But since what you did is what is already done for asciidoc and tar, I >> guess that's OK. >> >> Yet, I prefer we use ${..} to expand variables, it is /cleaner/... > > Sure, will do. >
Hi Arnout, On 09/02/2016 23:13, Arnout Vandecappelle wrote: > [snip] >>>> 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. >>> >>> Would it make sense to have the cmake-package infra check for that? >> >> Sure it would. But I don't know how to do it properly. >> >> Simply grepping CMakeLists.txt is not reliable: some packages do >> >> cmake_minimum_required(VERSION ${SOME_COMPUTED_VARIABLE}) >> >> Probably the only solid way to obtain the minimum required version would >> be to launch cmake and let it tell us. But I had a very quick look and >> I didn't find any way to ask it such information. > > I think the best way to check is to make sure there is an autobuilder with > cmake 3.0. Of course, when we bump the minimum required version, that > autobuilder should be bumped as well. Thomas, do you see this feasible? That would be great! > [snip] >>>> 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 >>> >>> At first, I was a bit worried that we use just 'CMAKE' as the variable >>> name. >>> >>> But in retrospect, it seems you want to allow the user to specify his >>> own locally-installed cmake, right? Is so, maybe you could add that to >>> the manual (chapter 8.6, Environment variables). >> >> "you want to allow" is an overstatement. What happened is that I lazily >> copied check-host-tar.sh, which happens to have the same construct. I >> don't know whether it was done on purpose there, but I realized being >> able to do 'make CMAKE=/my/dirty/cmake' might be useful, e.g. if one >> is hacking cmake itself and wants to use a specific version without >> tweaking PATH. > > Like, for example, on an autobuilder with cmake 3.0 :-) Sure!! :-) > [snip] >>>> +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 >>> >>> Damn, I would have suggested you make it a single condition: >>> >>> if [ ${major} -gt ${major_min} -o \ >>> ${major} -eq ${major_min} -a ${minor} -ge ${minor_min} ]; then >> >> It's nicer and cleaner. However it would produce fairly long lines when >> comparing three or more numbers. E.g. for asciidoc we want at least >> version >= 8.6.3. >> >> I wonder whether it would make sense to extract a shell function to >> compare a version number and reuse it in all check-host-*.sh, and >> possibly elsewhere. But that's for another day. > > It's something that would fit nicely in the concept of 'shell modules' that > Samuel introduces as part of the fix-rpath series [1]. Hey, cool stuff! Sorry, I hadn't had a look at that patch yet. However, since it it not yet applied, I'll keep the code as is for now and will take care of updating when it will be official.
Hi Luca, all, On Fri, Feb 5, 2016 at 12:07 PM, Luca Ceresoli <luca@lucaceresoli.net> 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. > > 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 and use 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. Sorry for not spotting this earlier, but: host-cmake has a dependency on host-pkgconf, which cmake (the host program) may use at runtime to find the dependencies when it is configuring a project. In the BR host tree, pkg-config is a wrapper with environment variables correctly preset to find stuff in the sysroot tree by default. Here is a test case: A is a cmake-package depending on B. B is a generic-package providing a *.pc file. CMake uses its FindPkgConfig module to detect B. So, currently we got: - A depends on: B and host-cmake - host-cmake depends on host-pkgconf - When configuring A, host-cmake should use host-pkgconf to find B With this patch, in the case cmake build is skipped, we get: - A depends on: B - When configuring A, cmake (from the host system) may use pkg-config (from the host system) to find B, but this pkg-config won't have the env. vars. correctly set to find B in the sysroot. In the best case, it won't find B, in the worst case it will try to link against B from the host system. I'm not sure what it the best and clean way to handle this... Maybe $(BUILD_HOST_CMAKE) != YES should be use to noop the HOST_CMAKE_{CONFIGURE,BUILD,INSTALLL}_CMDS commands, but we keep the dependency on host-cmake, so we don't lose host-cmake's dependencies. Any thought about this? > > 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) > > [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> > [1] https://cmake.org/cmake/help/v3.4/module/FindPkgConfig.html Regards,
On 16-02-16 14:55, Samuel Martin wrote: > Hi Luca, all, > > On Fri, Feb 5, 2016 at 12:07 PM, Luca Ceresoli <luca@lucaceresoli.net> wrote: [snip] >> > Finally, we do skip adding the dependency on host-cmake for all cmake >> > packages when $(BUILD_HOST_CMAKE) != YES. > Sorry for not spotting this earlier, but: > host-cmake has a dependency on host-pkgconf, which cmake (the host > program) may use at runtime to find the dependencies when it is > configuring a project. > In the BR host tree, pkg-config is a wrapper with environment > variables correctly preset to find stuff in the sysroot tree by > default. > > Here is a test case: > > A is a cmake-package depending on B. > B is a generic-package providing a *.pc file. > CMake uses its FindPkgConfig module to detect B. > > So, currently we got: > - A depends on: B and host-cmake > - host-cmake depends on host-pkgconf > - When configuring A, host-cmake should use host-pkgconf to find B > > With this patch, in the case cmake build is skipped, we get: > - A depends on: B > - When configuring A, cmake (from the host system) may use pkg-config > (from the host system) to find B, but this pkg-config won't have the > env. vars. correctly set to find B in the sysroot. In the best case, > it won't find B, in the worst case it will try to link against B from > the host system. > > I'm not sure what it the best and clean way to handle this... > Maybe $(BUILD_HOST_CMAKE) != YES should be use to noop the > HOST_CMAKE_{CONFIGURE,BUILD,INSTALLL}_CMDS commands, but we keep the > dependency on host-cmake, so we don't lose host-cmake's dependencies. > > Any thought about this? The correct approach would be to add explicit dependencies on host-pkgconf for packages that need it, just like we do for autotools packages. The difficulty is of course finding out which packages need host-pkgconf. It seems it's not as simple as searching for FindPkgConfig in the CMakeLists.txt files of each cmake package, because there are other modules that implicitly use this. So we'd have to make a list of all those modules and check for all of them. And then of course there are the packages that install new cmake modules that use this, but which don't use FindPkgConfig (or even cmake) themselves... In 3d475ee0 you added the unconditional host-pkgconf dependency to work around this problem. If we remove that dependency, the autobuilders should detect the missing host-pkgconf dependencies even when host-cmake is used. Regards, Arnout [snip]
Hi Samuel, Arnout, On 16/02/2016 16:08, Arnout Vandecappelle wrote: > > > On 16-02-16 14:55, Samuel Martin wrote: >> Hi Luca, all, >> >> On Fri, Feb 5, 2016 at 12:07 PM, Luca Ceresoli <luca@lucaceresoli.net> wrote: > [snip] >>>> Finally, we do skip adding the dependency on host-cmake for all cmake >>>> packages when $(BUILD_HOST_CMAKE) != YES. >> Sorry for not spotting this earlier, but: >> host-cmake has a dependency on host-pkgconf, which cmake (the host >> program) may use at runtime to find the dependencies when it is >> configuring a project. >> In the BR host tree, pkg-config is a wrapper with environment >> variables correctly preset to find stuff in the sysroot tree by >> default. >> >> Here is a test case: >> >> A is a cmake-package depending on B. >> B is a generic-package providing a *.pc file. >> CMake uses its FindPkgConfig module to detect B. >> >> So, currently we got: >> - A depends on: B and host-cmake >> - host-cmake depends on host-pkgconf >> - When configuring A, host-cmake should use host-pkgconf to find B >> >> With this patch, in the case cmake build is skipped, we get: >> - A depends on: B >> - When configuring A, cmake (from the host system) may use pkg-config >> (from the host system) to find B, but this pkg-config won't have the >> env. vars. correctly set to find B in the sysroot. In the best case, >> it won't find B, in the worst case it will try to link against B from >> the host system. Thanks for the insight. I wonder whether we can do anything in toolchainfile.cmake to prevent cmake from using the pkg-config on the system. Don't the following lines already do it? [Disclaimer: I'm in a hurry and checking the docs _now_ would take too long] set(CMAKE_PROGRAM_PATH "${RELOCATED_HOST_DIR}/usr/bin") set(CMAKE_FIND_ROOT_PATH "${RELOCATED_HOST_DIR}/@@STAGING_SUBDIR@@") set(CMAKE_FIND_ROOT_PATH_MODE_PROGRAM NEVER) set(CMAKE_FIND_ROOT_PATH_MODE_PACKAGE ONLY) set(CMAKE_FIND_ROOT_PATH_MODE_LIBRARY ONLY) set(CMAKE_FIND_ROOT_PATH_MODE_INCLUDE ONLY) >> >> I'm not sure what it the best and clean way to handle this... >> Maybe $(BUILD_HOST_CMAKE) != YES should be use to noop the >> HOST_CMAKE_{CONFIGURE,BUILD,INSTALLL}_CMDS commands, but we keep the >> dependency on host-cmake, so we don't lose host-cmake's dependencies. >> >> Any thought about this? > > The correct approach would be to add explicit dependencies on host-pkgconf for > packages that need it, just like we do for autotools packages. > > The difficulty is of course finding out which packages need host-pkgconf. It > seems it's not as simple as searching for FindPkgConfig in the CMakeLists.txt > files of each cmake package, because there are other modules that implicitly use > this. So we'd have to make a list of all those modules and check for all of > them. And then of course there are the packages that install new cmake modules > that use this, but which don't use FindPkgConfig (or even cmake) themselves... > > In 3d475ee0 you added the unconditional host-pkgconf dependency to work around > this problem. If we remove that dependency, the autobuilders should detect the > missing host-pkgconf dependencies even when host-cmake is used. The workaround Samuel added in that commit is in package/cmake/cmake.mk: +HOST_CMAKE_DEPENDENCIES = host-pkgconf This could probably ported to my patch in package/pkg-cmake.mk: +ifeq ($$(BUILD_HOST_CMAKE),YES) $(2)_DEPENDENCIES += host-cmake +else +$(2)_DEPENDENCIES += host-pkgconf +endif But I agree with Arnout that if a package actually uses pkg-conf, it should depend on it. Of course this means it must fail when the dependency is missing, not use the system pkg-conf.
On 16-02-16 17:53, Luca Ceresoli wrote: > Hi Samuel, Arnout, > > On 16/02/2016 16:08, Arnout Vandecappelle wrote: >> >> >> On 16-02-16 14:55, Samuel Martin wrote: >>> Hi Luca, all, >>> >>> On Fri, Feb 5, 2016 at 12:07 PM, Luca Ceresoli <luca@lucaceresoli.net> wrote: >> [snip] >>>>> Finally, we do skip adding the dependency on host-cmake for all cmake >>>>> packages when $(BUILD_HOST_CMAKE) != YES. >>> Sorry for not spotting this earlier, but: >>> host-cmake has a dependency on host-pkgconf, which cmake (the host >>> program) may use at runtime to find the dependencies when it is >>> configuring a project. >>> In the BR host tree, pkg-config is a wrapper with environment >>> variables correctly preset to find stuff in the sysroot tree by >>> default. >>> >>> Here is a test case: >>> >>> A is a cmake-package depending on B. >>> B is a generic-package providing a *.pc file. >>> CMake uses its FindPkgConfig module to detect B. >>> >>> So, currently we got: >>> - A depends on: B and host-cmake >>> - host-cmake depends on host-pkgconf >>> - When configuring A, host-cmake should use host-pkgconf to find B >>> >>> With this patch, in the case cmake build is skipped, we get: >>> - A depends on: B >>> - When configuring A, cmake (from the host system) may use pkg-config >>> (from the host system) to find B, but this pkg-config won't have the >>> env. vars. correctly set to find B in the sysroot. In the best case, >>> it won't find B, in the worst case it will try to link against B from >>> the host system. > > Thanks for the insight. > > I wonder whether we can do anything in toolchainfile.cmake to prevent > cmake from using the pkg-config on the system. Don't the following lines > already do it? [Disclaimer: I'm in a hurry and checking the docs _now_ > would take too long] > > set(CMAKE_PROGRAM_PATH "${RELOCATED_HOST_DIR}/usr/bin") > set(CMAKE_FIND_ROOT_PATH "${RELOCATED_HOST_DIR}/@@STAGING_SUBDIR@@") > set(CMAKE_FIND_ROOT_PATH_MODE_PROGRAM NEVER) > set(CMAKE_FIND_ROOT_PATH_MODE_PACKAGE ONLY) > set(CMAKE_FIND_ROOT_PATH_MODE_LIBRARY ONLY) > set(CMAKE_FIND_ROOT_PATH_MODE_INCLUDE ONLY) This will add the host dir to the program path, but not stop it from searching /usr/bin. Maybe we should always put a fake pkg-config script in $(HOST_DIR)/usr/bin that always returns false. This script will be overwritten by host-pkgconf, so anything that tries to use it without depending on host-pkgconf will get a failure, and the pkg-config from /usr/bin/ is completely avoided. Well, this script should be smart enough to still work for host packages... >>> >>> I'm not sure what it the best and clean way to handle this... >>> Maybe $(BUILD_HOST_CMAKE) != YES should be use to noop the >>> HOST_CMAKE_{CONFIGURE,BUILD,INSTALLL}_CMDS commands, but we keep the >>> dependency on host-cmake, so we don't lose host-cmake's dependencies. >>> >>> Any thought about this? >> >> The correct approach would be to add explicit dependencies on host-pkgconf for >> packages that need it, just like we do for autotools packages. >> >> The difficulty is of course finding out which packages need host-pkgconf. It >> seems it's not as simple as searching for FindPkgConfig in the CMakeLists.txt >> files of each cmake package, because there are other modules that implicitly use >> this. So we'd have to make a list of all those modules and check for all of >> them. And then of course there are the packages that install new cmake modules >> that use this, but which don't use FindPkgConfig (or even cmake) themselves... >> >> In 3d475ee0 you added the unconditional host-pkgconf dependency to work around >> this problem. If we remove that dependency, the autobuilders should detect the >> missing host-pkgconf dependencies even when host-cmake is used. > > The workaround Samuel added in that commit is in package/cmake/cmake.mk: > > +HOST_CMAKE_DEPENDENCIES = host-pkgconf > > This could probably ported to my patch in package/pkg-cmake.mk: > > +ifeq ($$(BUILD_HOST_CMAKE),YES) > $(2)_DEPENDENCIES += host-cmake > +else > +$(2)_DEPENDENCIES += host-pkgconf > +endif > > But I agree with Arnout that if a package actually uses pkg-conf, it > should depend on it. Of course this means it must fail when the > dependency is missing, not use the system pkg-conf. Ack that. Regards, Arnout
Hi, On 16/02/2016 17:53, Luca Ceresoli wrote: [...] >>> >>> I'm not sure what it the best and clean way to handle this... >>> Maybe $(BUILD_HOST_CMAKE) != YES should be use to noop the >>> HOST_CMAKE_{CONFIGURE,BUILD,INSTALLL}_CMDS commands, but we keep the >>> dependency on host-cmake, so we don't lose host-cmake's dependencies. >>> >>> Any thought about this? >> >> The correct approach would be to add explicit dependencies on host-pkgconf for >> packages that need it, just like we do for autotools packages. >> >> The difficulty is of course finding out which packages need host-pkgconf. It >> seems it's not as simple as searching for FindPkgConfig in the CMakeLists.txt >> files of each cmake package, because there are other modules that implicitly use >> this. So we'd have to make a list of all those modules and check for all of >> them. And then of course there are the packages that install new cmake modules >> that use this, but which don't use FindPkgConfig (or even cmake) themselves... >> >> In 3d475ee0 you added the unconditional host-pkgconf dependency to work around >> this problem. If we remove that dependency, the autobuilders should detect the >> missing host-pkgconf dependencies even when host-cmake is used. > > The workaround Samuel added in that commit is in package/cmake/cmake.mk: > > +HOST_CMAKE_DEPENDENCIES = host-pkgconf > > This could probably ported to my patch in package/pkg-cmake.mk: > > +ifeq ($$(BUILD_HOST_CMAKE),YES) > $(2)_DEPENDENCIES += host-cmake > +else > +$(2)_DEPENDENCIES += host-pkgconf > +endif I'm reviving this old patch, and after speaking with Thomas I chose the above strategy. It solves the issue raised by Samuel, thus my patch can work properly. It does not fix the specific packages depending on host-pkgconf, but that's a separate issue so I left it for another series. v3 on the way.
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..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
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. 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 and use 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. 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) [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> --- Note: there is still a pending clarification with Arnout about this patch. I'm sending it anyway since v1 is broken, and this one has improvements in addition to the bugfix. 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 v1 -> v2: - Require cmake >= 3.0. Fixes qjson failure (Luca, Samuel, Thomas) - In check-host-cmake.sh only search $1, not "cmake" (Arnout) - typo: host-ccache -> host-cmake (Arnout) --- package/pkg-cmake.mk | 6 ++++-- support/dependencies/check-host-cmake.mk | 6 ++++++ support/dependencies/check-host-cmake.sh | 30 ++++++++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 2 deletions(-) create mode 100644 support/dependencies/check-host-cmake.mk create mode 100755 support/dependencies/check-host-cmake.sh