[Buildroot] [PATCH v2 1/4] core: reverse the argument order in check-host-cmake

[Buildroot] [PATCH v2 1/4] core: reverse the argument order in check-host-cmake

[Carlos Santos]

Pass the minimal version before the program name. In a later change the
script will become able to test a list of candidates.

Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
---
 support/dependencies/check-host-cmake.mk | 2 +-
 support/dependencies/check-host-cmake.sh | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/support/dependencies/check-host-cmake.mk b/support/dependencies/check-host-cmake.mk
index 8002278..914327f 100644
--- a/support/dependencies/check-host-cmake.mk
+++ b/support/dependencies/check-host-cmake.mk
@@ -12,7 +12,7 @@ BR2_CMAKE_VERSION_MIN = 3.1
 
 BR2_CMAKE ?= cmake
 ifeq ($(call suitable-host-package,cmake,\
-	$(BR2_CMAKE) $(BR2_CMAKE_VERSION_MIN)),)
+	$(BR2_CMAKE_VERSION_MIN) $(BR2_CMAKE)),)
 BR2_CMAKE = $(HOST_DIR)/usr/bin/cmake
 BR2_CMAKE_HOST_DEPENDENCY = host-cmake
 endif
diff --git a/support/dependencies/check-host-cmake.sh b/support/dependencies/check-host-cmake.sh
index 9b63b06..73bd4ed 100755
--- a/support/dependencies/check-host-cmake.sh
+++ b/support/dependencies/check-host-cmake.sh
@@ -1,7 +1,7 @@
 #!/bin/sh
 
-candidate="${1}"
-version_min="${2}"
+version_min="${1}"
+candidate="${2}"
 
 major_min="${version_min%.*}"
 minor_min="${version_min#*.}"
-- 
2.7.4

[Buildroot] [PATCH v2 2/4] core: allow check-host-cmake.sh to try several candidates

[Carlos Santos]

This is useful on CentOS 7 whose "cmake" package provides cmake 2.8.12,
which is too old, but the "cmake3" package (from EPEL) provides version
3.6.3, which is satisfactory. Examples:

    $ sh support/dependencies/check-host-cmake.sh 2.8 cmake cmake3
    /usr/bin/cmake

    $ sh support/dependencies/check-host-cmake.sh 3.1 cmake cmake3
    /usr/bin/cmake3

    $ sh support/dependencies/check-host-cmake.sh 3.8 cmake cmake3
    (nothing)

Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
---
 support/dependencies/check-host-cmake.sh | 76 +++++++++++++++++---------------
 1 file changed, 41 insertions(+), 35 deletions(-)

diff --git a/support/dependencies/check-host-cmake.sh b/support/dependencies/check-host-cmake.sh
index 73bd4ed..fadeae9 100755
--- a/support/dependencies/check-host-cmake.sh
+++ b/support/dependencies/check-host-cmake.sh
@@ -1,39 +1,45 @@
 #!/bin/sh
 
-version_min="${1}"
-candidate="${2}"
-
-major_min="${version_min%.*}"
-minor_min="${version_min#*.}"
-
-cmake=`which ${candidate}`
-if [ ! -x "${cmake}" ]; then
-    # echo nothing: no suitable cmake found
-    exit 1
-fi
-
-# Extract version X.Y from versions in the form X.Y or X.Y.Z
-# with X, Y and Z numbers with one or more digits each, e.g.
-#   3.2     -> 3.2
-#   3.2.3   -> 3.2
-#   3.2.42  -> 3.2
-#   3.10    -> 3.10
-#   3.10.4  -> 3.10
-#   3.10.42 -> 3.10
-version="$(${cmake} --version \
-           |sed -r -e '/.* ([[:digit:]]+\.[[:digit:]]+).*$/!d;' \
-                   -e 's//\1/'
-          )"
-major="${version%.*}"
-minor="${version#*.}"
-
-if [ ${major} -gt ${major_min} ]; then
-    echo "${cmake}"
-else
-    if [ ${major} -eq ${major_min} -a ${minor} -ge ${minor_min} ]; then
+# prevent shift error
+[ $# -lt 2 ] && exit 1
+
+major_min="${1%.*}"
+minor_min="${1#*.}"
+
+shift
+
+for candidate; do
+
+    # Try to locate the candidate. Discard it if not located.
+    cmake=`which "${candidate}" 2>/dev/null`
+    [ -n "${cmake}" ] || continue
+
+    # Extract version X.Y from versions in the form X.Y or X.Y.Z
+    # with X, Y and Z numbers with one or more digits each, e.g.
+    #   3.2     -> 3.2
+    #   3.2.3   -> 3.2
+    #   3.2.42  -> 3.2
+    #   3.10    -> 3.10
+    #   3.10.4  -> 3.10
+    #   3.10.42 -> 3.10
+    # Discard the candidate if no version can be obtained
+    version="$(${cmake} --version \
+               |sed -r -e '/.* ([[:digit:]]+\.[[:digit:]]+).*$/!d;' \
+                       -e 's//\1/'
+              )"
+    [ -n "${version}" ] || continue
+
+    major="${version%.*}"
+    minor="${version#*.}"
+
+    if [ ${major} -gt ${major_min} ]; then
         echo "${cmake}"
-    else
-        # echo nothing: no suitable cmake found
-        exit 1
+        exit
+    elif [ ${major} -eq ${major_min} -a ${minor} -ge ${minor_min} ]; then
+        echo "${cmake}"
+        exit
     fi
-fi
+done
+
+# echo nothing: no suitable cmake found
+exit 1
-- 
2.7.4

[Buildroot] [PATCH v2 3/4] core: allow having a list of "cmake" candidates

[Carlos Santos]

Add the BR2_CMAKE_CANDIDATES variable, containing a list of candidates
to check and use as BR2_CMAKE, if possible.

This allows using "cmake3" on CentOS 7, whose default cmake corresponds
to version 2.8.12. Example:

    $ make BR2_CMAKE_CANDIDATES="cmake cmake3"

Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 support/dependencies/check-host-cmake.mk | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/support/dependencies/check-host-cmake.mk b/support/dependencies/check-host-cmake.mk
index 914327f..ef080ff 100644
--- a/support/dependencies/check-host-cmake.mk
+++ b/support/dependencies/check-host-cmake.mk
@@ -10,9 +10,10 @@
 #
 BR2_CMAKE_VERSION_MIN = 3.1
 
-BR2_CMAKE ?= cmake
-ifeq ($(call suitable-host-package,cmake,\
-	$(BR2_CMAKE_VERSION_MIN) $(BR2_CMAKE)),)
+BR2_CMAKE_CANDIDATES ?= cmake
+BR2_CMAKE ?= $(call suitable-host-package,cmake,\
+	$(BR2_CMAKE_VERSION_MIN) $(BR2_CMAKE_CANDIDATES))
+ifeq ($(BR2_CMAKE),)
 BR2_CMAKE = $(HOST_DIR)/usr/bin/cmake
 BR2_CMAKE_HOST_DEPENDENCY = host-cmake
 endif
-- 
2.7.4

[Buildroot] [PATCH v2 4/4] core: add "cmake3" to the list of cmake candidates

[Carlos Santos]

This is useful on CentOS 7, whose "cmake" utility corresponds to version
2.8.12, which is too old for Buildroot.

Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 support/dependencies/check-host-cmake.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/support/dependencies/check-host-cmake.mk b/support/dependencies/check-host-cmake.mk
index ef080ff..07b064c 100644
--- a/support/dependencies/check-host-cmake.mk
+++ b/support/dependencies/check-host-cmake.mk
@@ -10,7 +10,7 @@
 #
 BR2_CMAKE_VERSION_MIN = 3.1
 
-BR2_CMAKE_CANDIDATES ?= cmake
+BR2_CMAKE_CANDIDATES ?= cmake cmake3
 BR2_CMAKE ?= $(call suitable-host-package,cmake,\
 	$(BR2_CMAKE_VERSION_MIN) $(BR2_CMAKE_CANDIDATES))
 ifeq ($(BR2_CMAKE),)
-- 
2.7.4

[Buildroot] [PATCH v2 1/4] core: reverse the argument order in check-host-cmake

[Yann E. MORIN]

Carlos, All,

On 2017-05-07 01:32 -0300, Carlos Santos spake thusly:
> Pass the minimal version before the program name. In a later change the
> script will become able to test a list of candidates.
> 
> Signed-off-by: Carlos Santos <casantos@datacom.ind.br>

Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Regards,
Yann E. MORIN.

> ---
>  support/dependencies/check-host-cmake.mk | 2 +-
>  support/dependencies/check-host-cmake.sh | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/support/dependencies/check-host-cmake.mk b/support/dependencies/check-host-cmake.mk
> index 8002278..914327f 100644
> --- a/support/dependencies/check-host-cmake.mk
> +++ b/support/dependencies/check-host-cmake.mk
> @@ -12,7 +12,7 @@ BR2_CMAKE_VERSION_MIN = 3.1
>  
>  BR2_CMAKE ?= cmake
>  ifeq ($(call suitable-host-package,cmake,\
> -	$(BR2_CMAKE) $(BR2_CMAKE_VERSION_MIN)),)
> +	$(BR2_CMAKE_VERSION_MIN) $(BR2_CMAKE)),)
>  BR2_CMAKE = $(HOST_DIR)/usr/bin/cmake
>  BR2_CMAKE_HOST_DEPENDENCY = host-cmake
>  endif
> diff --git a/support/dependencies/check-host-cmake.sh b/support/dependencies/check-host-cmake.sh
> index 9b63b06..73bd4ed 100755
> --- a/support/dependencies/check-host-cmake.sh
> +++ b/support/dependencies/check-host-cmake.sh
> @@ -1,7 +1,7 @@
>  #!/bin/sh
>  
> -candidate="${1}"
> -version_min="${2}"
> +version_min="${1}"
> +candidate="${2}"
>  
>  major_min="${version_min%.*}"
>  minor_min="${version_min#*.}"
> -- 
> 2.7.4
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v2 2/4] core: allow check-host-cmake.sh to try several candidates

[Yann E. MORIN]

Carlos, All,

On 2017-05-07 01:32 -0300, Carlos Santos spake thusly:
> This is useful on CentOS 7 whose "cmake" package provides cmake 2.8.12,
> which is too old, but the "cmake3" package (from EPEL) provides version
> 3.6.3, which is satisfactory. Examples:
> 
>     $ sh support/dependencies/check-host-cmake.sh 2.8 cmake cmake3
>     /usr/bin/cmake
> 
>     $ sh support/dependencies/check-host-cmake.sh 3.1 cmake cmake3
>     /usr/bin/cmake3
> 
>     $ sh support/dependencies/check-host-cmake.sh 3.8 cmake cmake3
>     (nothing)
> 
> Signed-off-by: Carlos Santos <casantos@datacom.ind.br>

Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

However, there is a small nit-pick I'd like to point out...

[--SNIP--]
> +for candidate; do
> +
> +    # Try to locate the candidate. Discard it if not located.
> +    cmake=`which "${candidate}" 2>/dev/null`
> +    [ -n "${cmake}" ] || continue
> +
> +    # Extract version X.Y from versions in the form X.Y or X.Y.Z
> +    # with X, Y and Z numbers with one or more digits each, e.g.
> +    #   3.2     -> 3.2
> +    #   3.2.3   -> 3.2
> +    #   3.2.42  -> 3.2
> +    #   3.10    -> 3.10
> +    #   3.10.4  -> 3.10
> +    #   3.10.42 -> 3.10
> +    # Discard the candidate if no version can be obtained
> +    version="$(${cmake} --version \
> +               |sed -r -e '/.* ([[:digit:]]+\.[[:digit:]]+).*$/!d;' \
> +                       -e 's//\1/'
> +              )"
> +    [ -n "${version}" ] || continue

... here: the check that version is not empty is new, and semantically
it looks like it should have been in a spearate patch (prossibly before
that one).

Unless it is the act of testing multiple candidates that introduces a
case where the version is empty, but I fil to see how that would be.

In any case, don't re-spin just for that.

Regards,
Yann E. MORIN.

> +    major="${version%.*}"
> +    minor="${version#*.}"
> +
> +    if [ ${major} -gt ${major_min} ]; then
>          echo "${cmake}"
> -    else
> -        # echo nothing: no suitable cmake found
> -        exit 1
> +        exit
> +    elif [ ${major} -eq ${major_min} -a ${minor} -ge ${minor_min} ]; then
> +        echo "${cmake}"
> +        exit
>      fi
> -fi
> +done
> +
> +# echo nothing: no suitable cmake found
> +exit 1
> -- 
> 2.7.4
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v2 2/4] core: allow check-host-cmake.sh to try several candidates

[Carlos Santos]

> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> To: "Carlos Santos" <casantos@datacom.ind.br>
> Cc: buildroot at buildroot.org
> Sent: Sunday, May 7, 2017 6:47:06 AM
> Subject: Re: [PATCH v2 2/4] core: allow check-host-cmake.sh to try several candidates

> Carlos, All,
---8<---
>> +    # Discard the candidate if no version can be obtained
>> +    version="$(${cmake} --version \
>> +               |sed -r -e '/.* ([[:digit:]]+\.[[:digit:]]+).*$/!d;' \
>> +                       -e 's//\1/'
>> +              )"
>> +    [ -n "${version}" ] || continue
> 
> ... here: the check that version is not empty is new, and semantically
> it looks like it should have been in a spearate patch (prossibly before
> that one).
> 
> Unless it is the act of testing multiple candidates that introduces a
> case where the version is empty, but I fil to see how that would be.

The problem here is that if $version is empty both $major and $minor
become empty too and the test below emits an annoying error message.
Example (commenting the '[ -n "${version}" ] || continue' line):

    $ sh check-host-cmake.sh 3.1 python
    Python 2.7.12
    check-host-cmake.sh: 35: [: -gt: unexpected operator
    check-host-cmake.sh: 38: [: -eq: unexpected operator

Raising even more the paranoia level, the script is also fragile in
other aspects, e.g

    $ bash --version
    GNU bash, version 4.3.46(1)-release (x86_64-pc-linux-gnu)
    ---8<---
    $  sh check-host-cmake.sh 3.1 bash
    /bin/bash
    $ cat --version
    cat (GNU coreutils) 8.25
    ---8<---
    $ sh check-host-cmake.sh 3.1 cat
    /bin/cat

It is so fragile that I'm tempted to replace the regex by a
stricter one. Example:

    $ cmake --version | sed -n -r -e 's/^cmake version ([[:digit:]]+\.[[:digit:]]+).*$/\1/p'
    2.8
    $ cmake3 --version | sed -n -r -e 's/^cmake version ([[:digit:]]+\.[[:digit:]]+).*$/\1/p'
    3.6

-- 
Carlos Santos (Casantos) - DATACOM, P&D
?The greatest triumph that modern PR can offer is the transcendent 
success of having your words and actions judged by your reputation, 
rather than the other way about.? ? Christopher Hitchens

[Buildroot] [PATCH v2 2/4] core: allow check-host-cmake.sh to try several candidates

[Yann E. MORIN]

Carlos, All,

On 2017-05-07 12:28 -0300, Carlos Santos spake thusly:
> > From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > To: "Carlos Santos" <casantos@datacom.ind.br>
> > Cc: buildroot at buildroot.org
> > Sent: Sunday, May 7, 2017 6:47:06 AM
> > Subject: Re: [PATCH v2 2/4] core: allow check-host-cmake.sh to try several candidates
> 
> > Carlos, All,
> ---8<---
> >> +    # Discard the candidate if no version can be obtained
> >> +    version="$(${cmake} --version \
> >> +               |sed -r -e '/.* ([[:digit:]]+\.[[:digit:]]+).*$/!d;' \
> >> +                       -e 's//\1/'
> >> +              )"
> >> +    [ -n "${version}" ] || continue
> > 
> > ... here: the check that version is not empty is new, and semantically
> > it looks like it should have been in a spearate patch (prossibly before
> > that one).
> > 
> > Unless it is the act of testing multiple candidates that introduces a
> > case where the version is empty, but I fil to see how that would be.
> 
> The problem here is that if $version is empty both $major and $minor
> become empty too and the test below emits an annoying error message.
> Example (commenting the '[ -n "${version}" ] || continue' line):
> 
>     $ sh check-host-cmake.sh 3.1 python
>     Python 2.7.12
>     check-host-cmake.sh: 35: [: -gt: unexpected operator
>     check-host-cmake.sh: 38: [: -eq: unexpected operator

Well, this can only happen in one condition: the 'cmake' (or 'cmake3')
executable is in fact not CMake, which would be an excessively stupid
situation...

> Raising even more the paranoia level, the script is also fragile in
> other aspects, e.g
> 
>     $ bash --version
>     GNU bash, version 4.3.46(1)-release (x86_64-pc-linux-gnu)
>     ---8<---
>     $  sh check-host-cmake.sh 3.1 bash
>     /bin/bash
>     $ cat --version
>     cat (GNU coreutils) 8.25
>     ---8<---
>     $ sh check-host-cmake.sh 3.1 cat
>     /bin/cat

YUes, if you call the script manually, which it is not intended to be.
Remeber that the script is only called if the user did not explicitly
pass BR2_CMAKE, and its arguments are always the minimum version and the
candidates, which are always 'cmake' and 'cmake3'.

> It is so fragile that I'm tempted to replace the regex by a
> stricter one. Example:
> 
>     $ cmake --version | sed -n -r -e 's/^cmake version ([[:digit:]]+\.[[:digit:]]+).*$/\1/p'
>     2.8
>     $ cmake3 --version | sed -n -r -e 's/^cmake version ([[:digit:]]+\.[[:digit:]]+).*$/\1/p'
>     3.6

I'm not against having a stricter regex, but remember what you are
trying to protect against... And I personally don't think the cases you
point at would ever happen, unless one of the candidates is in fact not
CMake.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v2 2/4] core: allow check-host-cmake.sh to try several candidates

[Carlos Santos]

> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> To: "Carlos Santos" <casantos@datacom.ind.br>
> Cc: buildroot at buildroot.org
> Sent: Sunday, May 7, 2017 1:11:32 PM
> Subject: Re: [PATCH v2 2/4] core: allow check-host-cmake.sh to try several candidates

>>     $ sh check-host-cmake.sh 3.1 python
>>     Python 2.7.12
>>     check-host-cmake.sh: 35: [: -gt: unexpected operator
>>     check-host-cmake.sh: 38: [: -eq: unexpected operator
> 
> Well, this can only happen in one condition: the 'cmake' (or 'cmake3')
> executable is in fact not CMake, which would be an excessively stupid
> situation...
---8<---
> 
> I'm not against having a stricter regex, but remember what you are
> trying to protect against... And I personally don't think the cases you
> point at would ever happen, unless one of the candidates is in fact not
> CMake.

OK, let's keep the paranoia at its current level. :-)

-- 
Carlos Santos (Casantos) - DATACOM, P&D
?The greatest triumph that modern PR can offer is the transcendent 
success of having your words and actions judged by your reputation, 
rather than the other way about.? ? Christopher Hitchens

[Buildroot] [PATCH v2 1/4] core: reverse the argument order in check-host-cmake

[Thomas Petazzoni]

Hello,

On Sun,  7 May 2017 01:32:18 -0300, Carlos Santos wrote:
> Pass the minimal version before the program name. In a later change the
> script will become able to test a list of candidates.
> 
> Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
> ---
>  support/dependencies/check-host-cmake.mk | 2 +-
>  support/dependencies/check-host-cmake.sh | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)

Thanks, series applied to master! And thanks to Yann for reviewing
those patches.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com