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

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

[Ross Burton]

Hi,

Here is V2 of my cmake ${S}/${B} patch.  The class isn't that changed since V1,
apart from the addition of a note if the recipe has set OECMAKE_SOURCEPATH or
_BUILDPATH.

Then there's the choice of what we do with the default for ${B}.  One patch sets
${B} in separatebuilddir.inc for libproxy and taglib in oe-core.  The
alternative is to set B in cmake.bbclass directly to ${WORKDIR}/build.

Either way, this change to the class isn't backwards-compatible with many
recipes in the wild due to various ways of doing out-of-tree builds in meta-oe.
A branch that fixes up meta-oe (by generally deleting lines) is available in
meta-oe-contrib:ross/cmake for reference.  Some recipes are using
OECMAKE_SOURCEPATH/_BUILDPATH, and others are doing out-of-tree builds
"manually" by passing arguments to EXTRA_OECMAKE and doing cd's in
do_compile_prepend. Yuck.

Personally, I'd say we should merge cmake.bbclass with B set inside it by
default.  In my testing against meta-oe, only piglit is troublesome and that can
set B=${S}, everything else seems to be working fine once the recipes were cleaned up.

Ross

[PATCH 1/3] 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, 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.  If the
old variables are set, a warning is displayed.

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

diff --git a/meta/classes/cmake.bbclass b/meta/classes/cmake.bbclass
index 30c1792..1dc406d2 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,14 @@ 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 [ "${OECMAKE_BUILDPATH}" -o "${OECMAKE_SOURCEPATH}" ]; then
+		bbnote "cmake.bbclass no longer uses OECMAKE_SOURCEPATH and OECMAKE_BUILDPATH. This recipe now will do in-tree builds, to do out-of-tree builds set S and B."
+	fi
+
+	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 +83,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 +93,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

[PATCH 2/3] separatebuilddir: build libproxy and taglib out of the source tree

[Ross Burton]

Signed-off-by: Ross Burton <ross.burton@intel.com>
---
 meta/conf/distro/include/seperatebuilddir.inc |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/meta/conf/distro/include/seperatebuilddir.inc b/meta/conf/distro/include/seperatebuilddir.inc
index e1a5c6b..a6df6ac 100644
--- a/meta/conf/distro/include/seperatebuilddir.inc
+++ b/meta/conf/distro/include/seperatebuilddir.inc
@@ -320,8 +320,7 @@ B_pn-libpcre = "${SEPB}"
 B_pn-libpcre-native = "${SEPB}"
 B_pn-libpng = "${SEPB}"
 B_pn-libpng-native = "${SEPB}"
-# Needs automatic support in cmake.bbclass
-#B_pn-libproxy = "${SEPB}"
+B_pn-libproxy = "${SEPB}"
 B_pn-libpthread-stubs = "${SEPB}"
 B_pn-libpthread-stubs-native = "${SEPB}"
 B_pn-librsvg = "${SEPB}"
@@ -660,6 +659,7 @@ B_pn-sysprof = "${SEPB}"
 B_pn-systemd = "${SEPB}"
 B_pn-systemtap = "${SEPB}"
 B_pn-systemtap-native = "${SEPB}"
+B_pn-taglib = "${SEPB}"
 B_pn-tar = "${SEPB}"
 B_pn-tar-replacement-native = "${SEPB}"
 B_pn-tcl = "${SEPB}"
-- 
1.7.10.4

[PATCH 3/3] cmake: default to out-of-tree builds

[Ross Burton]

Set B=${WORKDIR}/build in cmake.bbclass so that recipes using cmake.bbclass do
out-of-tree builds by default.

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

diff --git a/meta/classes/cmake.bbclass b/meta/classes/cmake.bbclass
index 1dc406d2..3ac3fcc 100644
--- a/meta/classes/cmake.bbclass
+++ b/meta/classes/cmake.bbclass
@@ -1,4 +1,5 @@
 DEPENDS_prepend = "cmake-native "
+B = "${WORKDIR}/build"
 
 # We need to unset CCACHE otherwise cmake gets too confused
 CCACHE = ""
-- 
1.7.10.4

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

[Saul Wold]

On 01/10/2014 09:54 AM, 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, 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.  If the
> old variables are set, a warning is displayed.
>
> Signed-off-by: Ross Burton <ross.burton@intel.com>
> ---
>   meta/classes/cmake.bbclass |   35 +++++++++++------------------------
>   1 file changed, 11 insertions(+), 24 deletions(-)
>
> diff --git a/meta/classes/cmake.bbclass b/meta/classes/cmake.bbclass
> index 30c1792..1dc406d2 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,14 @@ 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 [ "${OECMAKE_BUILDPATH}" -o "${OECMAKE_SOURCEPATH}" ]; then
> +		bbnote "cmake.bbclass no longer uses OECMAKE_SOURCEPATH and OECMAKE_BUILDPATH. This recipe now will do in-tree builds, to do out-of-tree builds set S and B."
> +	fi
> +
> +	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 +83,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 +93,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

This seems to cause a problem:
ERROR: Logfile of failure stored in: 
/srv/hdd/builds/world/tmp/work/ppc7400-poky-linux/cmake/2.8.12.1-r0/temp/log.do_install.5697
Log data follows:
| DEBUG: SITE files ['endian-big', 'bit-32', 'powerpc-common', 
'common-linux', 'common-glibc', 'powerpc32-linux', 'powerpc-linux', 
'common']
| DEBUG: Executing shell function do_install
| NOTE: make -j 16 
DESTDIR=/srv/hdd/builds/world/tmp/work/ppc7400-poky-linux/cmake/2.8.12.1-r0/image 
install
| make: *** No rule to make target `install'.  Stop.
| ERROR: oe_runmake failed
| WARNING: 
/srv/hdd/builds/world/tmp/work/ppc7400-poky-linux/cmake/2.8.12.1-r0/temp/run.do_install.5697:1 
exit 1 from
|   exit 1
| ERROR: Function failed: do_install (log file is located at 
/srv/hdd/builds/world/tmp/work/ppc7400-poky-linux/cmake/2.8.12.1-r0/temp/log.do_install.5697)
ERROR: Task 7891 
(/srv/hdd/poky/meta/recipes-devtools/cmake/cmake_2.8.12.1.bb, 
do_install) failed with exit code '1'


Sau!

>   }
>
>

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

[Saul Wold]

On 01/13/2014 02:06 PM, Saul Wold wrote:
> On 01/10/2014 09:54 AM, 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,
>> 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.  If the
>> old variables are set, a warning is displayed.
>>
>> Signed-off-by: Ross Burton <ross.burton@intel.com>
>> ---
>>   meta/classes/cmake.bbclass |   35 +++++++++++------------------------
>>   1 file changed, 11 insertions(+), 24 deletions(-)
>>
>> diff --git a/meta/classes/cmake.bbclass b/meta/classes/cmake.bbclass
>> index 30c1792..1dc406d2 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,14 @@ 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 [ "${OECMAKE_BUILDPATH}" -o "${OECMAKE_SOURCEPATH}" ]; then
>> +        bbnote "cmake.bbclass no longer uses OECMAKE_SOURCEPATH and
>> OECMAKE_BUILDPATH. This recipe now will do in-tree builds, to do
>> out-of-tree builds set S and B."
>> +    fi
>> +
>> +    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 +83,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 +93,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
>
> This seems to cause a problem:
> ERROR: Logfile of failure stored in:
> /srv/hdd/builds/world/tmp/work/ppc7400-poky-linux/cmake/2.8.12.1-r0/temp/log.do_install.5697
>
> Log data follows:
> | DEBUG: SITE files ['endian-big', 'bit-32', 'powerpc-common',
> 'common-linux', 'common-glibc', 'powerpc32-linux', 'powerpc-linux',
> 'common']
> | DEBUG: Executing shell function do_install
> | NOTE: make -j 16
> DESTDIR=/srv/hdd/builds/world/tmp/work/ppc7400-poky-linux/cmake/2.8.12.1-r0/image
> install
> | make: *** No rule to make target `install'.  Stop.
> | ERROR: oe_runmake failed
> | WARNING:
> /srv/hdd/builds/world/tmp/work/ppc7400-poky-linux/cmake/2.8.12.1-r0/temp/run.do_install.5697:1
> exit 1 from
> |   exit 1
> | ERROR: Function failed: do_install (log file is located at
> /srv/hdd/builds/world/tmp/work/ppc7400-poky-linux/cmake/2.8.12.1-r0/temp/log.do_install.5697)
>
> ERROR: Task 7891
> (/srv/hdd/poky/meta/recipes-devtools/cmake/cmake_2.8.12.1.bb,
> do_install) failed with exit code '1'
>
>
Oops, sorry spoke to soon, needed to have a clean WORKDIR, I was 
building in a pre existing tmp.

Sorry for the noise.

Sau!

> Sau!
>
>>   }
>>
>>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>
>

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

[Burton, Ross]

On 13 January 2014 22:15, Saul Wold <sgw@linux.intel.com> wrote:
>> This seems to cause a problem:
>> ERROR: Logfile of failure stored in:
>>
>> DESTDIR=/srv/hdd/builds/world/tmp/work/ppc7400-poky-linux/cmake/2.8.12.1-r0/image
>> install
>> | make: *** No rule to make target `install'.  Stop.
>> | ERROR: oe_runmake failed
>> | WARNING:
>
> Oops, sorry spoke to soon, needed to have a clean WORKDIR, I was building in
> a pre existing tmp.
>
> Sorry for the noise.

Yes, this change means existing work directories need to be wiped,
just as the matching automake change did.  Not sure if it would be
possible to handle this migration sanely, maybe by checking for and
deleting the cmake cache in $S?

Ross

Re: [PATCH 3/3] cmake: default to out-of-tree builds

[Stefan Herbrechtsmeier]

Am 10.01.2014 18:54, schrieb Ross Burton:
> Set B=${WORKDIR}/build in cmake.bbclass so that recipes using cmake.bbclass do
> out-of-tree builds by default.
You should update the note to reflect the new behaviour.

Re: [PATCH 3/3] cmake: default to out-of-tree builds

[Burton, Ross]

Good point, thanks.  Patch sent.

Ross

On 16 January 2014 10:26, Stefan Herbrechtsmeier
<stefan@herbrechtsmeier.net> wrote:
> Am 10.01.2014 18:54, schrieb Ross Burton:
>
>> Set B=${WORKDIR}/build in cmake.bbclass so that recipes using
>> cmake.bbclass do
>> out-of-tree builds by default.
>
> You should update the note to reflect the new behaviour.
>