Message ID | 20171201205352.24287-5-thomas.petazzoni@free-electrons.com |
---|---|
State | Superseded |
Headers | show |
Series | Per-package host/target directory support | expand |
Thomas, All, On 2017-12-01 21:53 +0100, Thomas Petazzoni spake thusly: > Currently, the toolchainfile.cmake and Buildroot.cmake files are > installed outside of any package, just triggered by the toolchain > target. > > As part of the per-package SDK effort, we are trying to avoid anything > that installs to the global $(HOST_DIR), and this is one of the > remaining files installed in $(HOST_DIR) outside of any package. We > fix this by installing such files as part of the toolchain package > post-install staging hooks. > > Yes, a post-install staging hook to install things to $(HOST_DIR) is a > bit weird, but the toolchain infrastructure is made of target packages > only, and they all install a lot of stuff to $(HOST_DIR) already. What I am more concerned about, is that the cmake package is registering a hook for another package, toolchain. Have you tried to run check-package on that? Regards, Yann E. MORIN. > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > --- > Changes since v2: > - None > Changes since v1: > - New patch > --- > package/pkg-cmake.mk | 12 +++++++----- > toolchain/toolchain/toolchain.mk | 3 --- > 2 files changed, 7 insertions(+), 8 deletions(-) > > diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk > index 6739704e3c..6d4f948d07 100644 > --- a/package/pkg-cmake.mk > +++ b/package/pkg-cmake.mk > @@ -244,8 +244,8 @@ endif > # based on the toolchainfile.cmake file's location: $(HOST_DIR)/share/buildroot > # In all the other variables, HOST_DIR will be replaced by RELOCATED_HOST_DIR, > # so we have to strip "$(HOST_DIR)/" from the paths that contain it. > -$(HOST_DIR)/share/buildroot/toolchainfile.cmake: > - @mkdir -p $(@D) > +define TOOLCHAIN_CMAKE_INSTALL_FILES > + @mkdir -p $(HOST_DIR)/share/buildroot > sed \ > -e 's#@@STAGING_SUBDIR@@#$(call qstrip,$(STAGING_SUBDIR))#' \ > -e 's#@@TARGET_CFLAGS@@#$(call qstrip,$(TARGET_CFLAGS))#' \ > @@ -259,7 +259,9 @@ $(HOST_DIR)/share/buildroot/toolchainfile.cmake: > -e 's#@@TOOLCHAIN_HAS_FORTRAN@@#$(if $(BR2_TOOLCHAIN_HAS_FORTRAN),1,0)#' \ > -e 's#@@CMAKE_BUILD_TYPE@@#$(if $(BR2_ENABLE_DEBUG),Debug,Release)#' \ > $(TOPDIR)/support/misc/toolchainfile.cmake.in \ > - > $@ > + > $(HOST_DIR)/share/buildroot/toolchainfile.cmake > + $(Q)$(INSTALL) -D -m 0644 support/misc/Buildroot.cmake \ > + $(HOST_DIR)/share/buildroot/Platform/Buildroot.cmake > +endef > > -$(HOST_DIR)/share/buildroot/Platform/Buildroot.cmake: > - $(Q)$(INSTALL) -D -m 0644 support/misc/Buildroot.cmake $(@) > +TOOLCHAIN_POST_INSTALL_STAGING_HOOKS += TOOLCHAIN_CMAKE_INSTALL_FILES > diff --git a/toolchain/toolchain/toolchain.mk b/toolchain/toolchain/toolchain.mk > index b55b0c712c..283e0a74ee 100644 > --- a/toolchain/toolchain/toolchain.mk > +++ b/toolchain/toolchain/toolchain.mk > @@ -38,6 +38,3 @@ TOOLCHAIN_INSTALL_STAGING = YES > endif > > $(eval $(virtual-package)) > - > -toolchain: $(HOST_DIR)/share/buildroot/toolchainfile.cmake > -toolchain: $(HOST_DIR)/share/buildroot/Platform/Buildroot.cmake > -- > 2.13.6 >
Hello, On Sun, 3 Dec 2017 23:34:54 +0100, Yann E. MORIN wrote: > On 2017-12-01 21:53 +0100, Thomas Petazzoni spake thusly: > > Currently, the toolchainfile.cmake and Buildroot.cmake files are > > installed outside of any package, just triggered by the toolchain > > target. > > > > As part of the per-package SDK effort, we are trying to avoid anything > > that installs to the global $(HOST_DIR), and this is one of the > > remaining files installed in $(HOST_DIR) outside of any package. We > > fix this by installing such files as part of the toolchain package > > post-install staging hooks. > > > > Yes, a post-install staging hook to install things to $(HOST_DIR) is a > > bit weird, but the toolchain infrastructure is made of target packages > > only, and they all install a lot of stuff to $(HOST_DIR) already. > > What I am more concerned about, is that the cmake package is registering > a hook for another package, toolchain. Do we want to introduce a toolchain-cmake package, that all CMake packages would depend on, just for the sake of installing those files ? If we would do this, then a normal toolchain build (i.e without building any CMake package) would no longer generate the toolchainfile.cmake file, which is quite useful for SDKs. People could indeed enable that package explicitly when they are building a SDK. But all in all, I'm not sure it's really worth the hassle. But I'm open to comments on that. Or perhaps you had other suggestions in mind? > Have you tried to run check-package on that? Nope, I did not, but it does not complain: $ ./utils/check-package package/pkg-cmake.mk 0 lines processed 0 warnings generated Thanks for the review! Thomas
Thomas, All, On 2017-12-03 23:55 +0100, Thomas Petazzoni spake thusly: > On Sun, 3 Dec 2017 23:34:54 +0100, Yann E. MORIN wrote: > > On 2017-12-01 21:53 +0100, Thomas Petazzoni spake thusly: > > > Currently, the toolchainfile.cmake and Buildroot.cmake files are > > > installed outside of any package, just triggered by the toolchain > > > target. > > > > > > As part of the per-package SDK effort, we are trying to avoid anything > > > that installs to the global $(HOST_DIR), and this is one of the > > > remaining files installed in $(HOST_DIR) outside of any package. We > > > fix this by installing such files as part of the toolchain package > > > post-install staging hooks. > > > > > > Yes, a post-install staging hook to install things to $(HOST_DIR) is a > > > bit weird, but the toolchain infrastructure is made of target packages > > > only, and they all install a lot of stuff to $(HOST_DIR) already. > > > > What I am more concerned about, is that the cmake package is registering > > a hook for another package, toolchain. > > Do we want to introduce a toolchain-cmake package, that all CMake > packages would depend on, just for the sake of installing those files ? > > If we would do this, then a normal toolchain build (i.e without > building any CMake package) would no longer generate the > toolchainfile.cmake file, which is quite useful for SDKs. People could > indeed enable that package explicitly when they are building a SDK. > > But all in all, I'm not sure it's really worth the hassle. But I'm open > to comments on that. > > Or perhaps you had other suggestions in mind? No, I don't have a better solution. And in fact, it is not a package that is registering a hook for another one; it is the cmake infra that registers a hook for the toolchain package. And I'm pretty fine with this, in fact. Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr> > > Have you tried to run check-package on that? > > Nope, I did not, but it does not complain: > > $ ./utils/check-package package/pkg-cmake.mk > 0 lines processed > 0 warnings generated Right, because this is not a package, but a package infra. Thanks! :-) Regards, Yann E. MORIN. > Thanks for the review! > > Thomas > -- > Thomas Petazzoni, CTO, Free Electrons > Embedded Linux and Kernel engineering > http://free-electrons.com
diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk index 6739704e3c..6d4f948d07 100644 --- a/package/pkg-cmake.mk +++ b/package/pkg-cmake.mk @@ -244,8 +244,8 @@ endif # based on the toolchainfile.cmake file's location: $(HOST_DIR)/share/buildroot # In all the other variables, HOST_DIR will be replaced by RELOCATED_HOST_DIR, # so we have to strip "$(HOST_DIR)/" from the paths that contain it. -$(HOST_DIR)/share/buildroot/toolchainfile.cmake: - @mkdir -p $(@D) +define TOOLCHAIN_CMAKE_INSTALL_FILES + @mkdir -p $(HOST_DIR)/share/buildroot sed \ -e 's#@@STAGING_SUBDIR@@#$(call qstrip,$(STAGING_SUBDIR))#' \ -e 's#@@TARGET_CFLAGS@@#$(call qstrip,$(TARGET_CFLAGS))#' \ @@ -259,7 +259,9 @@ $(HOST_DIR)/share/buildroot/toolchainfile.cmake: -e 's#@@TOOLCHAIN_HAS_FORTRAN@@#$(if $(BR2_TOOLCHAIN_HAS_FORTRAN),1,0)#' \ -e 's#@@CMAKE_BUILD_TYPE@@#$(if $(BR2_ENABLE_DEBUG),Debug,Release)#' \ $(TOPDIR)/support/misc/toolchainfile.cmake.in \ - > $@ + > $(HOST_DIR)/share/buildroot/toolchainfile.cmake + $(Q)$(INSTALL) -D -m 0644 support/misc/Buildroot.cmake \ + $(HOST_DIR)/share/buildroot/Platform/Buildroot.cmake +endef -$(HOST_DIR)/share/buildroot/Platform/Buildroot.cmake: - $(Q)$(INSTALL) -D -m 0644 support/misc/Buildroot.cmake $(@) +TOOLCHAIN_POST_INSTALL_STAGING_HOOKS += TOOLCHAIN_CMAKE_INSTALL_FILES diff --git a/toolchain/toolchain/toolchain.mk b/toolchain/toolchain/toolchain.mk index b55b0c712c..283e0a74ee 100644 --- a/toolchain/toolchain/toolchain.mk +++ b/toolchain/toolchain/toolchain.mk @@ -38,6 +38,3 @@ TOOLCHAIN_INSTALL_STAGING = YES endif $(eval $(virtual-package)) - -toolchain: $(HOST_DIR)/share/buildroot/toolchainfile.cmake -toolchain: $(HOST_DIR)/share/buildroot/Platform/Buildroot.cmake
Currently, the toolchainfile.cmake and Buildroot.cmake files are installed outside of any package, just triggered by the toolchain target. As part of the per-package SDK effort, we are trying to avoid anything that installs to the global $(HOST_DIR), and this is one of the remaining files installed in $(HOST_DIR) outside of any package. We fix this by installing such files as part of the toolchain package post-install staging hooks. Yes, a post-install staging hook to install things to $(HOST_DIR) is a bit weird, but the toolchain infrastructure is made of target packages only, and they all install a lot of stuff to $(HOST_DIR) already. Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> --- Changes since v2: - None Changes since v1: - New patch --- package/pkg-cmake.mk | 12 +++++++----- toolchain/toolchain/toolchain.mk | 3 --- 2 files changed, 7 insertions(+), 8 deletions(-)