Patchwork cmake: respect ${S} and ${B} patch problem

login
register
mail settings
Submitter Miroslav Keš
Date June 13, 2014, 4:33 p.m.
Message ID <539B27F0.90109@gmail.com>
Download mbox | patch
Permalink /patch/73773/
State New
Headers show

Comments

Miroslav Keš - June 13, 2014, 4:33 p.m.
Hi!

I have found a problem related to the patch "respect ${S} and ${B}" on the
cmake.bbclass (commit 43073569cb67d98c11aa71211d77b566b64f9145)

The cmake.bbclass now generates the cmake command using the ${S}
variable as the path where the top most CMakeLists.txt should be found.
This works OK as along as the CMakeLists.txt is in the top level
directory of the package source tree.
But CMake doesn't require the directory tree to be structured that way.
If the top level CMakeLists.txt is in a subdirectory of the package
source tree AND the recipe needs to patch a file which is at a higher
level the OE build is broken.

Example:

${WORKDIR}
    - cmake
        - Modules
            - FindSomePackage.cmake
    - Src
        - CMakeLists.txt  <- top level CMake file

For the cmake.bbclass to work correctly after the patch the S must be
set to the ${WORKDIR}/Src.
But the ${S} is also used as the base directory for patching.
In the example, the patch task creates directory  ${S}/patches (i.e.
${WORKDIR}/Src/patches) and creates symbolic links to all relevant
patches in that directory.
But those patches can only patch files under the ${S} subtree.
The ${WORKDIR}/cmake/Modules/FindSomePackage.cmake becomes invisible for
patches and the patch task ends up with error "No file to patch."

I would propose to return the OECMAKE_SOURCEPATH variable to the
cmake.bbclass, pass it to the cmake command as the "path to the CMake
file", and set its default value to ${S}

Regards

Mira


Signed-off-by: Mira Kes <miroslav.kes@gmail.com>

     fi
 
     if [ "${S}" != "${B}" ]; then
@@ -84,7 +88,7 @@ cmake_do_configure() {
 
     cmake \
       ${OECMAKE_SITEFILE} \
-      ${S} \
+      ${OECMAKE_SOURCEPATH} \
       -DCMAKE_INSTALL_PREFIX:PATH=${prefix} \
       -DCMAKE_INSTALL_BINDIR:PATH=${bindir} \
       -DCMAKE_INSTALL_SBINDIR:PATH=${sbindir} \
Ross Burton - June 13, 2014, 4:38 p.m.
On 13 June 2014 17:33, Miroslav Keš <miroslav.kes@gmail.com> wrote:
> +    if [ -z "${OECMAKE_SOURCEPATH}" ]; then
> +        OECMAKE_SOURCEPATH="${S}"
>      fi
>
>      if [ "${S}" != "${B}" ]; then
> @@ -84,7 +88,7 @@ cmake_do_configure() {
>
>      cmake \
>        ${OECMAKE_SITEFILE} \
> -      ${S} \
> +      ${OECMAKE_SOURCEPATH} \

A better idiom that's more self-documenting would be to set
OECMAKE_SOURCEPATH ?= "${S}" at the top-level.

Would it be sensible to give that variable a different name as it
refers specifically to the location of the cmake file, and not the
rest of the source?

Ross
Miroslav Keš - June 13, 2014, 4:51 p.m.
On 06/13/14 18:38, Burton, Ross wrote:
> On 13 June 2014 17:33, Miroslav Keš <miroslav.kes@gmail.com> wrote:
>> +    if [ -z "${OECMAKE_SOURCEPATH}" ]; then
>> +        OECMAKE_SOURCEPATH="${S}"
>>      fi
>>
>>      if [ "${S}" != "${B}" ]; then
>> @@ -84,7 +88,7 @@ cmake_do_configure() {
>>
>>      cmake \
>>        ${OECMAKE_SITEFILE} \
>> -      ${S} \
>> +      ${OECMAKE_SOURCEPATH} \
> A better idiom that's more self-documenting would be to set
> OECMAKE_SOURCEPATH ?= "${S}" at the top-level.

You are right.

> Would it be sensible to give that variable a different name as it
> refers specifically to the location of the cmake file, and not the
> rest of the source?

I was thinking about a better name too.
The reason why I stayed with the OECMAKE_SOURCEPATH is that the cmake
man page says:

USAGE

  cmake [options] <path-to-source>

And that's the directory that I want to set. So I thought that it would
be understandable for people familiar with cmake.

Mira
> Ross

Patch

diff --git a/meta/classes/cmake.bbclass b/meta/classes/cmake.bbclass
index c9c15f3..95a1cbf 100644
--- a/meta/classes/cmake.bbclass
+++ b/meta/classes/cmake.bbclass
@@ -65,8 +65,12 @@  EOF
 addtask generate_toolchain_file after do_patch before do_configure
 
 cmake_do_configure() {
-    if [ "${OECMAKE_BUILDPATH}" -o "${OECMAKE_SOURCEPATH}" ]; then
-        bbnote "cmake.bbclass no longer uses OECMAKE_SOURCEPATH and
OECMAKE_BUILDPATH.  The default behaviour is now out-of-tree builds with
B=WORKDIR/build."
+    if [ "${OECMAKE_BUILDPATH}" ]; then
+        bbnote "cmake.bbclass no longer uses OECMAKE_BUILDPATH.  The
default behaviour is now out-of-tree builds with B=WORKDIR/build."
+    fi
+
+    if [ -z "${OECMAKE_SOURCEPATH}" ]; then
+        OECMAKE_SOURCEPATH="${S}"