[v5,1/3] pkg-cmake: allow to build package in a subdirectory

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

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