summaryrefslogtreecommitdiff
path: root/t
diff options
context:
space:
mode:
authorMichael Haggerty <mhagger@alum.mit.edu>2015-05-11 17:25:12 +0200
committerJunio C Hamano <gitster@pobox.com>2015-05-11 11:50:19 -0700
commite911104c84f2efd64aded6831bd7e6625c8a0fab (patch)
tree35b06c3e491bf34c11d9b43d9d4870d011daafde /t
parent07f9c881d6acef35b7dc8e5c783e63b1cac71084 (diff)
downloadgit-e911104c84f2efd64aded6831bd7e6625c8a0fab.tar.gz
refs: check for D/F conflicts among refs created in a transaction
If two references that D/F conflict (e.g., "refs/foo" and "refs/foo/bar") are created in a single transaction, the old code discovered the problem only after the "commit" phase of ref_transaction_commit() had already begun. This could leave some references updated and others not, which violates the promise of atomicity. Instead, check for such conflicts during the "locking" phase: * Teach is_refname_available() to take an "extras" parameter that can contain extra reference names with which the specified refname must not conflict. * Change lock_ref_sha1_basic() to take an "extras" parameter, which it passes through to is_refname_available(). * Change ref_transaction_commit() to pass "affected_refnames" to lock_ref_sha1_basic() as its "extras" argument. This change fixes a test case in t1404. This code is a bit stricter than it needs to be. We could conceivably allow reference "refs/foo/bar" to be created in the same transaction as "refs/foo" is deleted (or vice versa). But that would be complicated to implement, because it is not possible to lock "refs/foo/bar" while "refs/foo" exists as a loose reference, but on the other hand we don't want to delete some references before adding others (because that could leave a gap during which required objects are unreachable). There is also a complication that reflog files' paths can conflict. Any less-strict implementation would probably require tricks like the packing of all references before the start of the real transaction, or the use of temporary intermediate reference names. So for now let's accept too-strict checks. Some reference update transactions will be rejected unnecessarily, but they will be rejected in their entirety rather than leaving the repository in an intermediate state, as would happen now. Please note that there is still one kind of D/F conflict that is *not* handled correctly. If two processes are running at the same time, and one tries to create "refs/foo" at the same time that the other tries to create "refs/foo/bar", then they can race with each other. Both processes can obtain their respective locks ("refs/foo.lock" and "refs/foo/bar.lock"), proceed to the "commit" phase of ref_transaction_commit(), and then the slower process will discover that it cannot rename its lockfile into place (after possibly having committed changes to other references). There appears to be no way to fix this race without changing the locking policy, which in turn would require a change to *all* Git clients. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Diffstat (limited to 't')
-rwxr-xr-xt/t1404-update-ref-df-conflicts.sh2
1 files changed, 1 insertions, 1 deletions
diff --git a/t/t1404-update-ref-df-conflicts.sh b/t/t1404-update-ref-df-conflicts.sh
index 2541a231bc..6d6200a9a3 100755
--- a/t/t1404-update-ref-df-conflicts.sh
+++ b/t/t1404-update-ref-df-conflicts.sh
@@ -96,7 +96,7 @@ test_expect_success 'new ref is a deeper prefix of existing packed' '
'
-test_expect_failure 'one new ref is a simple prefix of another' '
+test_expect_success 'one new ref is a simple prefix of another' '
prefix=refs/5 &&
test_update_rejected $prefix "a e" false "b c c/x d" \