pkg-cmake: allow to build package in a subdirectory

From: Gwenhael Goavec-Merou <[email protected]>

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 <[email protected]>

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

docs/manual/adding-packages-cmake.txt +3 -0

index d92b209..0d4f2fa 100644
@@ -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.

package/pkg-cmake.mk +0 -0

index 2404c40..9de21b5 100644

package/pkg-cmake.mk +7 -3

+$(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

Quoted 1 line

+ifeq ($$($(2)_SUPPORTS_IN_SOURCE_BUILD), no)

s/, no/,no/

Dear Samuel Martin,

Quoted 2 lines · Samuel Martin

Quoted 1 line

+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?

From: Gwenhael Goavec-Merou <[email protected]>

Some packages, like GNURadio for VOLK, needs cheetah on host at buildtime.

Signed-off-by Gwenhael Goavec-Merou <[email protected]>

---
Changes v3 -> v4:
* use HOST_PYTHON_CHEETAH_DEPENDENCIES instead of PYTHON_CHEETAH_DEPENDENCIES
* suppress '+' for dependency definition

---
 package/python-cheetah/python-cheetah.mk | 2 ++
 1 file changed, 2 insertions(+)

package/python-cheetah/python-cheetah.mk +0 -0

index 08076b5..3155951 100644

package/python-cheetah/python-cheetah.mk +1 -0

 $(eval $(python-package))
+$(eval $(host-python-package))
 

This should probably carry a comment to remember what you explained in an
earlier mail, e.g.:

# Runtime dependency on markdown is not expressed in Config.in for host package

and perhaps a fuller explanation in the commit log (explaining that setuptools will download it if it can't be found).

BTW, Thomas, when you committed python-cheetah you removed the runtime
dependency on markdown because there are some examples you can run without markdown. Is that the way we work? The cheetah package itself declares a dependency on markdown because one of its classes (Filters.Markdown) uses it. Of course, as long as you don't use that particular filter, there won't be a problem. But it feels weird to me that we remove a dependency that is claimed by a package, unless there is a good reason for it (and I don't count saving 260K of .pyc files as a good enough reason).

Dear Arnout Vandecappelle,

On Tue, 10 Mar 2015 23:39:31 +0100, Arnout Vandecappelle wrote:

Quoted 8 lines

BTW, Thomas, when you committed python-cheetah you removed the runtime
dependency on markdown because there are some examples you can run without markdown. Is that the way we work? The cheetah package itself declares a dependency on markdown because one of its classes (Filters.Markdown) uses it. Of course, as long as you don't use that particular filter, there won't be a problem. But it feels weird to me that we remove a dependency that is claimed by a package, unless there is a good reason for it (and I don't count saving 260K of .pyc files as a good enough reason).

"Way we work" is fairly loosely defined when it comes to interpreted languages and more-or-less optional dependencies :-)

Should python-cheetah select all the possible dependencies it may use for all its filters, or should it leave this responsibility to the Buildroot user ? Not easy to decide.

Since the dependency was apparently not strictly needed, and there was (as far as I remember) no comment justifying the dependency, it seemed natural to me to remove this dependency. That being said, I wouldn't be offended at all if we decide to re-introduce the dependency, provided a comment indicating why we're adding this not-really mandatory dependency.

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

"Arnout" == Arnout Vandecappelle <[email protected]> writes:

On the other hand, as long as the lack of dependency is not bothering anyone,
we can leave it as it is :-) The purpose of discussion is what should be the guiding principle for future python packages. My proposal is: follow the egg info.

For me, the acid test is what the package declares itself.
Cheetah.egg-info/requires.txt says Markdown >= 2.0.1
For perl packages, we trust whatever dependencies the package declares. If we would have a scancpan-like script for pypi, the dependency would be there.

On the other hand, as long as the lack of dependency is not bothering anyone,
we can leave it as it is :-) The purpose of discussion is what should be the guiding principle for future python packages. My proposal is: follow the egg info.