Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Yann E. MORIN <yann.morin.1998@free.fr>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH v2 2/4] core: allow check-host-cmake.sh to try several candidates
Date: Sun, 7 May 2017 11:47:06 +0200	[thread overview]
Message-ID: <20170507094706.GB2949@scaer> (raw)
In-Reply-To: <1494131541-2960-2-git-send-email-casantos@datacom.ind.br>

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.  |
'------------------------------^-------^------------------^--------------------'

  reply	other threads:[~2017-05-07  9:47 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-07  4:32 [Buildroot] [PATCH v2 1/4] core: reverse the argument order in check-host-cmake Carlos Santos
2017-05-07  4:32 ` [Buildroot] [PATCH v2 2/4] core: allow check-host-cmake.sh to try several candidates Carlos Santos
2017-05-07  9:47   ` Yann E. MORIN [this message]
2017-05-07 15:28     ` Carlos Santos
2017-05-07 16:11       ` Yann E. MORIN
2017-05-08 17:14         ` Carlos Santos
2017-05-07  4:32 ` [Buildroot] [PATCH v2 3/4] core: allow having a list of "cmake" candidates Carlos Santos
2017-05-07  4:32 ` [Buildroot] [PATCH v2 4/4] core: add "cmake3" to the list of cmake candidates Carlos Santos
2017-05-07  9:40 ` [Buildroot] [PATCH v2 1/4] core: reverse the argument order in check-host-cmake Yann E. MORIN
2017-06-24 19:46 ` Thomas Petazzoni

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170507094706.GB2949@scaer \
    --to=yann.morin.1998@free.fr \
    --cc=buildroot@busybox.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox