[PATCH v13 35/41] refs.c: pack all refs before we start to rename a ref

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This means that most loose refs will no longer be present after the rename
which triggered a test failure since it assumes the file for an unrelated
ref would still be present after the rename. This also makes the rename
itself slightly slower, but as it now results in all refs being packed future
commands accessing refs might become slightly faster.

We want to do this to make it easier to handle atomic renames in rename_ref for
the case 'git branch -m foo/bar foo'. If we can guarantee that foo/bar does not
exist as a loose ref we can avoid locking foo/bar.lock during transaction
commit and thus make it possible to delete the foo directory and re-create
it as a file(branch) in a single transaction.

There is a small race between when we pack all refs and we eventually remove
the old ref from the packed refs file. If a concurrent process deletes the old
ref from the packed refs file, then creates a new ref with that name and
finally repacks the packed refs file, then our transaction will delete the new
ref without realizing it had changed during the transaction. The window for this
is very short and is still a slightly less race than the ones in the old
code for ref_rename.

Reviewed-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
Signed-off-by: Ronnie Sahlberg <sahlberg@xxxxxxxxxx>
---
 refs.c            | 3 +++
 t/t3200-branch.sh | 6 +++---
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/refs.c b/refs.c
index 8848dbf..03cff59 100644
--- a/refs.c
+++ b/refs.c
@@ -2652,6 +2652,9 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms
 		return error("unable to move logfile logs/%s to "TMP_RENAMED_LOG": %s",
 			oldrefname, strerror(errno));
 
+	if (pack_refs(PACK_REFS_ALL | PACK_REFS_PRUNE))
+		return error("unable to pack refs");
+
 	if (delete_ref(oldrefname, orig_sha1, REF_NODEREF)) {
 		error("unable to delete old %s", oldrefname);
 		goto rollback;
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index ac31b71..de0c2b9 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -288,9 +288,9 @@ test_expect_success 'deleting a dangling symref' '
 test_expect_success 'renaming a symref is not allowed' '
 	git symbolic-ref refs/heads/master2 refs/heads/master &&
 	test_must_fail git branch -m master2 master3 &&
-	git symbolic-ref refs/heads/master2 &&
-	test_path_is_file .git/refs/heads/master &&
-	test_path_is_missing .git/refs/heads/master3
+	git rev-parse --verify refs/heads/master &&
+	test_must_fail git symbolic-ref refs/heads/master3 &&
+	test_must_fail git rev-parse refs/heads/master3
 '
 
 test_expect_success SYMLINKS 'git branch -m u v should fail when the reflog for u is a symlink' '
-- 
2.0.0.567.g64a7adf

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]