[PATCH] pull: gracefully recover from delta retrieval failure.

This addresses a concern raised by Jason McMullan in the mailing
list discussion.  After retrieving and storing a potentially
deltified object, pull logic tries to check and fulfil its delta
dependency.  When the pull procedure is killed at this point,
however, there was no easy way to recover by re-running pull,
since next run would have found that we already have that
deltified object and happily reported success, without really
checking its delta dependency is satisfied.

This patch introduces --recover option to git-*-pull family
which causes them to re-validate dependency of deltified objects
we are fetching.  A new test t5100-delta-pull.sh covers such a
failure mode.

Signed-off-by: Junio C Hamano <junkio@cox.net>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

diff --git a/Documentation/git-http-pull.txt b/Documentation/git-http-pull.txt
index 8b54f09c223f0829ebf85cc4c5708e678fb4542a..e4b7b3789eec56444b782e8ba7a46bf87e7749e1 100644 (file)
--- a/Documentation/git-http-pull.txt
+++ b/Documentation/git-http-pull.txt
@@ -9,7 +9,7 @@ git-http-pull - Downloads a remote GIT repository via HTTP
 
 SYNOPSIS
 --------
-'git-http-pull' [-c] [-t] [-a] [-v] [-d] commit-id url
+'git-http-pull' [-c] [-t] [-a] [-v] [-d] [--recover] commit-id url
 
 DESCRIPTION
 -----------
@@ -25,6 +25,9 @@ Downloads a remote GIT repository via HTTP.
        Do not check for delta base objects (use this option
        only when you know the remote repository is not
        deltified).
+--recover::
+       Check dependency of deltified object more carefully than
+       usual, to recover after earlier pull that was interrupted.
 -v::
        Report what is downloaded.
 

diff --git a/Documentation/git-local-pull.txt b/Documentation/git-local-pull.txt
index 79d0f5c28f68f2634bc41de80b0c22a296aa6590..4bc66e77ba0c4e9f9595c3d0e9b3794dc946637a 100644 (file)
--- a/Documentation/git-local-pull.txt
+++ b/Documentation/git-local-pull.txt
@@ -9,7 +9,7 @@ git-local-pull - Duplicates another GIT repository on a local system
 
 SYNOPSIS
 --------
-'git-local-pull' [-c] [-t] [-a] [-l] [-s] [-n] [-v] [-d] commit-id path
+'git-local-pull' [-c] [-t] [-a] [-l] [-s] [-n] [-v] [-d] [--recover] commit-id path
 
 DESCRIPTION
 -----------
@@ -27,6 +27,9 @@ OPTIONS
        Do not check for delta base objects (use this option
        only when you know the remote repository is not
        deltified).
+--recover::
+       Check dependency of deltified object more carefully than
+       usual, to recover after earlier pull that was interrupted.
 -v::
        Report what is downloaded.
 

diff --git a/Documentation/git-rpull.txt b/Documentation/git-rpull.txt
index 3a8386629b7d05a2d6493815e232a8552a962585..a6c40ae97ca2414760f7ff9e15ed5a7888b36e74 100644 (file)
--- a/Documentation/git-rpull.txt
+++ b/Documentation/git-rpull.txt
@@ -10,7 +10,7 @@ git-rpull - Pulls from a remote repository over ssh connection
 
 SYNOPSIS
 --------
-'git-rpull' [-c] [-t] [-a] [-d] [-v] commit-id url
+'git-rpull' [-c] [-t] [-a] [-d] [-v] [--recover] commit-id url
 
 DESCRIPTION
 -----------
@@ -29,6 +29,9 @@ OPTIONS
        Do not check for delta base objects (use this option
        only when you know the remote repository is not
        deltified).
+--recover::
+       Check dependency of deltified object more carefully than
+       usual, to recover after earlier pull that was interrupted.
 -v::
        Report what is downloaded.
 

diff --git a/http-pull.c b/http-pull.c
index 551663e49234dc9b719ee4abb9f8dc8609d759aa..ec7f66af8f368b53e9eb0044ac9a8e43cea9a1f5 100644 (file)
--- a/http-pull.c
+++ b/http-pull.c
@@ -105,6 +105,8 @@ int main(int argc, char **argv)
                        get_history = 1;
                } else if (argv[arg][1] == 'd') {
                        get_delta = 0;
+               } else if (!strcmp(argv[arg], "--recover")) {
+                       get_delta = 2;
                } else if (argv[arg][1] == 'a') {
                        get_all = 1;
                        get_tree = 1;
@@ -115,7 +117,7 @@ int main(int argc, char **argv)
                arg++;
        }
        if (argc < arg + 2) {
-               usage("git-http-pull [-c] [-t] [-a] [-d] [-v] commit-id url");
+               usage("git-http-pull [-c] [-t] [-a] [-d] [-v] [--recover] commit-id url");
                return 1;
        }
        commit_id = argv[arg];

diff --git a/local-pull.c b/local-pull.c
index e5d834ff2f7d6949ca2c7dd2424c65f6431a839b..afdba9f527e2e78fcd62667cd17323e28a8ecb94 100644 (file)
--- a/local-pull.c
+++ b/local-pull.c
@@ -74,7 +74,7 @@ int fetch(unsigned char *sha1)
 }
 
 static const char *local_pull_usage = 
-"git-local-pull [-c] [-t] [-a] [-l] [-s] [-n] [-v] [-d] commit-id path";
+"git-local-pull [-c] [-t] [-a] [-l] [-s] [-n] [-v] [-d] [--recover] commit-id path";
 
 /* 
  * By default we only use file copy.
@@ -94,6 +94,8 @@ int main(int argc, char **argv)
                        get_history = 1;
                else if (argv[arg][1] == 'd')
                        get_delta = 0;
+               else if (!strcmp(argv[arg], "--recover"))
+                       get_delta = 2;
                else if (argv[arg][1] == 'a') {
                        get_all = 1;
                        get_tree = 1;

diff --git a/pull.c b/pull.c
index cd77738ac62be17e7382bc3b368e686f11f7098d..f4f1d8fcd38aa70a971c82e08fc08942f76f19bc 100644 (file)
--- a/pull.c
+++ b/pull.c
@@ -6,6 +6,7 @@
 
 int get_tree = 0;
 int get_history = 0;
+/* 1 means "get delta", 2 means "really check delta harder */
 int get_delta = 1;
 int get_all = 0;
 int get_verbosely = 0;
@@ -32,12 +33,16 @@ static void report_missing(const char *what, const unsigned char *missing)
 
 static int make_sure_we_have_it(const char *what, unsigned char *sha1)
 {
-       int status;
-       if (has_sha1_file(sha1))
+       int status = 0;
+
+       if (!has_sha1_file(sha1)) {
+               status = fetch(sha1);
+               if (status && what)
+                       report_missing(what, sha1);
+       }
+       else if (get_delta < 2)
                return 0;
-       status = fetch(sha1);
-       if (status && what)
-               report_missing(what, sha1);
+
        if (get_delta) {
                char delta_sha1[20];
                status = sha1_delta_base(sha1, delta_sha1);

diff --git a/pull.h b/pull.h
index 3cd14cfb811a755a8770a0d01e8e2f96ba604058..30086fdb12496a9da78aaf3969ca32fc5af98c5a 100644 (file)
--- a/pull.h
+++ b/pull.h
@@ -13,7 +13,9 @@ extern int get_history;
 /** Set to fetch the trees in the commit history. **/
 extern int get_all;
 
-/* Set to zero to skip the check for delta object base. */
+/* Set to zero to skip the check for delta object base;
+ * set to two to check delta dependency even for objects we already have.
+ */
 extern int get_delta;
 
 /* Set to be verbose */

diff --git a/rpull.c b/rpull.c
index f4ab89836455a40aaab3ff4114396185f6d5655a..a332fba23b0eb535ffacbdd68da80e1eebb62824 100644 (file)
--- a/rpull.c
+++ b/rpull.c
@@ -52,6 +52,8 @@ int main(int argc, char **argv)
                        get_history = 1;
                } else if (argv[arg][1] == 'd') {
                        get_delta = 0;
+               } else if (!strcmp(argv[arg], "--recover")) {
+                       get_delta = 2;
                } else if (argv[arg][1] == 'a') {
                        get_all = 1;
                        get_tree = 1;
@@ -62,7 +64,7 @@ int main(int argc, char **argv)
                arg++;
        }
        if (argc < arg + 2) {
-               usage("git-rpull [-c] [-t] [-a] [-v] [-d] commit-id url");
+               usage("git-rpull [-c] [-t] [-a] [-v] [-d] [--recover] commit-id url");
                return 1;
        }
        commit_id = argv[arg];

diff --git a/t/t5100-delta-pull.sh b/t/t5100-delta-pull.sh
new file mode 100644 (file)
index 0000000..8693c5c
--- /dev/null
+++ b/t/t5100-delta-pull.sh
@@ -0,0 +1,79 @@
+#!/bin/sh
+#
+# Copyright (c) 2005 Junio C Hamano
+#
+
+test_description='Test pulling deltified objects
+
+'
+. ./test-lib.sh
+
+locate_obj='s|\(..\)|.git/objects/\1/|'
+
+test_expect_success \
+    setup \
+    'cat ../README >a &&
+    git-update-cache --add a &&
+    a0=`git-ls-files --stage |
+        sed -e '\''s/^[0-7]* \([0-9a-f]*\) .*/\1/'\''` &&
+
+    sed -e 's/test/TEST/g' ../README >a &&
+    git-update-cache a &&
+    a1=`git-ls-files --stage |
+        sed -e '\''s/^[0-7]* \([0-9a-f]*\) .*/\1/'\''` &&
+    tree=`git-write-tree` &&
+    commit=`git-commit-tree $tree </dev/null` &&
+    a0f=`echo "$a0" | sed -e "$locate_obj"` &&
+    a1f=`echo "$a1" | sed -e "$locate_obj"` &&
+    echo commit $commit &&
+    echo a0 $a0 &&
+    echo a1 $a1 &&
+    ls -l $a0f $a1f &&
+    echo $commit >.git/HEAD &&
+    git-mkdelta -v $a0 $a1 &&
+    ls -l $a0f $a1f'
+
+# Now commit has a tree that records delitified "a" whose SHA1 is a1.
+# Create a new repo and pull this commit into it.
+
+test_expect_success \
+    'setup and cd into new repo' \
+    'mkdir dest && cd dest && rm -fr .git && git-init-db'
+     
+test_expect_success \
+    'pull from deltified repo into a new repo without -d' \
+    'rm -fr .git a && git-init-db &&
+     git-local-pull -v -a $commit ../.git/ &&
+     git-cat-file blob $a1 >a &&
+     diff -u a ../a'
+
+test_expect_failure \
+    'pull from deltified repo into a new repo with -d' \
+    'rm -fr .git a && git-init-db &&
+     git-local-pull -v -a -d $commit ../.git/ &&
+     git-cat-file blob $a1 >a &&
+     diff -u a ../a'
+
+test_expect_failure \
+    'pull from deltified repo after delta failure without --recover' \
+    'rm -f a &&
+     git-local-pull -v -a $commit ../.git/ &&
+     git-cat-file blob $a1 >a &&
+     diff -u a ../a'
+
+test_expect_success \
+    'pull from deltified repo after delta failure with --recover' \
+    'rm -f a &&
+     git-local-pull -v -a --recover $commit ../.git/ &&
+     git-cat-file blob $a1 >a &&
+     diff -u a ../a'
+
+test_expect_success \
+    'missing-tree or missing-blob should be re-fetched without --recover' \
+    'rm -f a $a0f $a1f &&
+     git-local-pull -v -a $commit ../.git/ &&
+     git-cat-file blob $a1 >a &&
+     diff -u a ../a'
+
+test_done
+