Don't do non-fastforward updates in fast-import.

If fast-import is being used to update an existing branch of
a repository, the user may not want to lose commits if another
process updates the same ref at the same time.  For example, the
user might be using fast-import to make just one or two commits
against a live branch.

We now perform a fast-forward check during the ref updating process.
If updating a branch would cause commits in that branch to be lost,
we skip over it and display the new SHA1 to standard error.

This new default behavior can be overridden with `--force`, like
git-push and git-fetch.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>

diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index 08450de9ac0e9ba7c116a9c1c92b806458073541..2be6c4b8070feaf6482e65f6517c1b801c16760d 100644 (file)
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -38,6 +38,11 @@ OPTIONS
        See ``Date Formats'' below for details about which formats
        are supported, and their syntax.
 
+--force::
+       Force updating modified existing branches, even if doing
+       so would cause commits to be lost (as the new commit does
+       not contain the old commit).
+
 --max-pack-size=<n>::
        Maximum size of each output packfile, expressed in MiB.
        The default is 4096 (4 GiB) as that is the maximum allowed
@@ -92,11 +97,18 @@ run alongside parallel `git repack -a -d` or `git gc` invocations,
 or any other Git operation (including `git prune`, as loose objects
 are never used by gfi).
 
-However, gfi does not lock the branch or tag refs it is actively
-importing.  After EOF, during its ref update phase, gfi blindly
-overwrites each imported branch or tag ref.  Consequently it is not
-safe to modify refs that are currently being used by a running gfi
-instance, as work could be lost when gfi overwrites the refs.
+gfi does not lock the branch or tag refs it is actively importing.
+After the import, during its ref update phase, gfi tests each
+existing branch ref to verify the update will be a fast-forward
+update (the commit stored in the ref is contained in the new
+history of the commit to be written).  If the update is not a
+fast-forward update, gfi will skip updating that ref and instead
+prints a warning message.  gfi will always attempt to update all
+branch refs, and does not stop on the first failure.
+
+Branch updates can be forced with `--force`, but its recommended that
+this only be used on an otherwise quiet repository.  Using `--force`
+is not necessary for an initial import into an empty repository.
 
 
 Technical Discussion

diff --git a/fast-import.c b/fast-import.c
index ee4777fdafcbedc24997155581e55c339b40981e..df84e4d87d3d7c151bd0cdda29b22519e7043c8a 100644 (file)
--- a/fast-import.c
+++ b/fast-import.c
@@ -121,6 +121,7 @@ Format of STDIN stream:
 #include "object.h"
 #include "blob.h"
 #include "tree.h"
+#include "commit.h"
 #include "delta.h"
 #include "pack.h"
 #include "refs.h"
@@ -247,6 +248,7 @@ typedef enum {
 /* Configured limits on output */
 static unsigned long max_depth = 10;
 static unsigned long max_packsize = (1LL << 32) - 1;
+static int force_update;
 
 /* Stats and misc. counters */
 static uintmax_t alloc_count;
@@ -257,6 +259,7 @@ static uintmax_t delta_count_by_type[1 << TYPE_BITS];
 static unsigned long object_count;
 static unsigned long branch_count;
 static unsigned long branch_load_count;
+static int failure;
 
 /* Memory pools */
 static size_t mem_pool_alloc = 2*1024*1024 - sizeof(struct mem_pool);
@@ -1278,19 +1281,48 @@ del_entry:
        return 1;
 }
 
-static void dump_branches(void)
+static int update_branch(struct branch *b)
 {
        static const char *msg = "fast-import";
+       struct ref_lock *lock;
+       unsigned char old_sha1[20];
+
+       if (read_ref(b->name, old_sha1))
+               hashclr(old_sha1);
+       lock = lock_any_ref_for_update(b->name, old_sha1);
+       if (!lock)
+               return error("Unable to lock %s", b->name);
+       if (!force_update && !is_null_sha1(old_sha1)) {
+               struct commit *old_cmit, *new_cmit;
+
+               old_cmit = lookup_commit_reference_gently(old_sha1, 0);
+               new_cmit = lookup_commit_reference_gently(b->sha1, 0);
+               if (!old_cmit || !new_cmit) {
+                       unlock_ref(lock);
+                       return error("Branch %s is missing commits.", b->name);
+               }
+
+               if (!in_merge_bases(old_cmit, new_cmit)) {
+                       unlock_ref(lock);
+                       warn("Not updating %s"
+                               " (new tip %s does not contain %s)",
+                               b->name, sha1_to_hex(b->sha1), sha1_to_hex(old_sha1));
+                       return -1;
+               }
+       }
+       if (write_ref_sha1(lock, b->sha1, msg) < 0)
+               return error("Unable to update %s", b->name);
+       return 0;
+}
+
+static void dump_branches(void)
+{
        unsigned int i;
        struct branch *b;
-       struct ref_lock *lock;
 
        for (i = 0; i < branch_table_sz; i++) {
-               for (b = branch_table[i]; b; b = b->table_next_branch) {
-                       lock = lock_any_ref_for_update(b->name, NULL);
-                       if (!lock || write_ref_sha1(lock, b->sha1, msg) < 0)
-                               die("Can't write %s", b->name);
-               }
+               for (b = branch_table[i]; b; b = b->table_next_branch)
+                       failure |= update_branch(b);
        }
 }
 
@@ -1299,13 +1331,13 @@ static void dump_tags(void)
        static const char *msg = "fast-import";
        struct tag *t;
        struct ref_lock *lock;
-       char path[PATH_MAX];
+       char ref_name[PATH_MAX];
 
        for (t = first_tag; t; t = t->next_tag) {
-               sprintf(path, "refs/tags/%s", t->name);
-               lock = lock_any_ref_for_update(path, NULL);
+               sprintf(ref_name, "tags/%s", t->name);
+               lock = lock_ref_sha1(ref_name, NULL);
                if (!lock || write_ref_sha1(lock, t->sha1, msg) < 0)
-                       die("Can't write %s", path);
+                       failure |= error("Unable to update %s", ref_name);
        }
 }
 
@@ -1936,6 +1968,8 @@ int main(int argc, const char **argv)
                        max_active_branches = strtoul(a + 18, NULL, 0);
                else if (!strncmp(a, "--export-marks=", 15))
                        mark_file = a + 15;
+               else if (!strcmp(a, "--force"))
+                       force_update = 1;
                else
                        die("unknown option %s", a);
        }
@@ -2001,5 +2035,5 @@ int main(int argc, const char **argv)
        fprintf(stderr, "---------------------------------------------------------------------\n");
        fprintf(stderr, "\n");
 
-       return 0;
+       return failure ? 1 : 0;
 }

diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 84b3c12a50c5fb3ea2e98ff5b4a7a441b203dbb6..23a2ba78f68ad32ea4bae2e6bb20734f38fc71c2 100755 (executable)
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -276,4 +276,84 @@ test_expect_success \
        'git-cat-file commit branch | sed 1,2d >actual &&
        diff -u expect actual'
 
+###
+### series F
+###
+
+old_branch=`git-rev-parse --verify branch^0`
+test_tick
+cat >input <<INPUT_END
+commit refs/heads/branch
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+losing things already?
+COMMIT
+
+from refs/heads/branch~1
+
+reset refs/heads/other
+from refs/heads/branch
+
+INPUT_END
+test_expect_success \
+    'F: non-fast-forward update skips' \
+    'if git-fast-import <input
+        then
+               echo BAD gfi did not fail
+               return 1
+        else
+               if test $old_branch = `git-rev-parse --verify branch^0`
+               then
+                       : branch unaffected and failure returned
+                       return 0
+               else
+                       echo BAD gfi changed branch $old_branch
+                       return 1
+               fi
+        fi
+       '
+test_expect_success \
+       'F: verify pack' \
+       'for p in .git/objects/pack/*.pack;do git-verify-pack $p||exit;done'
+
+cat >expect <<EOF
+tree `git-rev-parse branch~1^{tree}`
+parent `git-rev-parse branch~1`
+author $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+
+losing things already?
+EOF
+test_expect_success \
+       'F: verify other commit' \
+       'git-cat-file commit other >actual &&
+       diff -u expect actual'
+
+###
+### series G
+###
+
+old_branch=`git-rev-parse --verify branch^0`
+test_tick
+cat >input <<INPUT_END
+commit refs/heads/branch
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+losing things already?
+COMMIT
+
+from refs/heads/branch~1
+
+INPUT_END
+test_expect_success \
+    'G: non-fast-forward update forced' \
+    'git-fast-import --force <input'
+test_expect_success \
+       'G: verify pack' \
+       'for p in .git/objects/pack/*.pack;do git-verify-pack $p||exit;done'
+test_expect_success \
+       'G: branch changed, but logged' \
+       'test $old_branch != `git-rev-parse --verify branch^0` &&
+        test $old_branch = `git-rev-parse --verify branch@{1}`'
+
 test_done