diff mbox series

[2/4] download/git: ensure we have a sane repository

Message ID 8019bec4b6420d014d263c2c3e12e3c985a2ef0d.1523983687.git.yann.morin.1998@free.fr
State Accepted
Headers show
Series None | expand

Commit Message

Yann E. MORIN April 17, 2018, 4:48 p.m. UTC
There are cases where a repository might be broken, e.g. when a previous
operation was killed or otherwise failed unexpectedly.

We fix that by always initialising the repository, as suggested by
Ricardo. git-init is safe on an otherwise-healthy repository:

    Running git init in an existing repository is safe. It will not
    overwrite things that are already there. [...]

Using git-init will just ensure that we have the strictly required files
to form a sane tree. Any blob that is still missing would get fetched
later on.

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

Comments

Ricardo Martincoski April 19, 2018, 3:50 p.m. UTC | #1
Hello,

On Tue, Apr 17, 2018 at 01:48 PM, Yann E. MORIN wrote:

> There are cases where a repository might be broken, e.g. when a previous
> operation was killed or otherwise failed unexpectedly.
> 
> We fix that by always initialising the repository, as suggested by
> Ricardo. git-init is safe on an otherwise-healthy repository:
> 
>     Running git init in an existing repository is safe. It will not
>     overwrite things that are already there. [...]
> 
> Using git-init will just ensure that we have the strictly required files
> to form a sane tree. Any blob that is still missing would get fetched
> later on.
> 
> Reported-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Reported-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Cc: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

Acked-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
[a broken repo with a clean worktree is recovered at the extent that git allows]
Tested-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>


Tests performed with patches 1 and 2 applied:

1) In the same scenario tested for patch 1 (empty dl/<package>/git) instead of
   bailing out the script reinitialises and uses the git cache.

2) Using git 2.11.0, download all git packages in the tree, remove the tarball
   and regenerate it 2 times, before [1] and after [2] this patch.
[1] https://gitlab.com/RicardoMartincoski/buildroot/pipelines/20734086
[2] https://gitlab.com/RicardoMartincoski/buildroot/pipelines/20729761
In theses GitLab pipelines there are jobs marked as failures (remote server did
not respond, ...) but they are not related to patch 1 or 2. The same occur
before and after the 2 patches.

Regards,
Ricardo
Yann E. MORIN April 19, 2018, 7:45 p.m. UTC | #2
Ricardo, All,

On 2018-04-19 12:50 -0300, Ricardo Martincoski spake thusly:
> On Tue, Apr 17, 2018 at 01:48 PM, Yann E. MORIN wrote:
> > There are cases where a repository might be broken, e.g. when a previous
> > operation was killed or otherwise failed unexpectedly.
> > 
> > We fix that by always initialising the repository, as suggested by
> > Ricardo. git-init is safe on an otherwise-healthy repository:
> > 
> >     Running git init in an existing repository is safe. It will not
> >     overwrite things that are already there. [...]
> > 
> > Using git-init will just ensure that we have the strictly required files
> > to form a sane tree. Any blob that is still missing would get fetched
> > later on.
> > 
> > Reported-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> > Reported-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> > Cc: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
> > Cc: Arnout Vandecappelle <arnout@mind.be>
> > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> 
> Acked-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> [a broken repo with a clean worktree is recovered at the extent that git allows]
> Tested-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> 
> 
> Tests performed with patches 1 and 2 applied:
> 
> 1) In the same scenario tested for patch 1 (empty dl/<package>/git) instead of
>    bailing out the script reinitialises and uses the git cache.
> 
> 2) Using git 2.11.0, download all git packages in the tree, remove the tarball
>    and regenerate it 2 times, before [1] and after [2] this patch.
> [1] https://gitlab.com/RicardoMartincoski/buildroot/pipelines/20734086
> [2] https://gitlab.com/RicardoMartincoski/buildroot/pipelines/20729761
> In theses GitLab pipelines there are jobs marked as failures (remote server did
> not respond, ...) but they are not related to patch 1 or 2. The same occur
> before and after the 2 patches.

Thanks for the extensive testing! :-)

Regards,
Yann E. MORIN.
Thomas Petazzoni April 19, 2018, 8:38 p.m. UTC | #3
Hello,

On Tue, 17 Apr 2018 18:48:21 +0200, Yann E. MORIN wrote:
> There are cases where a repository might be broken, e.g. when a previous
> operation was killed or otherwise failed unexpectedly.
> 
> We fix that by always initialising the repository, as suggested by
> Ricardo. git-init is safe on an otherwise-healthy repository:
> 
>     Running git init in an existing repository is safe. It will not
>     overwrite things that are already there. [...]
> 
> Using git-init will just ensure that we have the strictly required files
> to form a sane tree. Any blob that is still missing would get fetched
> later on.
> 
> Reported-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Reported-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Cc: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
>  support/download/git | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)

Applied to master, thanks.

Thomas
diff mbox series

Patch

diff --git a/support/download/git b/support/download/git
index c166ae2813..1172310186 100755
--- a/support/download/git
+++ b/support/download/git
@@ -43,14 +43,16 @@  _git() {
     eval GIT_DIR="${git_cache}/.git" ${GIT} "${@}"
 }
 
-# If the cache directory doesn't exists, init a new repo, which will be
-# fetch'ed later.
-if [ ! -d "${git_cache}" ]; then
-    # 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}'"
-fi
+# 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
+# case, we might be missing blobs, but that's not a problem: we'll
+# fetch what we need later anyway.
+#
+# 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