[RFC][PATCH] cmake: respect ${S} and ${B}

[RFC][PATCH] cmake: respect ${S} and ${B}

[Ross Burton]

Hi,

This is a Request For Comments because it changes behaviour of the cmake class
and I'm not entirely knowledgeable in cmake.

For some reason, cmake.bbclass doesn't use ${S} and ${B}, but instead has it's
own variables OECMAKE_SOURCEPATH ("." by default) and OECMAKE_BUILDPATH ("" by
default).  Those defaults meant that the build happened in the source directory,
which conveniently was ${S}.  Unless ${B} was also set, in which case it all
broke.

I don't see a good reason for cmake.bbclass having it's own special versions of
${S} and ${B}, so this patch drops them and replicates some of the logic in
autotools.bbclass: specifically the part where if ${S} and ${B} are different,
delete ${B} before building.  This ensures that switching machine doesn't re-use
the same build directory, which was the cause of me going back to look at this
(libproxy trying to use the nuc sysroot when I'm building for qemux86-64).

Some open questions:

1) As I understand it cmake has more reliable support for out-of-tree builds
than autotools.  If this is the case should cmake.bbclass set B ?=
"${WORKDIR}/build", or leave setting of B to separatebuilddir.inc?  Are there
known recipes using cmake that fail with out-of-tree builds?

2) Is dropping OECMAKE_SOURCEPATH and OECMAKE_BUILDPATH acceptable?  Nothing in
oe-core uses them and there's three (IIRC) recipes in meta-oe that use them.
Assuming the answer to (1) is "separatebuilddir.inc" then the only fallout
should be these recipes using in-tree builds until OECMAKE_BUILDPATH is replaced
with B.

Feedback and testing from people who actively use cmake very welcome.

Cheers,
Ross

[PATCH] cmake: respect ${S} and ${B}

[Ross Burton]

Instead of the class-specific variables OECMAKE_BUILDPATH and
OECMAKE_SOURCEPATH, just use ${B} and ${S}.

If these two paths are different, delete any existing ${B} before running a
build so that previous builds don't taint the current build.

Note that OECMAKE_SOURCEPATH and OECMAKE_BUILDPATH are not respected at all, so
recipes that manually set these in the past will need to be updated to either
use something along the lines of separatebuilddir.inc or set B themselves.

Signed-off-by: Ross Burton <ross.burton@intel.com>
---
 meta/classes/cmake.bbclass |   31 +++++++------------------------
 1 file changed, 7 insertions(+), 24 deletions(-)

diff --git a/meta/classes/cmake.bbclass b/meta/classes/cmake.bbclass
index 30c1792..4d1489a 100644
--- a/meta/classes/cmake.bbclass
+++ b/meta/classes/cmake.bbclass
@@ -6,15 +6,6 @@ CCACHE = ""
 # We want the staging and installing functions from autotools
 inherit autotools
 
-# Use in-tree builds by default but allow this to be changed
-# since some packages do not support them (e.g. llvm 2.5).
-OECMAKE_SOURCEPATH ?= "."
-
-# If declaring this, make sure you also set EXTRA_OEMAKE to
-# "-C ${OECMAKE_BUILDPATH}". So it will run the right makefiles.
-OECMAKE_BUILDPATH ?= ""
-B="${S}"
-
 # C/C++ Compiler (without cpu arch/tune arguments)
 OECMAKE_C_COMPILER ?= "`echo ${CC} | sed 's/^\([^ ]*\).*/\1/'`"
 OECMAKE_CXX_COMPILER ?= "`echo ${CXX} | sed 's/^\([^ ]*\).*/\1/'`"
@@ -73,10 +64,10 @@ EOF
 addtask generate_toolchain_file after do_patch before do_configure
 
 cmake_do_configure() {
-	if [ ${OECMAKE_BUILDPATH} ]
-	then
-		mkdir -p ${OECMAKE_BUILDPATH}
-		cd ${OECMAKE_BUILDPATH}
+	if [ "${S}" != "${B}" ]; then
+		rm -rf ${B}
+		mkdir -p ${B}
+		cd ${B}
 	fi
 
 	# Just like autotools cmake can use a site file to cache result that need generated binaries to run
@@ -88,7 +79,7 @@ cmake_do_configure() {
 
 	cmake \
 	  ${OECMAKE_SITEFILE} \
-	  ${OECMAKE_SOURCEPATH} \
+	  ${S} \
 	  -DCMAKE_INSTALL_PREFIX:PATH=${prefix} \
 	  -DCMAKE_INSTALL_SO_NO_EXE=0 \
 	  -DCMAKE_TOOLCHAIN_FILE=${WORKDIR}/toolchain.cmake \
@@ -98,20 +89,12 @@ cmake_do_configure() {
 }
 
 cmake_do_compile()  {
-	if [ ${OECMAKE_BUILDPATH} ]
-	then
-		cd ${OECMAKE_BUILDPATH}
-	fi
-
+	cd ${B}
 	base_do_compile
 }
 
 cmake_do_install() {
-	if [ ${OECMAKE_BUILDPATH} ];
-	then
-		cd ${OECMAKE_BUILDPATH}
-	fi
-
+	cd ${B}
 	autotools_do_install
 }
 
-- 
1.7.10.4

Re: [PATCH] cmake: respect ${S} and ${B}

[Philip Balister]

On 12/04/2013 07:38 PM, Ross Burton wrote:
> Instead of the class-specific variables OECMAKE_BUILDPATH and
> OECMAKE_SOURCEPATH, just use ${B} and ${S}.
> 
> If these two paths are different, delete any existing ${B} before running a
> build so that previous builds don't taint the current build.
> 
> Note that OECMAKE_SOURCEPATH and OECMAKE_BUILDPATH are not respected at all, so
> recipes that manually set these in the past will need to be updated to either
> use something along the lines of separatebuilddir.inc or set B themselves.

I'm carrying EXTRA_OEMAKE = "-C $(OECMAKE_BUILDPATH)" in recipes. It
looks like this removes the need for this also? My cmake invocation is
rusty :)

Philip

> 
> Signed-off-by: Ross Burton <ross.burton@intel.com>
> ---
>  meta/classes/cmake.bbclass |   31 +++++++------------------------
>  1 file changed, 7 insertions(+), 24 deletions(-)
> 
> diff --git a/meta/classes/cmake.bbclass b/meta/classes/cmake.bbclass
> index 30c1792..4d1489a 100644
> --- a/meta/classes/cmake.bbclass
> +++ b/meta/classes/cmake.bbclass
> @@ -6,15 +6,6 @@ CCACHE = ""
>  # We want the staging and installing functions from autotools
>  inherit autotools
>  
> -# Use in-tree builds by default but allow this to be changed
> -# since some packages do not support them (e.g. llvm 2.5).
> -OECMAKE_SOURCEPATH ?= "."
> -
> -# If declaring this, make sure you also set EXTRA_OEMAKE to
> -# "-C ${OECMAKE_BUILDPATH}". So it will run the right makefiles.
> -OECMAKE_BUILDPATH ?= ""
> -B="${S}"
> -
>  # C/C++ Compiler (without cpu arch/tune arguments)
>  OECMAKE_C_COMPILER ?= "`echo ${CC} | sed 's/^\([^ ]*\).*/\1/'`"
>  OECMAKE_CXX_COMPILER ?= "`echo ${CXX} | sed 's/^\([^ ]*\).*/\1/'`"
> @@ -73,10 +64,10 @@ EOF
>  addtask generate_toolchain_file after do_patch before do_configure
>  
>  cmake_do_configure() {
> -	if [ ${OECMAKE_BUILDPATH} ]
> -	then
> -		mkdir -p ${OECMAKE_BUILDPATH}
> -		cd ${OECMAKE_BUILDPATH}
> +	if [ "${S}" != "${B}" ]; then
> +		rm -rf ${B}
> +		mkdir -p ${B}
> +		cd ${B}
>  	fi
>  
>  	# Just like autotools cmake can use a site file to cache result that need generated binaries to run
> @@ -88,7 +79,7 @@ cmake_do_configure() {
>  
>  	cmake \
>  	  ${OECMAKE_SITEFILE} \
> -	  ${OECMAKE_SOURCEPATH} \
> +	  ${S} \
>  	  -DCMAKE_INSTALL_PREFIX:PATH=${prefix} \
>  	  -DCMAKE_INSTALL_SO_NO_EXE=0 \
>  	  -DCMAKE_TOOLCHAIN_FILE=${WORKDIR}/toolchain.cmake \
> @@ -98,20 +89,12 @@ cmake_do_configure() {
>  }
>  
>  cmake_do_compile()  {
> -	if [ ${OECMAKE_BUILDPATH} ]
> -	then
> -		cd ${OECMAKE_BUILDPATH}
> -	fi
> -
> +	cd ${B}
>  	base_do_compile
>  }
>  
>  cmake_do_install() {
> -	if [ ${OECMAKE_BUILDPATH} ];
> -	then
> -		cd ${OECMAKE_BUILDPATH}
> -	fi
> -
> +	cd ${B}
>  	autotools_do_install
>  }
>  
> 

Re: [PATCH] cmake: respect ${S} and ${B}

[Richard Purdie]

On Thu, 2013-12-05 at 17:18 -0500, Philip Balister wrote:
> On 12/04/2013 07:38 PM, Ross Burton wrote:
> > Instead of the class-specific variables OECMAKE_BUILDPATH and
> > OECMAKE_SOURCEPATH, just use ${B} and ${S}.
> > 
> > If these two paths are different, delete any existing ${B} before running a
> > build so that previous builds don't taint the current build.
> > 
> > Note that OECMAKE_SOURCEPATH and OECMAKE_BUILDPATH are not respected at all, so
> > recipes that manually set these in the past will need to be updated to either
> > use something along the lines of separatebuilddir.inc or set B themselves.
> 
> I'm carrying EXTRA_OEMAKE = "-C $(OECMAKE_BUILDPATH)" in recipes. It
> looks like this removes the need for this also? My cmake invocation is
> rusty :)

do_compile runs by default in ${B} so in most cases that shouldn't be
needed. The new equivalent would be "-C ${B}" but that shouldn't be
needed in most cases.

Cheers,

Richard

Re: [RFC][PATCH] cmake: respect ${S} and ${B}

[Martin Jansa]

[-- Attachment #1: Type: text/plain, Size: 2169 bytes --]

On Thu, Dec 05, 2013 at 12:38:57AM +0000, Ross Burton wrote:
> Hi,
> 
> This is a Request For Comments because it changes behaviour of the cmake class
> and I'm not entirely knowledgeable in cmake.

Neither am I, but patch looks reasonable and I like it, because I was
planing to do something similar.

> For some reason, cmake.bbclass doesn't use ${S} and ${B}, but instead has it's
> own variables OECMAKE_SOURCEPATH ("." by default) and OECMAKE_BUILDPATH ("" by
> default).  Those defaults meant that the build happened in the source directory,
> which conveniently was ${S}.  Unless ${B} was also set, in which case it all
> broke.
> 
> I don't see a good reason for cmake.bbclass having it's own special versions of
> ${S} and ${B}, so this patch drops them and replicates some of the logic in
> autotools.bbclass: specifically the part where if ${S} and ${B} are different,
> delete ${B} before building.  This ensures that switching machine doesn't re-use
> the same build directory, which was the cause of me going back to look at this
> (libproxy trying to use the nuc sysroot when I'm building for qemux86-64).
> 
> Some open questions:
> 
> 1) As I understand it cmake has more reliable support for out-of-tree builds
> than autotools.  If this is the case should cmake.bbclass set B ?=
> "${WORKDIR}/build", or leave setting of B to separatebuilddir.inc?  Are there
> known recipes using cmake that fail with out-of-tree builds?
> 
> 2) Is dropping OECMAKE_SOURCEPATH and OECMAKE_BUILDPATH acceptable?  Nothing in
> oe-core uses them and there's three (IIRC) recipes in meta-oe that use them.
> Assuming the answer to (1) is "separatebuilddir.inc" then the only fallout
> should be these recipes using in-tree builds until OECMAKE_BUILDPATH is replaced
> with B.
> 
> Feedback and testing from people who actively use cmake very welcome.
> 
> Cheers,
> Ross
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core

-- 
Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 205 bytes --]

Re: [RFC][PATCH] cmake: respect ${S} and ${B}

[Koen Kooi]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Ross Burton schreef op 05-12-13 01:38:
> Hi,
> 
> This is a Request For Comments because it changes behaviour of the cmake
> class and I'm not entirely knowledgeable in cmake.
> 
> For some reason, cmake.bbclass doesn't use ${S} and ${B}, but instead has
> it's own variables OECMAKE_SOURCEPATH ("." by default) and
> OECMAKE_BUILDPATH ("" by default).  Those defaults meant that the build
> happened in the source directory, which conveniently was ${S}.  Unless
> ${B} was also set, in which case it all broke.
> 
> I don't see a good reason for cmake.bbclass having it's own special
> versions of ${S} and ${B}, so this patch drops them and replicates some
> of the logic in autotools.bbclass: specifically the part where if ${S}
> and ${B} are different, delete ${B} before building.  This ensures that
> switching machine doesn't re-use the same build directory, which was the
> cause of me going back to look at this (libproxy trying to use the nuc
> sysroot when I'm building for qemux86-64).
> 
> Some open questions:
> 
> 1) As I understand it cmake has more reliable support for out-of-tree
> builds than autotools.

Next to toolchain files it's the only positive think I can say about cmake.

> If this is the case should cmake.bbclass set B ?= "${WORKDIR}/build"

In opencv I set it to ${WORKDIR}/build-${TARGET_ARCH} to match the autotools
class, but that was a while ago. All the other meta-oe recipes seem to use
what you propose.

> , or leave setting of B to separatebuilddir.inc?  Are there known recipes
> using cmake that fail with out-of-tree builds?
> 
> 2) Is dropping OECMAKE_SOURCEPATH and OECMAKE_BUILDPATH acceptable?
> Nothing in oe-core uses them and there's three (IIRC) recipes in meta-oe
> that use them. Assuming the answer to (1) is "separatebuilddir.inc" then
> the only fallout should be these recipes using in-tree builds until
> OECMAKE_BUILDPATH is replaced with B.

I'm not sure, but I think I favour the cmake.bbclass path, it's only a
handfull of recipe that need straightforward changes.

regards,

Koen
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (Darwin)
Comment: GPGTools - http://gpgtools.org

iD8DBQFSoEq+MkyGM64RGpERAuXDAJ9T+TbAii88x2r9PQ5XCRbP5x5pYgCgltNm
4rXcZm+/l/sMnSNiPbFbRrI=
=6oHW
-----END PGP SIGNATURE-----

Re: [RFC][PATCH] cmake: respect ${S} and ${B}

[Richard Purdie]

On Thu, 2013-12-05 at 00:38 +0000, Ross Burton wrote:
> Hi,
> 
> This is a Request For Comments because it changes behaviour of the cmake class
> and I'm not entirely knowledgeable in cmake.
> 
> For some reason, cmake.bbclass doesn't use ${S} and ${B}, but instead has it's
> own variables OECMAKE_SOURCEPATH ("." by default) and OECMAKE_BUILDPATH ("" by
> default).  Those defaults meant that the build happened in the source directory,
> which conveniently was ${S}.  Unless ${B} was also set, in which case it all
> broke.
> 
> I don't see a good reason for cmake.bbclass having it's own special versions of
> ${S} and ${B}, so this patch drops them and replicates some of the logic in
> autotools.bbclass: specifically the part where if ${S} and ${B} are different,
> delete ${B} before building.  This ensures that switching machine doesn't re-use
> the same build directory, which was the cause of me going back to look at this
> (libproxy trying to use the nuc sysroot when I'm building for qemux86-64).
> 
> Some open questions:
> 
> 1) As I understand it cmake has more reliable support for out-of-tree builds
> than autotools.  If this is the case should cmake.bbclass set B ?=
> "${WORKDIR}/build", or leave setting of B to separatebuilddir.inc?  Are there
> known recipes using cmake that fail with out-of-tree builds?

separatebuilddir.inc was really a stopgap solution to see how widespread
potential issues were. Ideally I'd like to get to the point where
recipes flag themselves are broken with out of tree builds rather than
having a list of ones which are compatible.

Doing this for OE-Core is straightforward now but what about meta-oe?

Would there be interest in trying to change that default or is it going
to be too painful?

This means I'm in favour of changing the cmake default if we can, it
looks like a simpler problem space than autotools.bbclass

Cheers,

Richard

Re: [RFC][PATCH] cmake: respect ${S} and ${B}

[Martin Jansa]

[-- Attachment #1: Type: text/plain, Size: 2505 bytes --]

On Thu, Dec 05, 2013 at 10:10:02AM +0000, Richard Purdie wrote:
> On Thu, 2013-12-05 at 00:38 +0000, Ross Burton wrote:
> > Hi,
> > 
> > This is a Request For Comments because it changes behaviour of the cmake class
> > and I'm not entirely knowledgeable in cmake.
> > 
> > For some reason, cmake.bbclass doesn't use ${S} and ${B}, but instead has it's
> > own variables OECMAKE_SOURCEPATH ("." by default) and OECMAKE_BUILDPATH ("" by
> > default).  Those defaults meant that the build happened in the source directory,
> > which conveniently was ${S}.  Unless ${B} was also set, in which case it all
> > broke.
> > 
> > I don't see a good reason for cmake.bbclass having it's own special versions of
> > ${S} and ${B}, so this patch drops them and replicates some of the logic in
> > autotools.bbclass: specifically the part where if ${S} and ${B} are different,
> > delete ${B} before building.  This ensures that switching machine doesn't re-use
> > the same build directory, which was the cause of me going back to look at this
> > (libproxy trying to use the nuc sysroot when I'm building for qemux86-64).
> > 
> > Some open questions:
> > 
> > 1) As I understand it cmake has more reliable support for out-of-tree builds
> > than autotools.  If this is the case should cmake.bbclass set B ?=
> > "${WORKDIR}/build", or leave setting of B to separatebuilddir.inc?  Are there
> > known recipes using cmake that fail with out-of-tree builds?
> 
> separatebuilddir.inc was really a stopgap solution to see how widespread
> potential issues were. Ideally I'd like to get to the point where
> recipes flag themselves are broken with out of tree builds rather than
> having a list of ones which are compatible.
> 
> Doing this for OE-Core is straightforward now but what about meta-oe?
> 
> Would there be interest in trying to change that default or is it going
> to be too painful?

It would be nice to change it in smaller steps, e.g. by bbclasses.

I'm already using separate B by default in meta-qt5 and it works good.

Adding it in cmake, qmake4, then autotools.bbclass, then ... would allow
to fix meta-oe recipes in smaller chunks instead of trying to fix them
all at once. And if we change it only for few .bbclasses then it would
still be improvement.

> This means I'm in favour of changing the cmake default if we can, it
> looks like a simpler problem space than autotools.bbclass

-- 
Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 205 bytes --]