[Buildroot] [PATCH 4/7 v4] download/git: try to recover from utterly-broken repositories

Yann E. MORIN yann.morin.1998 at free.fr
Sun Apr 29 21:24:21 UTC 2018


Ricardo, All,

On 2018-04-29 15:49 -0300, Ricardo Martincoski spake thusly:
> Using the previous iteration I tested adding '&& false' after the git checkout
> command and the trap worked as expected.

I did test with a similar trick, just adding 'false' on its own line.

> But since this patch is not easy to me to review (my bash scripts use a small
> subset of bash features), I decided to do more extensive testing. I think I
> found something to be changed. Could you double-check? See at the end.

Gah, you're right. I did not know how to actually test that script
except with the aforementioned 'false' hack. Thanks for the hint of
making HEAD a directory.

Now I can indeed see that the hook is not called. Using set -E as you
suggested makes it work. I've fixed locally.

Note: typoes all fixed, thanks!

> On Sat, Apr 28, 2018 at 04:34 PM, Yann E. MORIN wrote:
> 
> > +++ b/support/download/git
> > @@ -16,6 +16,33 @@ set -e
> >  # Environment:
> >  #   GIT      : the git command to call
> >  
> > +# Save out path and options in case we need to call ourselves again
> 
> s/out/our/ ?
> 
> > +myname="${0}"
> > +declare -a OPTS=("${@}")
> 
> It looks OK to me.
> Similar syntax is used in support/scripts/check-bin-arch and no one using old
> bash versions complained so far.

Indexed arrays have been in bash for ages now. That is, since bash-2.0,
which was released 1996-12-31.

Regards,
Yann E. MORIN.

> I didn't checked the bash version available on RHEL6 neither the bash source
> code.
> 
> > +
> > +# This function is called when an error occurs. Its job is to attempt a
> > +# clone from scratch (only once!) in case the git tree is borked, or in
> > +# case an unexpected and unsupported situation arises with submodules
> > +# or uncomitted stuff (e.g. if the user manually mucked around in the
> 
> s/uncomitted/uncommitted/ ?
> 
> > +# git cache).
> > +_on_error() {
> > +    local ret=${?}
> > +
> > +    printf "Detected a corrupted git cache.\n" >&2
> > +    if ${BR_GIT_BACKEND_FIRST_FAULT:-false}; then
> > +        printf "This is the second time in a row; bailing out\n" >&2
> > +        exit ${ret}
> > +    fi
> > +    export BR_GIT_BACKEND_FIRST_FAULT=true
> > +
> > +    printf "Removing it and starting afresh.\n" >&2
> > +
> > +    popd >/dev/null
> > +    rm -rf "${git_cache}"
> > +
> > +    exec "${myname}" "${OPTS[@]}" || exit ${ret}
> > +}
> > +
> >  verbose=
> >  recurse=0
> >  while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do
> > @@ -39,6 +66,9 @@ git_cache="${dl_dir}/git"
> >  mkdir -p "${git_cache}"
> >  pushd "${git_cache}" >/dev/null
> >  
> > +# Any error now should try to recover
> > +trap _on_error ERR
> > +
> >  # Caller needs to single-quote its arguments to prevent them from
> >  # being expanded a second time (in case there are spaces in them)
> >  _git() {
> 
> It seems the trap is not executed when a git command fails.
> 
> I first created a git cache. I used squashfs because it is small, its server is
> fast, and it has hash to check against.
> I removed .git/index in the cache so the next git init will create a brand new
> one leaving lots of untracked files to be overwritten. Since the -f is not yet
> added to git checkout (it belongs to a later patch) it should trigger a
> ditch+restart, but it didn't.
> I recreated the cache from scratch and replaced .git/HEAD with a directory. This
> way the cache is broken and git init does not recovers it, as expected, leading
> 'git remote' to fail. It also should trigger a ditch+restart, but it didn't.
> 
> With the help of [1] I found in the bash manual, not mentioned in the SHELL
> BUILTIN COMMANDS/trap section, only in the FUNCTIONS section:
> "FUNCTIONS" [...]
> "All other aspects of the shell execution environment are identical between a
>  function and its caller with these exceptions:" [...]
> "the ERR trap is not inherited unless the -o errtrace shell option has been
>  enabled."
> 
> As a dirty hack I repeated the trap inside _git:
>   _git() {
>  +    trap _on_error ERR
> It fixes the 2 cases I mentioned above.
> 
> Maybe the proper fix is to enable errtrace, but I didn't tested it, neither I
> know all the implications of using this for all the other cases.
> From the manual:
> "-E      If set, any trap on ERR is inherited by shell functions, command
>  substitutions, and commands executed in a subshell environment. The ERR trap is
>  normally not inherited in such cases."
> 
> [1] https://unix.stackexchange.com/questions/419017/return-trap-in-bash-not-executing-for-function
> 
> Regards,
> Ricardo


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



More information about the buildroot mailing list