Re: [PATCH] refs: handle null-oid for pseudorefs

According to the documentation on `git update-ref`, it is possible to
"specify 40 '0' or an empty string as <oldvalue> to make sure that
the
ref you are creating does not exist." But in the code for pseudorefs,
we
do not implement this. If we fail to read the old ref, we immediately
die. A failure to read would actually be a good thing if we have been
given the null-oid.

With the null-oid, allow -- and even require -- the ref-reading to
fail.
This implements the "make sure that the ref ... does not exist" part
of
the documentation.

Since we have a `strbuf err` for collecting errors, let's use it and
signal an error to the caller instead of dying hard.

Reported-by: Rafael Ascensão <redacted>
Helped-by: Rafael Ascensão [off-list ref]
Signed-off-by: Martin Ågren <redacted>
---
(David's twopensource-address bounced, so I'm trying instead the one
he
most recently posted from.)

 t/t1400-update-ref.sh |  7 +++++++
 refs.c                | 19 +++++++++++++++----
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 664a3a4e4e..bd41f86f22 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh

@@ -457,6 +457,13 @@ 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_expect_success 'create pseudoref with old oid null, but do not
overwrite' '
+	git update-ref PSEUDOREF $A $Z &&
+	test_when_finished "git update-ref -d PSEUDOREF" &&
+	test $A = $(cat .git/PSEUDOREF) &&
+	test_must_fail git update-ref PSEUDOREF $A $Z
+'
+
 a=refs/heads/a
 b=refs/heads/b
 c=refs/heads/c

diff --git a/refs.c b/refs.c
index 8b7a77fe5e..3669190499 100644
--- a/refs.c
+++ b/refs.c

@@ -666,10 +666,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 sha1 when
writing '%s'",
+				    pseudoref);
 			rollback_lock_file(&lock);
 			goto done;
 		}