git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Martin Ågren" <martin.agren@gmail.com>
To: "Rafael Ascensão" <rafa.almas@gmail.com>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
	David Turner <dturner@twopensource.com>,
	Jonathan Nieder <jrnieder@gmail.com>,
	Michael Haggerty <mhagger@alum.mit.edu>,
	"brian m . carlson" <sandals@crustytoothpaste.net>
Subject: Re: git update-ref fails to create reference. (bug)
Date: Fri,  4 May 2018 20:26:46 +0200	[thread overview]
Message-ID: <20180504182646.7738-1-martin.agren@gmail.com> (raw)
In-Reply-To: <cced9e2c-7d02-47fd-109a-1185eed13e63@gmail.com>

Hi Rafael,

On 4 May 2018 at 18:28, Rafael Ascensão <rafa.almas@gmail.com> wrote:
> While trying to create a pseudo reference named REF pointing to the
> empty tree iff it doesn't exist, I stumbled on the following:
>
> I assume both are valid ways to create such reference:
>         a) $ echo -e option no-deref\\nupdate REF $(git hash-object -t
>         tree /dev/null) 0000000000000000000000000000000000000000 | git
>         update-ref --stdin
>         b) $ git update-ref --no-deref REF $(git hash-object -t tree
>         /dev/null) 0000000000000000000000000000000000000000
>
> While a) works, b) will throw:
>         fatal: could not read ref 'REF'


I can reproduce this and I agree with your understanding of what should
happen here. The patch below makes this work according to my and your
expectations, at least in my command-line testing.

The die("... already exists") could instead be a no-op, trusting that
the backend discovers the problem. "die" could also be strbuf_addf(...),
I'm just following 2c3aed138 here.

Anyway, that's not where I'm stuck... Regardless of how I try to write
tests (in t1400), they just pass beautifully even before this patch. I
might be able to look into that more on the weekend. If anyone has
ideas, I am all ears. Or if someone feels like picking this up and
running with it, feel free.

Martin

diff --git a/refs.c b/refs.c
index 8b7a77fe5e..cdb0a5ab29 100644
--- a/refs.c
+++ b/refs.c
@@ -666,9 +666,12 @@ 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))
+				die("could not read ref '%s'", pseudoref);
+		} else if (is_null_oid(old_oid)) {
+			die("reference '%s' already exists", pseudoref);
+		} else if (oidcmp(&actual_old_oid, old_oid)) {
 			strbuf_addf(err, "unexpected sha1 when writing '%s'", pseudoref);
 			rollback_lock_file(&lock);
 			goto done;
-- 
2.17.0.392.g7fa371e468


  reply	other threads:[~2018-05-04 18:27 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 [this message]
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             ` [PATCH v2 3/3] refs: handle zero oid for pseudorefs Martin Ågren

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=20180504182646.7738-1-martin.agren@gmail.com \
    --to=martin.agren@gmail.com \
    --cc=dturner@twopensource.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=mhagger@alum.mit.edu \
    --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).