[Buildroot] [PATCH 1/7 v4] download/git: add warning not to use our git cache

[Buildroot] [PATCH 1/7 v4] download/git: add warning not to use our git cache

[Yann E. MORIN]

We really want the user not to use our git cache manually, or their
changes (committed or not) may eventually get lost.

So, add a warning file, not unlike the one we put in the target/
directory, to warn the user not to use the git tree.

Ideally, we would have carried this file in support/misc/, but the git
backend does not have acces to it: the working directory is somewhere
unknown, and TOPDIR is not exported in the environment.

So, we have to carry it in-line in the backend instead.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
---
 support/download/git | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/support/download/git b/support/download/git
index bf05c595a5..3b5c8a6cfe 100755
--- a/support/download/git
+++ b/support/download/git
@@ -43,6 +43,24 @@ _git() {
     eval GIT_DIR="${git_cache}/.git" ${GIT} "${@}"
 }
 
+# Create a warning file, that the user should not use the git cache.
+# It's ours. Our precious.
+cat <<-_EOF_ >"${dl_dir}/git.readme"
+	IMPORTANT NOTE!
+
+	The git tree located in this directory is for the exclusive use
+	by Buildroot, which uses it as a local cache to reduce bandwidth
+	usage.
+
+	Buildroot *will* trash any changes in that tree whenever it needs
+	to use it. Buildroot may even remove it in case it detects the
+	repository may have been damaged or corrupted.
+
+	Do *not* work in that directory; your changes will eventually get
+	lost. Do *not* even use it as a remote, or as the source for new
+	worktrees; your commits will eventually get lost.
+_EOF_
+
 # Initialise a repository in the git cache. If the repository already
 # existed, this is a noop, unless the repository was broken, in which
 # case this magically restores it to working conditions. In the latter
-- 
2.14.1

[Buildroot] [PATCH 0/7 v4] support/download: make the git backend even more robust

[Yann E. MORIN]

Hello All!

This series makes our git backend even more robust than what we
currently have. Especially, it will try to recover from a repository
that was so utterly butchered that even a git-init can't salvage it.

The most significant changes are:

  - ensure we can checkout from an unclean state;

  - ensure we can checkout across changes in submodules setup;

  - as a last-ditch recovery, trash the local cache and clone again
    from scratch.

Additionally, we also remove support for shallow clones, because they
were in fact fundamentally broken, and did only work by chance.

Finally, we add a warning file, that the user should not use our git
cache for development, neither directly in it nor as a remote or the
origin for worktrees.

Thanks a lot to Ricardo, Arnout and Thomas for their inputs during the
discussions that led to this series. :-)

---
Changes v3 -> v4:
  - git clean --ffdx  is needed in the same patch that fircs the
    checkout  (Ricardo)

Changes v2 -> v3:
  - do not trash the cache if the cset is missing  (Thomas)


Regards,
Yann E. MORIN.


The following changes since commit 371204253273ba2984104e2c7a59f6afb657cd81

  libcgicc: add hash of license files (2018-04-28 19:02:03 +0200)


are available in the git repository at:

  git://git.buildroot.org/~ymorin/git/buildroot.git

for you to fetch changes up to b047fa7d0f366bfe767231112121e22cfc88f0de

  download/git: always do full-clone (2018-04-28 21:17:34 +0200)


----------------------------------------------------------------
Yann E. MORIN (7):
      download/git: add warning not to use our git cache
      download/git: run all git commands in the current directory
      download/git: quickly exit when the cset does not exist
      download/git: try to recover from utterly-broken repositories
      download/git: ensure we checkout to a clean state
      download/git: ensure we can checkout repos with submodule conversions
      download/git: always do full-clone

 support/download/git | 121 ++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 95 insertions(+), 26 deletions(-)

-- 
.-----------------.--------------------.------------------.--------------------.
|  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 2/7 v4] download/git: run all git commands in the current directory

[Yann E. MORIN]

That way, we can pushd earlier, which will help with last-ditch recovery
in a followup commit.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
---
 support/download/git | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/support/download/git b/support/download/git
index 3b5c8a6cfe..60d6c24f1e 100755
--- a/support/download/git
+++ b/support/download/git
@@ -34,8 +34,10 @@ done
 
 shift $((OPTIND-1)) # Get rid of our options
 
-# We want to check if a cache of the git clone of this repo already exists.
+# Create and cd into the directory that will contain the local git cache
 git_cache="${dl_dir}/git"
+mkdir -p "${git_cache}"
+pushd "${git_cache}" >/dev/null
 
 # Caller needs to single-quote its arguments to prevent them from
 # being expanded a second time (in case there are spaces in them)
@@ -70,9 +72,7 @@ _EOF_
 # We can still go through the wrapper, because 'init' does not use the
 # path pointed to by GIT_DIR, but really uses the directory passed as
 # argument.
-_git init "'${git_cache}'"
-
-pushd "${git_cache}" >/dev/null
+_git init .
 
 # Ensure the repo has an origin (in case a previous run was killed).
 if ! _git remote |grep -q -E '^origin$'; then
-- 
2.14.1

[Buildroot] [PATCH 3/7 v4] download/git: quickly exit when the cset does not exist

[Yann E. MORIN]

Check that the given cset is indeed something we can checkout. If not,
then exit early.

This will be usefull when a later commit will trap any failing git
command to try to recover the repository by doing a clone from scratch:
when the cset is not a commit, it does not mean the repository is broken
or what, and recloning from scratch would not help, so no need to trash
a good cache.

Reported-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
---
 support/download/git | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/support/download/git b/support/download/git
index 60d6c24f1e..bd37a0a8d9 100755
--- a/support/download/git
+++ b/support/download/git
@@ -114,6 +114,13 @@ if ! _git fetch origin "'${cset}:${cset}'" >/dev/null 2>&1; then
     printf "Could not fetch special ref '%s'; assuming it is not special.\n" "${cset}"
 fi
 
+# Check that the changeset does exist. If it does not, no reason to go
+# on, we can fast-track to the exit path.
+if ! _git rev-parse --quiet --verify "'${cset}^{commit}'" >/dev/null 2>&1; then
+    printf "Commit '%s' does not exist in this repository\n." "${cset}"
+    exit 1
+fi
+
 # Checkout the required changeset, so that we can update the required
 # submodules.
 _git checkout -q "'${cset}'"
-- 
2.14.1

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

[Yann E. MORIN]

In some cases, the repository may be in a state we can't automatically
recover from, especially since we must still support oldish git versions
that do not provide the necessary commands or options thereof.

As a last-ditch recovery, delete the repository and recreate the cache
from scratch.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
---
 support/download/git | 35 +++++++++++++++++++++++++++++++++--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/support/download/git b/support/download/git
index bd37a0a8d9..921a35998b 100755
--- a/support/download/git
+++ 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
+myname="${0}"
+declare -a OPTS=("${@}")
+
+# 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
+# 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() {
@@ -114,8 +144,9 @@ if ! _git fetch origin "'${cset}:${cset}'" >/dev/null 2>&1; then
     printf "Could not fetch special ref '%s'; assuming it is not special.\n" "${cset}"
 fi
 
-# Check that the changeset does exist. If it does not, no reason to go
-# on, we can fast-track to the exit path.
+# Check that the changeset does exist. If it does not, re-cloning from
+# scratch won't help, so we don't want to trash the repository for a
+# missing commit. We just exit without going through the ERR trap.
 if ! _git rev-parse --quiet --verify "'${cset}^{commit}'" >/dev/null 2>&1; then
     printf "Commit '%s' does not exist in this repository\n." "${cset}"
     exit 1
-- 
2.14.1

[Buildroot] [PATCH 5/7 v4] download/git: ensure we checkout to a clean state

[Yann E. MORIN]

Force the checkout to ignore and throw away any local changes. This
allows recovering from a previous partial checkout (e.g. killed by
the user, or by a CI job...)

git checkout -f has been supported since the inception of git, so we
can use it without any second thought.

Also do a forced-forced clean, to really get rid of all untracked stuff.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
---
 support/download/git | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/support/download/git b/support/download/git
index 921a35998b..ca8c94fa3b 100755
--- a/support/download/git
+++ b/support/download/git
@@ -154,7 +154,11 @@ fi
 
 # Checkout the required changeset, so that we can update the required
 # submodules.
-_git checkout -q "'${cset}'"
+_git checkout -f -q "'${cset}'"
+
+# Get rid of now-untracked directories (in case a git operation was
+# interrupted in a previous run).
+_git clean -ffdx
 
 # Get date of commit to generate a reproducible archive.
 # %cD is RFC2822, so it's fully qualified, with TZ and all.
-- 
2.14.1

[Buildroot] [PATCH 6/7 v4] download/git: ensure we can checkout repos with submodule conversions

[Yann E. MORIN]

When a git tree has had sub-dir <-> sub-module conversions, or has had
submodules added or removed over the course of time, checking out a
changeset across those conversions/additions/removals may leave
untracked files, or may fail because of a conflict of type.

So, before we checkout the new changeset, we forcibly remove the
submodules. The new set of submodules, if any, will be restored later.

Ideally, we would use a native git command: git submodule deinit --all.
However, that was only introduced in git 1.8.3 which, while not being
recent by modern standards, is still too old for some enterprise-grade
distributions (RHEL6 only has git-1.7.1).

So, instead, we just use git submodule foreach, to rm -rf the submodules
directory.

Again, we would ideally use 'cd $toplevel && rm -rf $path', but
$toplevel was only introduced in git 1.7.2. $path has always been there.

So, instead, we just cd back one level, and remove the basename of the
directory.

Eventually, we need to get rid of now-empty and untracked directories,
that were parents of a removed submodule. For example. ./foo/bar/ was a
submodule, so ./foo/bar/ was removed, which left ./foo/ around.

Yet again, recent-ish git versions would have removed it during the
forced checkout, but old-ish versions (e.g. 1.7.1) do not remove it with
the forced checkout.

Instead we rely on the already used forced-forced clean of directories,
untracked, and ignored content, to really get rid of extra stuff we are
not interested in.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
---
 support/download/git | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/support/download/git b/support/download/git
index ca8c94fa3b..b98a5d6143 100755
--- a/support/download/git
+++ b/support/download/git
@@ -152,12 +152,39 @@ if ! _git rev-parse --quiet --verify "'${cset}^{commit}'" >/dev/null 2>&1; then
     exit 1
 fi
 
+# The new cset we want to checkout might have different submodules, or
+# have sub-dirs converted to/from a submodule. So we would need to
+# deregister _current_ submodules before we checkout.
+#
+# Using "git submodule deinit --all" would remove all the files for
+# all submodules, including the corresponding .git files or directories.
+# However, it  was only introduced with git-1.8.3, which is too recent
+# for some enterprise-grade distros.
+#
+# So, we fall-back to just removing all submodules directories. We do
+# not need to be recursive, as removing a submodule will de-facto remove
+# its own submodules.
+#
+# For recent git versions, the repository for submodules is stored
+# inside the repository of the super repository, so the following will
+# only remove the working copies of submodules, effectively caching the
+# submodules.
+#
+# For older versions however, the repository is stored in the .git/ of
+# the submodule directory, so the following will effectively remove the
+# the working copy as well as the repository, which means submodules
+# will not be cached for older versions.
+#
+cmd='printf "Deregistering submodule \"%s\"\n" "${path}" && cd .. && rm -rf "${path##*/}"'
+_git submodule --quiet foreach "'${cmd}'"
+
 # Checkout the required changeset, so that we can update the required
 # submodules.
 _git checkout -f -q "'${cset}'"
 
 # Get rid of now-untracked directories (in case a git operation was
-# interrupted in a previous run).
+# interrupted in a previous run, or to get rid of empty directories
+# that were parrents of submodules removed above).
 _git clean -ffdx
 
 # Get date of commit to generate a reproducible archive.
-- 
2.14.1

[Buildroot] [PATCH 7/7 v4] download/git: always do full-clone

[Yann E. MORIN]

We currently attempt a shallow clone, as tentative to save bandwidth and
download time.

However, now that we keep the git tree as a cache, it may happen that we
need to checkout an earlier commit, and that would not be present with a
shallow clone.

Furthermore, the shallow fetch is already really broken, and just
happens to work by chance. Consider the following actions, which are
basically what happens today:

    mkdir git
    git init git
    cd git
    git remote add origin https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
    git fetch origin --depth 1 v4.17-rc1
    if ! git fetch origin v4.17-rc1:v4.17-rc1 ; then
        echo "warning"
    fi
    git checkout v4.17-rc1

The checkout succeeds just because of the git-fetch in the if-condition,
which is initially there to fetch the special refs from github PRs, or
gerrit reviews. That fails, but we just print a warning. If we were to
ever remove support for special refs, then the checkout would fail.

The whole purpose of the git cache is to actually save bandwidth and
download time, but in the long run. For one-offs, people would
preferably use a wget download (e.g. with the github macro) instead of
a git clone.

We switch to always doing a full clone. It is more correct, and pays off
in the long run...

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
---
 support/download/git | 24 +++---------------------
 1 file changed, 3 insertions(+), 21 deletions(-)

diff --git a/support/download/git b/support/download/git
index b98a5d6143..a41dad339b 100755
--- a/support/download/git
+++ b/support/download/git
@@ -111,27 +111,9 @@ fi
 
 _git remote set-url origin "'${uri}'"
 
-# Try to fetch with limited depth, since it is faster than a full clone - but
-# that only works if the version is a ref (tag or branch). Before trying to do
-# a shallow clone we check if ${cset} is in the list provided by git ls-remote.
-# If not we fallback to a full fetch.
-#
-# Messages for the type of clone used are provided to ease debugging in
-# case of problems
-git_done=0
-if [ -n "$(_git ls-remote origin "'${cset}'" 2>&1)" ]; then
-    printf "Doing a shallow fetch\n"
-    if _git fetch "${@}" --depth 1 origin "'${cset}'"; then
-        git_done=1
-    else
-        printf "Shallow fetch failed, falling back to fetching all refs\n"
-    fi
-fi
-if [ ${git_done} -eq 0 ]; then
-    printf "Fetching all references\n"
-    _git fetch origin
-    _git fetch origin -t
-fi
+printf "Fetching all references\n"
+_git fetch origin
+_git fetch origin -t
 
 # Try to get the special refs exposed by some forges (pull-requests for
 # github, changes for gerrit...). There is no easy way to know whether
-- 
2.14.1

[Buildroot] [PATCH 1/7 v4] download/git: add warning not to use our git cache

[Ricardo Martincoski]

Hello,

On Sat, Apr 28, 2018 at 04:34 PM, Yann E. MORIN wrote:

> We really want the user not to use our git cache manually, or their
> changes (committed or not) may eventually get lost.
> 
> So, add a warning file, not unlike the one we put in the target/
> directory, to warn the user not to use the git tree.
> 
> Ideally, we would have carried this file in support/misc/, but the git
> backend does not have acces to it: the working directory is somewhere

s/acces/access/

> unknown, and TOPDIR is not exported in the environment.
> 
> So, we have to carry it in-line in the backend instead.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

[The typo in the commit log can be fixed while applying.]
Reviewed-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
[with only this patch applied: the warning file is correctly created/updated on
 dl/<package>/]
Tested-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>


Regards,
Ricardo

[Buildroot] [PATCH 2/7 v4] download/git: run all git commands in the current directory

[Ricardo Martincoski]

Hello,

On Sat, Apr 28, 2018 at 04:34 PM, Yann E. MORIN wrote:

> That way, we can pushd earlier, which will help with last-ditch recovery
> in a followup commit.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Reviewed-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
[with patches until this one applied: works with empty, broken (by removing
 .git/HEAD) or pre-existing git cache using git versions 1.7.1, 1.8.3, 2.11.0,
 2.14.1]
Tested-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>

Regards,
Ricardo

[Buildroot] [PATCH 3/7 v4] download/git: quickly exit when the cset does not exist

[Ricardo Martincoski]

Hello,

On Sat, Apr 28, 2018 at 04:34 PM, Yann E. MORIN wrote:

> Check that the given cset is indeed something we can checkout. If not,
> then exit early.
> 
> This will be usefull when a later commit will trap any failing git
> command to try to recover the repository by doing a clone from scratch:
> when the cset is not a commit, it does not mean the repository is broken
> or what, and recloning from scratch would not help, so no need to trash
> a good cache.
> 
> Reported-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Reviewed-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
[with patches until this one applied: the script bails out when a wrong sha1 or
 wrong tag is used as version for a package, using git versions 1.7.1, 1.8.3,
 2.11.0, 2.14.1]
Tested-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>


Regards,
Ricardo

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

[Ricardo Martincoski]

Hello,

Using the previous iteration I tested adding '&& false' after the git checkout
command and the trap worked as expected.

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.

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

[Buildroot] [PATCH 5/7 v4] download/git: ensure we checkout to a clean state

[Ricardo Martincoski]

Hello,

On Sat, Apr 28, 2018 at 04:34 PM, Yann E. MORIN wrote:

> Force the checkout to ignore and throw away any local changes. This
> allows recovering from a previous partial checkout (e.g. killed by
> the user, or by a CI job...)
> 
> git checkout -f has been supported since the inception of git, so we
> can use it without any second thought.
> 
> Also do a forced-forced clean, to really get rid of all untracked stuff.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Acked-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
[with patches until this one applied: the checkout succeeds and the hash matches
 even if staged changes, not staged changes, untracked files or untracked
 directories containing another git repo are manually added to the cache, tested
 using git versions 1.7.1, 1.8.3, 2.11.0, 2.14.1.
 The only exception is that the hash mismatches when using git 1.7.1 with a
 package with submodules, but it always failed, even with previous versions of
 Buildroot, because ancient git versions do not create .git files for
 submodules, and use inplace .git directories instead. No one complained so far
 for this case, so we shouldn't try to fix this specific scenario that no one
 uses, IMO.]
Tested-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>


Regards,
Ricardo

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

[Yann E. MORIN]

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

[Buildroot] [PATCH 6/7 v4] download/git: ensure we can checkout repos with submodule conversions

[Ricardo Martincoski]

Hello,

On Sat, Apr 28, 2018 at 04:34 PM, Yann E. MORIN wrote:

> When a git tree has had sub-dir <-> sub-module conversions, or has had
> submodules added or removed over the course of time, checking out a
> changeset across those conversions/additions/removals may leave
> untracked files, or may fail because of a conflict of type.
> 
> So, before we checkout the new changeset, we forcibly remove the
> submodules. The new set of submodules, if any, will be restored later.
> 
> Ideally, we would use a native git command: git submodule deinit --all.
> However, that was only introduced in git 1.8.3 which, while not being
> recent by modern standards, is still too old for some enterprise-grade
> distributions (RHEL6 only has git-1.7.1).
> 
> So, instead, we just use git submodule foreach, to rm -rf the submodules
> directory.
> 
> Again, we would ideally use 'cd $toplevel && rm -rf $path', but
> $toplevel was only introduced in git 1.7.2. $path has always been there.
> 
> So, instead, we just cd back one level, and remove the basename of the
> directory.
> 
> Eventually, we need to get rid of now-empty and untracked directories,
> that were parents of a removed submodule. For example. ./foo/bar/ was a
> submodule, so ./foo/bar/ was removed, which left ./foo/ around.
> 
> Yet again, recent-ish git versions would have removed it during the
> forced checkout, but old-ish versions (e.g. 1.7.1) do not remove it with
> the forced checkout.
> 
> Instead we rely on the already used forced-forced clean of directories,
> untracked, and ignored content, to really get rid of extra stuff we are
> not interested in.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

[I do not use submodules in a daily basis, but the changes seem OK.
 With the typo in a comment in the code fixed, please add]
 Reviewed-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>

[with patches until this one applied: I confirmed [0] using git 2.14.1 and a
 fake remote alternating between submodule <-> subdir that this patch is needed.
 I did *not* tested using recursive submodules by the lack of time to do.
 As a sanity check to see that all git commands used are supported by various
 git versions, I also downloaded the current version of sunxi-mali with an empty
 git cache, using git versions 1.7.1, 1.8.3, 2.11.0, 2.14.1. All git commands
 succeed, only the hash for git 1.7.1 mismatched as expected and described in
 the review of the previous patch]
Tested-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>


[snip]
>  # Get rid of now-untracked directories (in case a git operation was
> -# interrupted in a previous run).
> +# interrupted in a previous run, or to get rid of empty directories
> +# that were parrents of submodules removed above).

s/parrents/parents/

>  _git clean -ffdx
>  
>  # Get date of commit to generate a reproducible archive.
> -- 
> 2.14.1

[0] Test performed:
To test this patch I first created a local clone for sunxi-mali, then I
converted the submodule to a subdir using [1] and I exported the local repo
using 'git daemon'.
Then I used Buildroot with an empty git cache, without this patch (but with
previous patches applied), removing sunxi-mali.hash and changing sunxi-mali.mk
to point to my local repo at the commit I created, to generate the tarball for
this new commit. I calculated the sha256sum, recovered the original .hash file
and added the new hash to it.
With the same Buildroot tree and again with an empty cache, I downloaded the new
version and the hash matched.
I changed the sunxi-mali version back to its original value (the site version
still pointing to my local repo), I downloaded the "old" version without
emptying the cache and the hash matched.
Finally I changed the sunxi-mali version once again to the commit I created,
downloaded the "new" version without emptying the cache, the log shows [2] and
the hash mismatched.

Then I applied this patch and did the same sequence.
The hashes match for both "old" and "new" package versions, independent of the
previously checked out cset in the git cache.

[1] commands to convert submodule to subdir:
git rm lib/mali/
mkdir lib/mali
cd lib/mali
git init
git remote add origin https://github.com/linux-sunxi/sunxi-mali-proprietary.git
git fetch
git checkout 1c5063f43cdc9de341c0d63b2e3921cab86c7742
rm -rf .git/
cd -
git add lib/mali/
git commit

[2] warning: unable to rmdir lib/mali: Directory not empty


Regards,
Ricardo