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>

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

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 @@ static int tree_content_remove(struct tree_entry *root, const char *p)
 	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;
 }

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