git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Martin Ågren" <martin.agren@gmail.com>
To: git@vger.kernel.org
Cc: "Michael Haggerty" <mhagger@alum.mit.edu>,
	"Rafael Ascensão" <rafa.almas@gmail.com>,
	"Junio C Hamano" <gitster@pobox.com>,
	"David Turner" <novalis@novalis.org>,
	"Jonathan Nieder" <jrnieder@gmail.com>,
	"brian m. carlson" <sandals@crustytoothpaste.net>
Subject: [PATCH v2 3/3] refs: handle zero oid for pseudorefs
Date: Thu, 10 May 2018 21:29:56 +0200	[thread overview]
Message-ID: <1b6b1a25acf42747df479b90b1cc28e4c31eabe0.1525979881.git.martin.agren@gmail.com> (raw)
In-Reply-To: <cover.1525979881.git.martin.agren@gmail.com>

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>
---
 t/t1400-update-ref.sh |  4 ++--
 refs.c                | 16 +++++++++++++---
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 3996109ba4..faf0dfe993 100755
--- 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) &&
diff --git a/refs.c b/refs.c
index 7820a52c4f..26af07fc51 100644
--- 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);
-- 
2.17.0


      parent reply	other threads:[~2018-05-10 19:30 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-04 16:28 git update-ref fails to create reference. (bug) Rafael Ascensão
2018-05-04 18:26 ` Martin Ågren
2018-05-05 19:08   ` Rafael Ascensão
2018-05-06 13:35     ` [PATCH] refs: handle null-oid for pseudorefs Martin Ågren
2018-05-06 15:37       ` David Turner
2018-05-07  7:39       ` Michael Haggerty
2018-05-07 10:05         ` Martin Ågren
2018-05-10 19:29           ` [PATCH v2 0/3] refs: handle zero oid " Martin Ågren
2018-05-10 19:29             ` [PATCH v2 1/3] refs.c: refer to "object ID", not "sha1", in error messages Martin Ågren
2018-05-10 19:29             ` [PATCH v2 2/3] t1400: add tests around adding/deleting pseudorefs Martin Ågren
2018-05-10 19:29             ` Martin Ågren [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1b6b1a25acf42747df479b90b1cc28e4c31eabe0.1525979881.git.martin.agren@gmail.com \
    --to=martin.agren@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=mhagger@alum.mit.edu \
    --cc=novalis@novalis.org \
    --cc=rafa.almas@gmail.com \
    --cc=sandals@crustytoothpaste.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).