diff options
author | Jeff King <peff@peff.net> | 2015-12-29 00:57:01 -0500 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2015-12-29 10:33:31 -0800 |
commit | 370e5ad65e8878989eecbae28a479b1a4d6a841b (patch) | |
tree | 26d398e38837a10faf7d108c6eb7e4ffa41ce550 /t | |
parent | b9badadd06ae307c5e1e0e7c36985a1360cabc22 (diff) | |
download | git-370e5ad65e8878989eecbae28a479b1a4d6a841b.tar.gz |
create_symref: use existing ref-lock code
The create_symref() function predates the existence of
"struct lock_file", let alone the more recent "struct
ref_lock". Instead, it just does its own manual dot-locking.
Besides being more code, this has a few downsides:
- if git is interrupted while holding the lock, we don't
clean up the lockfile
- we don't do the usual directory/filename conflict check.
So you can sometimes create a symref "refs/heads/foo/bar",
even if "refs/heads/foo" exists (namely, if the refs are
packed and we do not hit the d/f conflict in the
filesystem).
This patch refactors create_symref() to use the "struct
ref_lock" interface, which handles both of these things.
There are a few bonus cleanups that come along with it:
- we leaked ref_path in some error cases
- the symref contents were stored in a fixed-size buffer,
putting an artificial (albeit large) limitation on the
length of the refname. We now write through fprintf, and
handle refnames of any size.
- we called adjust_shared_perm only after the file was
renamed into place, creating a potential race with
readers in a shared repository. The lockfile code now
handles this when creating the lockfile, making it
atomic.
- the legacy prefer_symlink_refs path did not do any
locking at all. Admittedly, it is not atomic from a
reader's perspective (as it unlinks and re-creates the
symlink to overwrite), but at least it cannot conflict
with other writers now.
- the result of this patch is hopefully more readable. It
eliminates three goto labels. Two were for error checking
that is now simplified, and the third was to reach shared
code that has been pulled into its own function.
Signed-off-by: Jeff King <peff@peff.net>
Reviewed-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 't')
-rwxr-xr-x | t/t1401-symbolic-ref.sh | 8 |
1 files changed, 8 insertions, 0 deletions
diff --git a/t/t1401-symbolic-ref.sh b/t/t1401-symbolic-ref.sh index 1f0dff3a0b..5db876c6a1 100755 --- a/t/t1401-symbolic-ref.sh +++ b/t/t1401-symbolic-ref.sh @@ -114,4 +114,12 @@ test_expect_success 'symbolic-ref writes reflog entry' ' test_cmp expect actual ' +test_expect_success 'symbolic-ref does not create ref d/f conflicts' ' + git checkout -b df && + test_commit df && + test_must_fail git symbolic-ref refs/heads/df/conflict refs/heads/df && + git pack-refs --all --prune && + test_must_fail git symbolic-ref refs/heads/df/conflict refs/heads/df +' + test_done |