refs: handle zero oid for pseudorefs

According to the documentation, 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, as demonstrated by the failing tests added in the previous commit.
If we fail to read the old ref, we immediately die. But a failure to
read would actually be a good thing if we have been given the zero oid.

With the zero 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 and fixes both failing tests from the previous commit.

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 <rafa.almas@gmail.com>
Helped-by: Rafael Ascensão <rafa.almas@gmail.com>
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/refs.c b/refs.c
index 7820a52c4f4362829a95f73e5899451c486b2c35..26af07fc51791e511200c5011e8655822be18337 100644 (file)
--- a/refs.c
+++ b/refs.c
@@ -681,9 +681,19 @@ 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)) {
+               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);

diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 3996109ba48b5ce81e71ff4d90cb05486477f31b..faf0dfe993d375c885762b2d15706cd8ebe6d577 100755 (executable)
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -503,12 +503,12 @@ test_expect_success 'delete pseudoref with correct old value' '
        test_path_is_missing .git/PSEUDOREF
 '
 
-test_expect_failure 'create pseudoref with old OID zero' '
+test_expect_success 'create pseudoref with old OID zero' '
        git update-ref PSEUDOREF $A $Z &&
        test $A = $(cat .git/PSEUDOREF)
 '
 
-test_expect_failure 'do not overwrite pseudoref with old OID zero' '
+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) &&