All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yann E. MORIN <yann.morin.1998@free.fr>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCHv3 2/7] support/download/hg: implement source-check
Date: Sat, 9 Feb 2019 22:53:19 +0100	[thread overview]
Message-ID: <20190209215319.GH3079@scaer> (raw)
In-Reply-To: <20190209202350.4984-2-patrickdepinguin@gmail.com>

Thomas, All,

On 2019-02-09 21:23 +0100, Thomas De Schampheleire spake thusly:
> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> 
> Note: the implementation is different (better) than what used to be in
> Buildroot before source-check was removed.
> 
> The original implementation:
>     hg incoming --force -l1 <URL>
> would only verify that the repository exists, not that the requested
> revision is present.
> 
> An already better implementation is:
>     hg incoming --force -l1 -r <revision> <URL>
> but compared to the next solution it has a large resource consumption on the
> local machine. In the background, the full repository is first downloaded.
> 
> The implemented solution is:
>     hg identify -r <revision> <URL>
> which operates directly on the remote repository.
> 
> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> ---
>  support/download/hg | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> v3: no changes
> 
> diff --git a/support/download/hg b/support/download/hg
> index efb515fca5..ed8dcfbcff 100755
> --- a/support/download/hg
> +++ b/support/download/hg
> @@ -7,6 +7,7 @@ set -e
>  #
>  # Options:
>  #   -q          Be quiet.
> +#   -C          Only check that the changeset exists in the remote repository.
>  #   -o FILE     Generate archive in FILE.
>  #   -u URI      Clone from repository at URI.
>  #   -c CSET     Use changeset (or revision) CSET.
> @@ -19,6 +20,7 @@ verbose=
>  while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do
>      case "${OPT}" in
>      q)  verbose=-q;;
> +    C)  checkonly=1;;

I've come to like an alternate solution to handle boolean options:

    checkonly=false
    while getopts ...; do
        case "${OPT}" in
            C)  checkonly=true;;
        esac
    done

Which allows to write nicer conditions:

    if ${checkonly}; then
        ...
    fi

>      o)  output="${OPTARG}";;
>      u)  uri="${OPTARG}";;
>      c)  cset="${OPTARG}";;
> @@ -36,6 +38,11 @@ _hg() {
>      eval ${HG} "${@}"
>  }
>  
> +if [ -n "${checkonly}" ]; then
> +    _hg identify ${verbose} "${@}" --rev "'${cset}'" "'${uri}'" > /dev/null
> +    exit ${?}

Incorrect use of exit. The script has a 'set -e' at the beginning, so a
command that exits in error will cause the script to abort. So, if the
script reaches the exit clause, it means the call to _hg did not fail.
Ditto the other backends, of course.

Also, I think it always makes sense to check that the revision exists,
even if doing the actual download: it allows for a fastpath even in case
the user wants to do the actual download anyway:

    if ! _hg identify ${verbose} "${@}" --rev "'${cset}'" "'${uri}'" > /dev/null; then
        printf 'error blabla no such revision blabla\n'
        exit 1
    fi
    if ${checkonly}; then exit 0; fi

Note that we did use to have a similar test with git, but it was nt
reliable (i.e. old git servers and/or configuration of git server that
did not behave when ls-remote was fed some forms of refs, like a sha1 or
susch)

Regards,
Yann E. MORIN.

> +fi
> +
>  _hg clone ${verbose} "${@}" --noupdate "'${uri}'" "'${basename}'"
>  
>  _hg archive ${verbose} --repository "'${basename}'" --type tgz \
> -- 
> 2.19.2
> 

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

  reply	other threads:[~2019-02-09 21:53 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-09 20:23 [Buildroot] [PATCHv3 1/7] support/download: reintroduce 'source-check' target Thomas De Schampheleire
2019-02-09 20:23 ` [Buildroot] [PATCHv3 2/7] support/download/hg: implement source-check Thomas De Schampheleire
2019-02-09 21:53   ` Yann E. MORIN [this message]
2019-02-15 19:10     ` Thomas De Schampheleire
2019-02-09 20:23 ` [Buildroot] [PATCHv3 3/7] support/download/wget: " Thomas De Schampheleire
2019-02-09 20:23 ` [Buildroot] [PATCHv3 4/7] support/download/file: " Thomas De Schampheleire
2019-02-09 20:23 ` [Buildroot] [PATCHv3 5/7] Config.in: reintroduce BR2_SSH Thomas De Schampheleire
2019-02-09 20:23 ` [Buildroot] [PATCHv3 6/7] support/download/scp: implement source-check Thomas De Schampheleire
2019-02-09 22:09   ` Yann E. MORIN
2019-02-15 19:15     ` Thomas De Schampheleire
2019-02-15 21:00       ` Thomas De Schampheleire
2019-02-09 20:23 ` [Buildroot] [PATCHv3 7/7] support/download/{bzr, cvs, git, svn}: highlight unimplemented source-check Thomas De Schampheleire
2019-02-09 22:13   ` Yann E. MORIN
2019-02-15 19:15     ` Thomas De Schampheleire

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=20190209215319.GH3079@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.