git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: "Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Han-Wen Nienhuys <hanwenn@gmail.com>,
	Han-Wen Nienhuys <hanwen@google.com>
Subject: [PATCH 1/2] Modify pseudo refs through ref backend storage
Date: Mon, 06 Jul 2020 17:29:31 +0000	[thread overview]
Message-ID: <6821f57bdf326f161f152a8af0e47b54513c77b1.1594056572.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.673.git.1594056572.gitgitgadget@gmail.com>

From: Han-Wen Nienhuys <hanwen@google.com>

The previous behavior was introduced in commit 74ec19d4be
("pseudorefs: create and use pseudoref update and delete functions",
Jul 31, 2015), with the justification "alternate ref backends still
need to store pseudorefs in GIT_DIR".

Refs such as REBASE_HEAD are read through the ref backend. This can
only work consistently if they are written through the ref backend as
well. Tooling that works directly on files under .git should be
updated to use git commands to read refs instead.

This needs the following fixes

* Only write log for HEAD pseudo ref. Fixes t1400
  'core.logAllRefUpdates=always creates no reflog for ORIG_HEAD'

* t1400: change hard coded error messages to check for.

* don't deref non-HEAD pseudoref symrefs. This fixes t1405. Without
  this, a deleting a FOO symref pointing to refs/heads/master will remove
  master too.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 refs.c                | 120 +++---------------------------------------
 refs/files-backend.c  |  11 ++--
 t/t1400-update-ref.sh |   6 +--
 3 files changed, 17 insertions(+), 120 deletions(-)

diff --git a/refs.c b/refs.c
index 224ff66c7b..a2fd42364f 100644
--- a/refs.c
+++ b/refs.c
@@ -739,102 +739,6 @@ long get_files_ref_lock_timeout_ms(void)
 	return timeout_ms;
 }
 
-static int write_pseudoref(const char *pseudoref, const struct object_id *oid,
-			   const struct object_id *old_oid, struct strbuf *err)
-{
-	const char *filename;
-	int fd;
-	struct lock_file lock = LOCK_INIT;
-	struct strbuf buf = STRBUF_INIT;
-	int ret = -1;
-
-	if (!oid)
-		return 0;
-
-	strbuf_addf(&buf, "%s\n", oid_to_hex(oid));
-
-	filename = git_path("%s", pseudoref);
-	fd = hold_lock_file_for_update_timeout(&lock, filename, 0,
-					       get_files_ref_lock_timeout_ms());
-	if (fd < 0) {
-		strbuf_addf(err, _("could not open '%s' for writing: %s"),
-			    filename, strerror(errno));
-		goto done;
-	}
-
-	if (old_oid) {
-		struct object_id actual_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 (!oideq(&actual_old_oid, old_oid)) {
-			strbuf_addf(err, _("unexpected object ID when writing '%s'"),
-				    pseudoref);
-			rollback_lock_file(&lock);
-			goto done;
-		}
-	}
-
-	if (write_in_full(fd, buf.buf, buf.len) < 0) {
-		strbuf_addf(err, _("could not write to '%s'"), filename);
-		rollback_lock_file(&lock);
-		goto done;
-	}
-
-	commit_lock_file(&lock);
-	ret = 0;
-done:
-	strbuf_release(&buf);
-	return ret;
-}
-
-static int delete_pseudoref(const char *pseudoref, const struct object_id *old_oid)
-{
-	const char *filename;
-
-	filename = git_path("%s", pseudoref);
-
-	if (old_oid && !is_null_oid(old_oid)) {
-		struct lock_file lock = LOCK_INIT;
-		int fd;
-		struct object_id actual_old_oid;
-
-		fd = hold_lock_file_for_update_timeout(
-				&lock, filename, 0,
-				get_files_ref_lock_timeout_ms());
-		if (fd < 0) {
-			error_errno(_("could not open '%s' for writing"),
-				    filename);
-			return -1;
-		}
-		if (read_ref(pseudoref, &actual_old_oid))
-			die(_("could not read ref '%s'"), pseudoref);
-		if (!oideq(&actual_old_oid, old_oid)) {
-			error(_("unexpected object ID when deleting '%s'"),
-			      pseudoref);
-			rollback_lock_file(&lock);
-			return -1;
-		}
-
-		unlink(filename);
-		rollback_lock_file(&lock);
-	} else {
-		unlink(filename);
-	}
-
-	return 0;
-}
-
 int refs_delete_ref(struct ref_store *refs, const char *msg,
 		    const char *refname,
 		    const struct object_id *old_oid,
@@ -843,11 +747,6 @@ int refs_delete_ref(struct ref_store *refs, const char *msg,
 	struct ref_transaction *transaction;
 	struct strbuf err = STRBUF_INIT;
 
-	if (ref_type(refname) == REF_TYPE_PSEUDOREF) {
-		assert(refs == get_main_ref_store(the_repository));
-		return delete_pseudoref(refname, old_oid);
-	}
-
 	transaction = ref_store_transaction_begin(refs, &err);
 	if (!transaction ||
 	    ref_transaction_delete(transaction, refname, old_oid,
@@ -1170,18 +1069,13 @@ int refs_update_ref(struct ref_store *refs, const char *msg,
 	struct strbuf err = STRBUF_INIT;
 	int ret = 0;
 
-	if (ref_type(refname) == REF_TYPE_PSEUDOREF) {
-		assert(refs == get_main_ref_store(the_repository));
-		ret = write_pseudoref(refname, new_oid, old_oid, &err);
-	} else {
-		t = ref_store_transaction_begin(refs, &err);
-		if (!t ||
-		    ref_transaction_update(t, refname, new_oid, old_oid,
-					   flags, msg, &err) ||
-		    ref_transaction_commit(t, &err)) {
-			ret = 1;
-			ref_transaction_free(t);
-		}
+	t = ref_store_transaction_begin(refs, &err);
+	if (!t ||
+	    ref_transaction_update(t, refname, new_oid, old_oid, flags, msg,
+				   &err) ||
+	    ref_transaction_commit(t, &err)) {
+		ret = 1;
+		ref_transaction_free(t);
 	}
 	if (ret) {
 		const char *str = _("update_ref failed for ref '%s': %s");
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 6516c7bc8c..9951c2e403 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1228,7 +1228,6 @@ static int files_delete_refs(struct ref_store *ref_store, const char *msg,
 
 	for (i = 0; i < refnames->nr; i++) {
 		const char *refname = refnames->items[i].string;
-
 		if (refs_delete_ref(&refs->base, msg, refname, NULL, flags))
 			result |= error(_("could not remove reference %s"), refname);
 	}
@@ -2436,7 +2435,9 @@ static int lock_ref_for_update(struct files_ref_store *refs,
 	update->backend_data = lock;
 
 	if (update->type & REF_ISSYMREF) {
-		if (update->flags & REF_NO_DEREF) {
+		if (update->flags & REF_NO_DEREF ||
+		    (ref_type(update->refname) == REF_TYPE_PSEUDOREF &&
+		     strcmp(update->refname, "HEAD"))) {
 			/*
 			 * We won't be reading the referent as part of
 			 * the transaction, so we have to read it here
@@ -2782,8 +2783,10 @@ static int files_transaction_finish(struct ref_store *ref_store,
 		struct ref_update *update = transaction->updates[i];
 		struct ref_lock *lock = update->backend_data;
 
-		if (update->flags & REF_NEEDS_COMMIT ||
-		    update->flags & REF_LOG_ONLY) {
+		if ((ref_type(lock->ref_name) != REF_TYPE_PSEUDOREF ||
+		     !strcmp(lock->ref_name, "HEAD")) &&
+		    (update->flags & REF_NEEDS_COMMIT ||
+		     update->flags & REF_LOG_ONLY)) {
 			if (files_log_ref_write(refs,
 						lock->ref_name,
 						&lock->old_oid,
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 27171f8261..6b8030e8fe 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -476,7 +476,7 @@ test_expect_success 'git cat-file blob master@{2005-05-26 23:42}:F (expect OTHER
 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 &&
-	test_i18ngrep "could not read ref" err
+	test_i18ngrep "unable to resolve reference" err
 '
 
 test_expect_success 'create pseudoref' '
@@ -497,7 +497,7 @@ test_expect_success 'overwrite pseudoref with correct old value' '
 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) &&
-	test_i18ngrep "unexpected object ID" err
+	test_i18ngrep "cannot lock ref.*expected" err
 '
 
 test_expect_success 'delete pseudoref' '
@@ -509,7 +509,7 @@ 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) &&
-	test_i18ngrep "unexpected object ID" err
+	test_i18ngrep "cannot lock ref.*expected" err
 '
 
 test_expect_success 'delete pseudoref with correct old value' '
-- 
gitgitgadget


  reply	other threads:[~2020-07-06 17:29 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-06 17:29 [PATCH 0/2] Remove special casing for PSEUDOREF updates Han-Wen Nienhuys via GitGitGadget
2020-07-06 17:29 ` Han-Wen Nienhuys via GitGitGadget [this message]
2020-07-06 20:26   ` [PATCH 1/2] Modify pseudo refs through ref backend storage Junio C Hamano
2020-07-07 13:56     ` Han-Wen Nienhuys
2020-07-07 15:20       ` Junio C Hamano
2020-07-07 17:15         ` Han-Wen Nienhuys
2020-07-07 18:14           ` Junio C Hamano
2020-07-06 17:29 ` [PATCH 2/2] Make HEAD a PSEUDOREF rather than PER_WORKTREE Han-Wen Nienhuys via GitGitGadget
2020-07-06 20:30   ` Junio C Hamano
2020-07-07  9:24     ` Han-Wen Nienhuys
2020-07-09 21:11 ` [PATCH v2 0/3] Remove special casing for PSEUDOREF updates Han-Wen Nienhuys via GitGitGadget
2020-07-09 21:11   ` [PATCH v2 1/3] t1400: use git rev-parse for testing PSEUDOREF existence Han-Wen Nienhuys via GitGitGadget
2020-07-09 21:11   ` [PATCH v2 2/3] Modify pseudo refs through ref backend storage Han-Wen Nienhuys via GitGitGadget
2020-07-09 21:11   ` [PATCH v2 3/3] Make HEAD a PSEUDOREF rather than PER_WORKTREE Han-Wen Nienhuys via GitGitGadget
2020-07-10 19:25   ` [PATCH v2 0/3] Remove special casing for PSEUDOREF updates Junio C Hamano
2020-07-16 18:45   ` [PATCH v3 " Han-Wen Nienhuys via GitGitGadget
2020-07-16 18:45     ` [PATCH v3 1/3] t1400: use git rev-parse for testing PSEUDOREF existence Han-Wen Nienhuys via GitGitGadget
2020-07-16 18:45     ` [PATCH v3 2/3] Modify pseudo refs through ref backend storage Han-Wen Nienhuys via GitGitGadget
2020-07-17  8:52       ` Johannes Schindelin
2020-07-27 15:41         ` Han-Wen Nienhuys
2020-07-16 18:45     ` [PATCH v3 3/3] Make HEAD a PSEUDOREF rather than PER_WORKTREE Han-Wen Nienhuys via GitGitGadget
2020-07-16 22:09     ` [PATCH v3 0/3] Remove special casing for PSEUDOREF updates Junio C Hamano
2020-07-27  8:50       ` Han-Wen Nienhuys
2020-07-27 16:20         ` Junio C Hamano
2020-08-03 19:07           ` Han-Wen Nienhuys
2020-08-03 20:07             ` Junio C Hamano
2020-08-05  1:45               ` Junio C Hamano
2020-08-05 10:48                 ` Han-Wen Nienhuys
2020-08-05 15:54                   ` Junio C Hamano
2020-08-10 14:27                     ` Han-Wen Nienhuys
2020-08-10 16:04                       ` Junio C Hamano
2020-08-11 10:49                         ` Han-Wen Nienhuys
2020-08-11 18:38                           ` Junio C Hamano
2020-08-19 13:19                     ` Han-Wen Nienhuys
2020-08-19 15:52                       ` Junio C Hamano
2020-07-27 16:25     ` [PATCH v4 " Han-Wen Nienhuys via GitGitGadget
2020-07-27 16:25       ` [PATCH v4 1/3] t1400: use git rev-parse for testing PSEUDOREF existence Han-Wen Nienhuys via GitGitGadget
2020-07-27 16:25       ` [PATCH v4 2/3] Modify pseudo refs through ref backend storage Han-Wen Nienhuys via GitGitGadget
2020-07-27 16:25       ` [PATCH v4 3/3] Make HEAD a PSEUDOREF rather than PER_WORKTREE Han-Wen Nienhuys via GitGitGadget

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=6821f57bdf326f161f152a8af0e47b54513c77b1.1594056572.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=hanwen@google.com \
    --cc=hanwenn@gmail.com \
    --subject='Re: [PATCH 1/2] Modify pseudo refs through ref backend storage' \
    /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

Code repositories for project(s) associated with this 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).