Message ID | 1426011874-11011-1-git-send-email-gwenj@trabucayre.com |
---|---|
State | Superseded |
Headers | show |
Hi Gwenhael, On Tue, Mar 10, 2015 at 7:24 PM, Gwenhael Goavec-Merou <gwenj@trabucayre.com> wrote: > From: Gwenhael Goavec-Merou <gwenhael.goavec-merou@trabucayre.com> > > For some cmake based packages, like GNURadio, it's forbidden to do the > compilation directly in the sources directory. This patch add a new > variable to specify, if needed, the name of a sub-directory used to compile. > > Signed-off-by: Gwenhael Goavec-Merou <gwenhael.goavec-merou@trabucayre.com> > --- > Changes v4 -> v5: > * Instead of overloading BUILDDIR uses a variable to set if package is compiled > in-source-tree or not > * update again manual > Changes v2 -> v3: > * Update docs to add detail about LIBFOO_BUILDDIR > Changes v1 -> v2: > * Allow to overload $(2)_BUILDDIR instead of adding a new variable and test > --- > docs/manual/adding-packages-cmake.txt | 3 +++ > package/pkg-cmake.mk | 11 +++++++++-- > 2 files changed, 12 insertions(+), 2 deletions(-) > > diff --git a/docs/manual/adding-packages-cmake.txt b/docs/manual/adding-packages-cmake.txt > index d92b209..0d4f2fa 100644 > --- a/docs/manual/adding-packages-cmake.txt > +++ b/docs/manual/adding-packages-cmake.txt > @@ -146,3 +146,6 @@ possible to customize what is done in any particular step: > +LIBFOO_CONFIGURE_CMDS+ variable, it will be used instead of the > default CMake one. However, using this method should be restricted > to very specific cases. Do not use it in the general case. > + > +* By adding +LIBFOO_SUPPORTS_IN_SOURCE_BUILD = no+ when a package > + prevents doing an in-source-tree build. > diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk > index 2404c40..9de21b5 100644 > --- a/package/pkg-cmake.mk > +++ b/package/pkg-cmake.mk > @@ -61,7 +61,12 @@ $(2)_INSTALL_STAGING_OPTS ?= DESTDIR=$$(STAGING_DIR) install > $(2)_INSTALL_TARGET_OPTS ?= DESTDIR=$$(TARGET_DIR) install > > $(2)_SRCDIR = $$($(2)_DIR)/$$($(2)_SUBDIR) > + > +ifeq ($$($(2)_SUPPORTS_IN_SOURCE_BUILD), no) s/, no/,no/ s/$(2)/$(3)/ When in-srctree build is forbidden, it is for both target and host package, so there is no need for having HOST_FOO_SUPPORTS_IN_SOURCE_BUILD set when FOO_SUPPORTS_IN_SOURCE_BUILD is already set. > +$(2)_BUILDDIR = $$($(2)_SRCDIR)/buildroot-build > +else > $(2)_BUILDDIR = $$($(2)_SRCDIR) > +endif > > # > # Configure step. Only define it if not already defined by the package > @@ -73,7 +78,8 @@ ifeq ($(4),target) > > # Configure package for target > define $(2)_CONFIGURE_CMDS > - (cd $$($$(PKG)_BUILDDIR) && \ > + (mkdir -p $$($$(PKG)_BUILDDIR) && \ > + cd $$($$(PKG)_BUILDDIR) && \ > rm -f CMakeCache.txt && \ > PATH=$$(BR_PATH) \ > $$($$(PKG)_CONF_ENV) $$(HOST_DIR)/usr/bin/cmake $$($$(PKG)_SRCDIR) \ > @@ -98,7 +104,8 @@ else > > # Configure package for host > define $(2)_CONFIGURE_CMDS > - (cd $$($$(PKG)_BUILDDIR) && \ > + (mkdir -p $$($$(PKG)_BUILDDIR) && \ > + cd $$($$(PKG)_BUILDDIR) && \ > rm -f CMakeCache.txt && \ > PATH=$$(BR_PATH) \ > $$(HOST_DIR)/usr/bin/cmake $$($$(PKG)_SRCDIR) \ > -- > 2.0.5 > Regards,
Dear Samuel Martin, On Tue, 10 Mar 2015 18:35:21 +0100, Samuel Martin wrote: > > +ifeq ($$($(2)_SUPPORTS_IN_SOURCE_BUILD), no) > s/, no/,no/ Actually it should be 'NO' and not 'no'. All other package infra variables have upper-case values. I know I suggested the SUPPORTS_IN_SOURCE_BUILD name, but what worries me a bit is that if one day we want to make the default to build things out of tree, a variable named <pkg>_SUPPORTS_IN_SOURCE_BUILD would not make much sense anymore. It would have to be named <pkg>_REQUIRES_IN_SOURCE_BUILD, set to NO by default in the package infra, and that can be overridden to YES by those packages that don't support out of tree build. So I'm not sure what to do know. Go with <pkg>_SUPPORTS_IN_SOURCE_BUILD for the moment, and change that when time comes? Best regards, Thomas
On 10/03/15 18:56, Thomas Petazzoni wrote: > Dear Samuel Martin, > > On Tue, 10 Mar 2015 18:35:21 +0100, Samuel Martin wrote: > >>> +ifeq ($$($(2)_SUPPORTS_IN_SOURCE_BUILD), no) >> s/, no/,no/ > > Actually it should be 'NO' and not 'no'. All other package infra > variables have upper-case values. > > I know I suggested the SUPPORTS_IN_SOURCE_BUILD name, but what worries > me a bit is that if one day we want to make the default to build things > out of tree, a variable named <pkg>_SUPPORTS_IN_SOURCE_BUILD would not > make much sense anymore. It would have to be named > <pkg>_REQUIRES_IN_SOURCE_BUILD, set to NO by default in the package > infra, and that can be overridden to YES by those packages that don't > support out of tree build. > > So I'm not sure what to do know. Go with <pkg>_SUPPORTS_IN_SOURCE_BUILD > for the moment, and change that when time comes? I'm all for building in a subirectory by default and only do it directly in the tree when necessary. And in fact, I would not even introduce the _REQUIRES_IN_SOURCE_BUILD variable until it is proven to be necessary. BTW, Thomas, did you introduce something like that in your out-of-source-build branch? Regards, Arnout
Dear Arnout Vandecappelle, On Tue, 10 Mar 2015 22:43:10 +0100, Arnout Vandecappelle wrote: > I'm all for building in a subirectory by default and only do it directly in the > tree when necessary. > > And in fact, I would not even introduce the _REQUIRES_IN_SOURCE_BUILD variable > until it is proven to be necessary. BTW, Thomas, did you introduce something > like that in your out-of-source-build branch? Yes, I did have a flag that allows a package to say "I support out-of-tree build". It defaulted to "NO" in generic-package, and to "YES" in autotools-package and cmake-package, but can be overridden by individual packages. For example, 'linux' can support out of tree build even though it's a generic-package (well, now a kconfig-package), or an autotools package may not support out of tree due to stupidities done by the upstream developer. However, this branch is now very old, and was far from being ready. I really would like to put the smallest amount of work on Gwenhael shoulders: I don't want to ask him, as a prerequisite of getting gnuradio merged, to do some major surgery work deep in our package infrastructure. So can we settle on a temporary solution that works for gnuradio and is acceptable, and implement the grand plan of doing full out of tree build at some later point? Cheers, Thomas
diff --git a/docs/manual/adding-packages-cmake.txt b/docs/manual/adding-packages-cmake.txt index d92b209..0d4f2fa 100644 --- a/docs/manual/adding-packages-cmake.txt +++ b/docs/manual/adding-packages-cmake.txt @@ -146,3 +146,6 @@ possible to customize what is done in any particular step: +LIBFOO_CONFIGURE_CMDS+ variable, it will be used instead of the default CMake one. However, using this method should be restricted to very specific cases. Do not use it in the general case. + +* By adding +LIBFOO_SUPPORTS_IN_SOURCE_BUILD = no+ when a package + prevents doing an in-source-tree build. diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk index 2404c40..9de21b5 100644 --- a/package/pkg-cmake.mk +++ b/package/pkg-cmake.mk @@ -61,7 +61,12 @@ $(2)_INSTALL_STAGING_OPTS ?= DESTDIR=$$(STAGING_DIR) install $(2)_INSTALL_TARGET_OPTS ?= DESTDIR=$$(TARGET_DIR) install $(2)_SRCDIR = $$($(2)_DIR)/$$($(2)_SUBDIR) + +ifeq ($$($(2)_SUPPORTS_IN_SOURCE_BUILD), no) +$(2)_BUILDDIR = $$($(2)_SRCDIR)/buildroot-build +else $(2)_BUILDDIR = $$($(2)_SRCDIR) +endif # # Configure step. Only define it if not already defined by the package @@ -73,7 +78,8 @@ ifeq ($(4),target) # Configure package for target define $(2)_CONFIGURE_CMDS - (cd $$($$(PKG)_BUILDDIR) && \ + (mkdir -p $$($$(PKG)_BUILDDIR) && \ + cd $$($$(PKG)_BUILDDIR) && \ rm -f CMakeCache.txt && \ PATH=$$(BR_PATH) \ $$($$(PKG)_CONF_ENV) $$(HOST_DIR)/usr/bin/cmake $$($$(PKG)_SRCDIR) \ @@ -98,7 +104,8 @@ else # Configure package for host define $(2)_CONFIGURE_CMDS - (cd $$($$(PKG)_BUILDDIR) && \ + (mkdir -p $$($$(PKG)_BUILDDIR) && \ + cd $$($$(PKG)_BUILDDIR) && \ rm -f CMakeCache.txt && \ PATH=$$(BR_PATH) \ $$(HOST_DIR)/usr/bin/cmake $$($$(PKG)_SRCDIR) \