Merge branch 'ma/create-pseudoref-with-null-old-oid'

"git update-ref A B" is supposed to ensure that ref A does not yet
exist when B is a NULL OID, but this check was not done correctly
for pseudo-refs outside refs/ hierarchy, e.g. MERGE_HEAD.

* ma/create-pseudoref-with-null-old-oid:
  refs: handle zero oid for pseudorefs
  t1400: add tests around adding/deleting pseudorefs
  refs.c: refer to "object ID", not "sha1", in error messages

2 files changed, 77 insertions, 5 deletions

diff --git a/refs.c b/refs.c
index dabcd850a9..20fb35d895 100644
--- a/refs.c
+++ b/refs.c
@@ -681,10 +681,21 @@ static int write_pseudoref(const char *pseudoref, const struct object_id *oid,
 	if (old_oid) {
 		struct object_id actual_old_oid;
 
-		if (read_ref(pseudoref, &actual_old_oid))
-			die("could not read ref '%s'", pseudoref);
-		if (oidcmp(&actual_old_oid, old_oid)) {
-			strbuf_addf(err, "unexpected sha1 when writing '%s'", pseudoref);
+		if (read_ref(pseudoref, &actual_old_oid)) {
+			if (!is_null_oid(old_oid)) {
+				strbuf_addf(err, "could not read ref '%s'",
+					    pseudoref);
+				rollback_lock_file(&lock);
+				goto done;
+			}
+		} else if (is_null_oid(old_oid)) {
+			strbuf_addf(err, "ref '%s' already exists",
+				    pseudoref);
+			rollback_lock_file(&lock);
+			goto done;
+		} else if (oidcmp(&actual_old_oid, old_oid)) {
+			strbuf_addf(err, "unexpected object ID when writing '%s'",
+				    pseudoref);
 			rollback_lock_file(&lock);
 			goto done;
 		}
@@ -725,7 +736,8 @@ static int delete_pseudoref(const char *pseudoref, const struct object_id *old_o
 		if (read_ref(pseudoref, &actual_old_oid))
 			die("could not read ref '%s'", pseudoref);
 		if (oidcmp(&actual_old_oid, old_oid)) {
-			warning("Unexpected sha1 when deleting %s", pseudoref);
+			error("unexpected object ID when deleting '%s'",
+			      pseudoref);
 			rollback_lock_file(&lock);
 			return -1;
 		}
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 664a3a4e4e..faf0dfe993 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -457,6 +457,66 @@ test_expect_success 'git cat-file blob master@{2005-05-26 23:42}:F (expect OTHER
 	test OTHER = $(git cat-file blob "master@{2005-05-26 23:42}:F")
 '
 
+# Test adding and deleting pseudorefs
+
+test_expect_success 'given old value for missing pseudoref, do not create' '
+	test_must_fail git update-ref PSEUDOREF $A $B 2>err &&
+	test_path_is_missing .git/PSEUDOREF &&
+	grep "could not read ref" err
+'
+
+test_expect_success 'create pseudoref' '
+	git update-ref PSEUDOREF $A &&
+	test $A = $(cat .git/PSEUDOREF)
+'
+
+test_expect_success 'overwrite pseudoref with no old value given' '
+	git update-ref PSEUDOREF $B &&
+	test $B = $(cat .git/PSEUDOREF)
+'
+
+test_expect_success 'overwrite pseudoref with correct old value' '
+	git update-ref PSEUDOREF $C $B &&
+	test $C = $(cat .git/PSEUDOREF)
+'
+
+test_expect_success 'do not overwrite pseudoref with wrong old value' '
+	test_must_fail git update-ref PSEUDOREF $D $E 2>err &&
+	test $C = $(cat .git/PSEUDOREF) &&
+	grep "unexpected object ID" err
+'
+
+test_expect_success 'delete pseudoref' '
+	git update-ref -d PSEUDOREF &&
+	test_path_is_missing .git/PSEUDOREF
+'
+
+test_expect_success 'do not delete pseudoref with wrong old value' '
+	git update-ref PSEUDOREF $A &&
+	test_must_fail git update-ref -d PSEUDOREF $B 2>err &&
+	test $A = $(cat .git/PSEUDOREF) &&
+	grep "unexpected object ID" err
+'
+
+test_expect_success 'delete pseudoref with correct old value' '
+	git update-ref -d PSEUDOREF $A &&
+	test_path_is_missing .git/PSEUDOREF
+'
+
+test_expect_success 'create pseudoref with old OID zero' '
+	git update-ref PSEUDOREF $A $Z &&
+	test $A = $(cat .git/PSEUDOREF)
+'
+
+test_expect_success 'do not overwrite pseudoref with old OID zero' '
+	test_when_finished git update-ref -d PSEUDOREF &&
+	test_must_fail git update-ref PSEUDOREF $B $Z 2>err &&
+	test $A = $(cat .git/PSEUDOREF) &&
+	grep "already exists" err
+'
+
+# Test --stdin
+
 a=refs/heads/a
 b=refs/heads/b
 c=refs/heads/c