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

login
register
mail settings
Submitter Miroslav Keš
Date June 17, 2014, 8:20 p.m.
Message ID <53A0A2F0.2020701@gmail.com>
Download mbox | patch
Permalink /patch/73973/
State New
Headers show

Comments

Miroslav Keš - June 17, 2014, 8:20 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.
>
> 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

Here is the updated patch:

Signed-off-by: Mira Kes <miroslav.kes@gmail.com>
Saul Wold - June 17, 2014, 8:36 p.m.
On 06/17/2014 01:20 PM, Miroslav Keš wrote:
> 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.
>>
>> 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
>
> Here is the updated patch:
>

Can you send this as a proper v2 patch please.
Thanks
	Sau!

> Signed-off-by: Mira Kes <miroslav.kes@gmail.com>
>
> diff --git a/meta/classes/cmake.bbclass b/meta/classes/cmake.bbclass
> index c9c15f3..f762792 100644
> --- a/meta/classes/cmake.bbclass
> +++ b/meta/classes/cmake.bbclass
> @@ -23,6 +23,9 @@ OECMAKE_RPATH ?= ""
>   OECMAKE_PERLNATIVE_DIR ??= ""
>   OECMAKE_EXTRA_ROOT_PATH ?= ""
>
> +# Path to the CMake file to process.
> +OECMAKE_SOURCEPATH ?= "${S}"
> +
>   cmake_do_generate_toolchain_file() {
>          cat > ${WORKDIR}/toolchain.cmake <<EOF
>   # CMake system name must be something like "Linux".
> @@ -65,8 +68,8 @@ 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 [ "${S}" != "${B}" ]; then
> @@ -84,7 +87,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 17, 2014, 9:21 p.m.
Apart from what Saul said, this looks good to me.

Ross

On 17 June 2014 21:20, Miroslav Keš <miroslav.kes@gmail.com> wrote:
> 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.
>>
>> 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
>
> Here is the updated patch:
>
> Signed-off-by: Mira Kes <miroslav.kes@gmail.com>
>
> diff --git a/meta/classes/cmake.bbclass b/meta/classes/cmake.bbclass
> index c9c15f3..f762792 100644
> --- a/meta/classes/cmake.bbclass
> +++ b/meta/classes/cmake.bbclass
> @@ -23,6 +23,9 @@ OECMAKE_RPATH ?= ""
>  OECMAKE_PERLNATIVE_DIR ??= ""
>  OECMAKE_EXTRA_ROOT_PATH ?= ""
>
> +# Path to the CMake file to process.
> +OECMAKE_SOURCEPATH ?= "${S}"
> +
>  cmake_do_generate_toolchain_file() {
>         cat > ${WORKDIR}/toolchain.cmake <<EOF
>  # CMake system name must be something like "Linux".
> @@ -65,8 +68,8 @@ 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 [ "${S}" != "${B}" ]; then
> @@ -84,7 +87,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} \
>
>

Patch

diff --git a/meta/classes/cmake.bbclass b/meta/classes/cmake.bbclass
index c9c15f3..f762792 100644
--- a/meta/classes/cmake.bbclass
+++ b/meta/classes/cmake.bbclass
@@ -23,6 +23,9 @@  OECMAKE_RPATH ?= ""
 OECMAKE_PERLNATIVE_DIR ??= ""
 OECMAKE_EXTRA_ROOT_PATH ?= ""
 
+# Path to the CMake file to process.
+OECMAKE_SOURCEPATH ?= "${S}"
+
 cmake_do_generate_toolchain_file() {
        cat > ${WORKDIR}/toolchain.cmake <<EOF
 # CMake system name must be something like "Linux".
@@ -65,8 +68,8 @@  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 [ "${S}" != "${B}" ]; then
@@ -84,7 +87,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} \