From b9badadd06ae307c5e1e0e7c36985a1360cabc22 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 29 Dec 2015 00:56:44 -0500 Subject: create_symref: modernize variable names Once upon a time, create_symref() was used only to point HEAD at a branch name, and the variable names reflect that (e.g., calling the path git_HEAD). However, it is much more generic these days (and has been for some time). Let's update the variable names to make it easier to follow: - `ref_target` is now just `refname`. This is closer to the `ref` that is already in `cache.h`, but with the extra twist that "name" makes it clear this is the name and not a ref struct. Dropping "target" hopefully makes it clear that we are talking about the symref itself, not what it points to. - `git_HEAD` is now `ref_path`; the on-disk path corresponding to `ref`. - `refs_heads_master` is now just `target`; i.e., what the symref points at. This term also matches what is in the symlink(2) manpage (at least on Linux). - the buffer to hold the symref file's contents was simply called `ref`. It's now `buf` (admittedly also generic, but at least not actively introducing confusion with the other variable holding the refname). Signed-off-by: Jeff King Reviewed-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.h | 2 +- refs/files-backend.c | 41 ++++++++++++++++++++--------------------- 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/refs.h b/refs.h index 7a0407748..3c3da29bf 100644 --- a/refs.h +++ b/refs.h @@ -292,7 +292,7 @@ extern char *shorten_unambiguous_ref(const char *refname, int strict); /** rename ref, return 0 on success **/ extern int rename_ref(const char *oldref, const char *newref, const char *logmsg); -extern int create_symref(const char *ref, const char *refs_heads_master, const char *logmsg); +extern int create_symref(const char *refname, const char *target, const char *logmsg); enum action_on_err { UPDATE_REFS_MSG_ON_ERR, diff --git a/refs/files-backend.c b/refs/files-backend.c index c648b5e85..d6f912363 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -2811,58 +2811,57 @@ static int commit_ref_update(struct ref_lock *lock, return 0; } -int create_symref(const char *ref_target, const char *refs_heads_master, - const char *logmsg) +int create_symref(const char *refname, const char *target, const char *logmsg) { char *lockpath = NULL; - char ref[1000]; + char buf[1000]; int fd, len, written; - char *git_HEAD = git_pathdup("%s", ref_target); + char *ref_path = git_pathdup("%s", refname); unsigned char old_sha1[20], new_sha1[20]; struct strbuf err = STRBUF_INIT; - if (logmsg && read_ref(ref_target, old_sha1)) + if (logmsg && read_ref(refname, old_sha1)) hashclr(old_sha1); - if (safe_create_leading_directories(git_HEAD) < 0) - return error("unable to create directory for %s", git_HEAD); + if (safe_create_leading_directories(ref_path) < 0) + return error("unable to create directory for %s", ref_path); #ifndef NO_SYMLINK_HEAD if (prefer_symlink_refs) { - unlink(git_HEAD); - if (!symlink(refs_heads_master, git_HEAD)) + unlink(ref_path); + if (!symlink(target, ref_path)) goto done; fprintf(stderr, "no symlink - falling back to symbolic ref\n"); } #endif - len = snprintf(ref, sizeof(ref), "ref: %s\n", refs_heads_master); - if (sizeof(ref) <= len) { - error("refname too long: %s", refs_heads_master); + len = snprintf(buf, sizeof(buf), "ref: %s\n", target); + if (sizeof(buf) <= len) { + error("refname too long: %s", target); goto error_free_return; } - lockpath = mkpathdup("%s.lock", git_HEAD); + lockpath = mkpathdup("%s.lock", ref_path); fd = open(lockpath, O_CREAT | O_EXCL | O_WRONLY, 0666); if (fd < 0) { error("Unable to open %s for writing", lockpath); goto error_free_return; } - written = write_in_full(fd, ref, len); + written = write_in_full(fd, buf, len); if (close(fd) != 0 || written != len) { error("Unable to write to %s", lockpath); goto error_unlink_return; } - if (rename(lockpath, git_HEAD) < 0) { - error("Unable to create %s", git_HEAD); + if (rename(lockpath, ref_path) < 0) { + error("Unable to create %s", ref_path); goto error_unlink_return; } - if (adjust_shared_perm(git_HEAD)) { + if (adjust_shared_perm(ref_path)) { error("Unable to fix permissions on %s", lockpath); error_unlink_return: unlink_or_warn(lockpath); error_free_return: free(lockpath); - free(git_HEAD); + free(ref_path); return -1; } free(lockpath); @@ -2870,13 +2869,13 @@ int create_symref(const char *ref_target, const char *refs_heads_master, #ifndef NO_SYMLINK_HEAD done: #endif - if (logmsg && !read_ref(refs_heads_master, new_sha1) && - log_ref_write(ref_target, old_sha1, new_sha1, logmsg, 0, &err)) { + if (logmsg && !read_ref(target, new_sha1) && + log_ref_write(refname, old_sha1, new_sha1, logmsg, 0, &err)) { error("%s", err.buf); strbuf_release(&err); } - free(git_HEAD); + free(ref_path); return 0; } -- cgit v1.2.1 From 370e5ad65e8878989eecbae28a479b1a4d6a841b Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 29 Dec 2015 00:57:01 -0500 Subject: 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 Reviewed-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs/files-backend.c | 109 ++++++++++++++++++++++++------------------------ t/t1401-symbolic-ref.sh | 8 ++++ 2 files changed, 62 insertions(+), 55 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index d6f912363..3d1994deb 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -2811,74 +2811,73 @@ static int commit_ref_update(struct ref_lock *lock, return 0; } -int create_symref(const char *refname, const char *target, const char *logmsg) +static int create_ref_symlink(struct ref_lock *lock, const char *target) { - char *lockpath = NULL; - char buf[1000]; - int fd, len, written; - char *ref_path = git_pathdup("%s", refname); - unsigned char old_sha1[20], new_sha1[20]; - struct strbuf err = STRBUF_INIT; - - if (logmsg && read_ref(refname, old_sha1)) - hashclr(old_sha1); - - if (safe_create_leading_directories(ref_path) < 0) - return error("unable to create directory for %s", ref_path); - + int ret = -1; #ifndef NO_SYMLINK_HEAD - if (prefer_symlink_refs) { - unlink(ref_path); - if (!symlink(target, ref_path)) - goto done; + char *ref_path = get_locked_file_path(lock->lk); + unlink(ref_path); + ret = symlink(target, ref_path); + free(ref_path); + + if (ret) fprintf(stderr, "no symlink - falling back to symbolic ref\n"); - } #endif + return ret; +} - len = snprintf(buf, sizeof(buf), "ref: %s\n", target); - if (sizeof(buf) <= len) { - error("refname too long: %s", target); - goto error_free_return; - } - lockpath = mkpathdup("%s.lock", ref_path); - fd = open(lockpath, O_CREAT | O_EXCL | O_WRONLY, 0666); - if (fd < 0) { - error("Unable to open %s for writing", lockpath); - goto error_free_return; - } - written = write_in_full(fd, buf, len); - if (close(fd) != 0 || written != len) { - error("Unable to write to %s", lockpath); - goto error_unlink_return; - } - if (rename(lockpath, ref_path) < 0) { - error("Unable to create %s", ref_path); - goto error_unlink_return; - } - if (adjust_shared_perm(ref_path)) { - error("Unable to fix permissions on %s", lockpath); - error_unlink_return: - unlink_or_warn(lockpath); - error_free_return: - free(lockpath); - free(ref_path); - return -1; - } - free(lockpath); - -#ifndef NO_SYMLINK_HEAD - done: -#endif +static void update_symref_reflog(struct ref_lock *lock, const char *refname, + const char *target, const char *logmsg) +{ + struct strbuf err = STRBUF_INIT; + unsigned char new_sha1[20]; if (logmsg && !read_ref(target, new_sha1) && - log_ref_write(refname, old_sha1, new_sha1, logmsg, 0, &err)) { + log_ref_write(refname, lock->old_oid.hash, new_sha1, logmsg, 0, &err)) { error("%s", err.buf); strbuf_release(&err); } +} - free(ref_path); +static int create_symref_locked(struct ref_lock *lock, const char *refname, + const char *target, const char *logmsg) +{ + if (prefer_symlink_refs && !create_ref_symlink(lock, target)) { + update_symref_reflog(lock, refname, target, logmsg); + return 0; + } + + if (!fdopen_lock_file(lock->lk, "w")) + return error("unable to fdopen %s: %s", + lock->lk->tempfile.filename.buf, strerror(errno)); + + /* no error check; commit_ref will check ferror */ + fprintf(lock->lk->tempfile.fp, "ref: %s\n", target); + if (commit_ref(lock) < 0) + return error("unable to write symref for %s: %s", refname, + strerror(errno)); + update_symref_reflog(lock, refname, target, logmsg); return 0; } +int create_symref(const char *refname, const char *target, const char *logmsg) +{ + struct strbuf err = STRBUF_INIT; + struct ref_lock *lock; + int ret; + + lock = lock_ref_sha1_basic(refname, NULL, NULL, NULL, REF_NODEREF, NULL, + &err); + if (!lock) { + error("%s", err.buf); + strbuf_release(&err); + return -1; + } + + ret = create_symref_locked(lock, refname, target, logmsg); + unlock_ref(lock); + return ret; +} + int reflog_exists(const char *refname) { struct stat st; diff --git a/t/t1401-symbolic-ref.sh b/t/t1401-symbolic-ref.sh index 1f0dff3a0..5db876c6a 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 -- cgit v1.2.1 From 396da8f7a07ae02a6152e0c0fc3eaac8a99b4c65 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 29 Dec 2015 00:57:25 -0500 Subject: create_symref: write reflog while holding lock We generally hold a lock on the matching ref while writing to its reflog; this prevents two simultaneous writers from clobbering each other's reflog lines (it does not even have to be two symref updates; because we don't hold the lock, we could race with somebody writing to the pointed-to ref via HEAD, for example). We can fix this by writing the reflog before we commit the lockfile. This runs the risk of writing the reflog but failing the final rename(), but at least we now err on the same side as the rest of the ref code. Noticed-by: Michael Haggerty Signed-off-by: Jeff King Reviewed-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs/files-backend.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 3d1994deb..180c837d1 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -2850,12 +2850,13 @@ static int create_symref_locked(struct ref_lock *lock, const char *refname, return error("unable to fdopen %s: %s", lock->lk->tempfile.filename.buf, strerror(errno)); + update_symref_reflog(lock, refname, target, logmsg); + /* no error check; commit_ref will check ferror */ fprintf(lock->lk->tempfile.fp, "ref: %s\n", target); if (commit_ref(lock) < 0) return error("unable to write symref for %s: %s", refname, strerror(errno)); - update_symref_reflog(lock, refname, target, logmsg); return 0; } -- cgit v1.2.1 From 4be49d756894daca0e8a4477d36c6ed1096ccddc Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 12 Jan 2016 04:57:34 -0500 Subject: checkout,clone: check return value of create_symref It's unlikely that we would fail to create or update a symbolic ref (especially HEAD), but if we do, we should notice and complain. Note that there's no need to give more details in our error message; create_symref will already have done so. While we're here, let's also fix a minor memory leak in clone. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/checkout.c | 3 ++- builtin/clone.c | 11 +++++++---- t/t2011-checkout-invalid-head.sh | 6 ++++++ 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index e8110a924..5af84a311 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -661,7 +661,8 @@ static void update_refs_for_switch(const struct checkout_opts *opts, describe_detached_head(_("HEAD is now at"), new->commit); } } else if (new->path) { /* Switch branches. */ - create_symref("HEAD", new->path, msg.buf); + if (create_symref("HEAD", new->path, msg.buf) < 0) + die("unable to update HEAD"); if (!opts->quiet) { if (old->path && !strcmp(new->path, old->path)) { if (opts->new_branch_force) diff --git a/builtin/clone.c b/builtin/clone.c index a0b3cd9e5..a7c8def8c 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -636,9 +636,11 @@ static void update_remote_refs(const struct ref *refs, struct strbuf head_ref = STRBUF_INIT; strbuf_addstr(&head_ref, branch_top); strbuf_addstr(&head_ref, "HEAD"); - create_symref(head_ref.buf, - remote_head_points_at->peer_ref->name, - msg); + if (create_symref(head_ref.buf, + remote_head_points_at->peer_ref->name, + msg) < 0) + die("unable to update %s", head_ref.buf); + strbuf_release(&head_ref); } } @@ -648,7 +650,8 @@ static void update_head(const struct ref *our, const struct ref *remote, const char *head; if (our && skip_prefix(our->name, "refs/heads/", &head)) { /* Local default branch link */ - create_symref("HEAD", our->name, NULL); + if (create_symref("HEAD", our->name, NULL) < 0) + die("unable to update HEAD"); if (!option_bare) { update_ref(msg, "HEAD", our->old_oid.hash, NULL, 0, UPDATE_REFS_DIE_ON_ERR); diff --git a/t/t2011-checkout-invalid-head.sh b/t/t2011-checkout-invalid-head.sh index 300f8bf25..d444d5ee4 100755 --- a/t/t2011-checkout-invalid-head.sh +++ b/t/t2011-checkout-invalid-head.sh @@ -19,4 +19,10 @@ test_expect_success 'checkout master from invalid HEAD' ' git checkout master -- ' +test_expect_success 'checkout notices failure to lock HEAD' ' + test_when_finished "rm -f .git/HEAD.lock" && + >.git/HEAD.lock && + test_must_fail git checkout -b other +' + test_done -- cgit v1.2.1 From 6294dcb49fb2654993fbc132f1b49443adfe4c87 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 12 Jan 2016 16:44:39 -0500 Subject: lock_ref_sha1_basic: always fill old_oid while holding lock Our basic strategy for taking a ref lock is: 1. Create $ref.lock to take the lock 2. Read the ref again while holding the lock (during which time we know that nobody else can be updating it). 3. Compare the value we read to the expected "old_sha1" The value we read in step (2) is returned to the caller via the lock->old_oid field, who may use it for other purposes (such as writing a reflog). If we have no "old_sha1" (i.e., we are unconditionally taking the lock), then we obviously must omit step 3. But we _also_ omit step 2. This seems like a nice optimization, but it means that the caller sees only whatever was left in lock->old_oid from previous calls to resolve_ref_unsafe(), which happened outside of the lock. We can demonstrate this race pretty easily. Imagine you have three commits, $one, $two, and $three. One script just flips between $one and $two, without providing an old-sha1: while true; do git update-ref -m one refs/heads/foo $one git update-ref -m two refs/heads/foo $two done Meanwhile, another script tries to set the value to $three, also not using an old-sha1: while true; do git update-ref -m three refs/heads/foo $three done If these run simultaneously, we'll see a lot of lock contention, but each of the writes will succeed some of the time. The reflog may record movements between any of the three refs, but we would expect it to provide a consistent log: the "from" field of each log entry should be the same as the "to" field of the previous one. But if we check this: perl -alne ' print "mismatch on line $." if defined $last && $F[0] ne $last; $last = $F[1]; ' .git/logs/refs/heads/foo we'll see many mismatches. Why? Because sometimes, in the time between lock_ref_sha1_basic filling lock->old_oid via resolve_ref_unsafe() and it taking the lock, there may be a complete write by another process. And the "from" field in our reflog entry will be wrong, and will refer to an older value. This is probably quite rare in practice. It requires writers which do not provide an old-sha1 value, and it is a very quick race. However, it is easy to fix: we simply perform step (2), the read-under-lock, whether we have an old-sha1 or not. Then the value we hand back to the caller is always atomic. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- refs/files-backend.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 180c837d1..69c3ecfe1 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1840,12 +1840,17 @@ static int verify_lock(struct ref_lock *lock, if (read_ref_full(lock->ref_name, mustexist ? RESOLVE_REF_READING : 0, lock->old_oid.hash, NULL)) { - int save_errno = errno; - strbuf_addf(err, "can't verify ref %s", lock->ref_name); - errno = save_errno; - return -1; + if (old_sha1) { + int save_errno = errno; + strbuf_addf(err, "can't verify ref %s", lock->ref_name); + errno = save_errno; + return -1; + } else { + hashclr(lock->old_oid.hash); + return 0; + } } - if (hashcmp(lock->old_oid.hash, old_sha1)) { + if (old_sha1 && hashcmp(lock->old_oid.hash, old_sha1)) { strbuf_addf(err, "ref %s is at %s but expected %s", lock->ref_name, sha1_to_hex(lock->old_oid.hash), @@ -1985,7 +1990,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname, goto error_return; } } - if (old_sha1 && verify_lock(lock, old_sha1, mustexist, err)) { + if (verify_lock(lock, old_sha1, mustexist, err)) { last_errno = errno; goto error_return; } -- cgit v1.2.1 From 2859dcd4c8605a5b9cf35efb815418ea8892658b Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 12 Jan 2016 16:45:09 -0500 Subject: lock_ref_sha1_basic: handle REF_NODEREF with invalid refs We sometimes call lock_ref_sha1_basic with REF_NODEREF to operate directly on a symbolic ref. This is used, for example, to move to a detached HEAD, or when updating the contents of HEAD via checkout or symbolic-ref. However, the first step of the function is to resolve the refname to get the "old" sha1, and we do so without telling resolve_ref_unsafe() that we are only interested in the symref. As a result, we may detect a problem there not with the symref itself, but with something it points to. The real-world example I found (and what is used in the test suite) is a HEAD pointing to a ref that cannot exist, because it would cause a directory/file conflict with other existing refs. This situation is somewhat broken, of course, as trying to _commit_ on that HEAD would fail. But it's not explicitly forbidden, and we should be able to move away from it. However, neither "git checkout" nor "git symbolic-ref" can do so. We try to take the lock on HEAD, which is pointing to a non-existent ref. We bail from resolve_ref_unsafe() with errno set to EISDIR, and the lock code thinks we are attempting to create a d/f conflict. Of course we're not. The problem is that the lock code has no idea what level we were at when we got EISDIR, so trying to diagnose or remove empty directories for HEAD is not useful. To make things even more complicated, we only get EISDIR in the loose-ref case. If the refs are packed, the resolution may "succeed", giving us the pointed-to ref in "refname", but a null oid. Later, we say "ah, the null oid means we are creating; let's make sure there is room for it", but mistakenly check against the _resolved_ refname, not the original. We can fix this by making two tweaks: 1. Call resolve_ref_unsafe() with RESOLVE_REF_NO_RECURSE when REF_NODEREF is set. This means any errors we get will be from the orig_refname, and we can act accordingly. We already do this in the REF_DELETING case, but we should do it for update, too. 2. If we do get a "refname" return from resolve_ref_unsafe(), even with RESOLVE_REF_NO_RECURSE it may be the name of the ref pointed-to by a symref. We already normalize this back to orig_refname before taking the lockfile, but we need to do so before the null_oid check. While we're rearranging the REF_NODEREF handling, we can also bump the initialization of lflags to the top of the function, where we are setting up other flags. This saves us from having yet another conditional block on REF_NODEREF just to set it later. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- refs/files-backend.c | 19 ++++++++++--------- t/t1401-symbolic-ref.sh | 7 +++++++ t/t2011-checkout-invalid-head.sh | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 9 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 69c3ecfe1..81c92b410 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1887,7 +1887,8 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname, const char *orig_refname = refname; struct ref_lock *lock; int last_errno = 0; - int type, lflags; + int type; + int lflags = 0; int mustexist = (old_sha1 && !is_null_sha1(old_sha1)); int resolve_flags = 0; int attempts_remaining = 3; @@ -1898,10 +1899,11 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname, if (mustexist) resolve_flags |= RESOLVE_REF_READING; - if (flags & REF_DELETING) { + if (flags & REF_DELETING) resolve_flags |= RESOLVE_REF_ALLOW_BAD_NAME; - if (flags & REF_NODEREF) - resolve_flags |= RESOLVE_REF_NO_RECURSE; + if (flags & REF_NODEREF) { + resolve_flags |= RESOLVE_REF_NO_RECURSE; + lflags |= LOCK_NO_DEREF; } refname = resolve_ref_unsafe(refname, resolve_flags, @@ -1937,6 +1939,10 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname, goto error_return; } + + if (flags & REF_NODEREF) + refname = orig_refname; + /* * If the ref did not exist and we are creating it, make sure * there is no existing packed ref whose name begins with our @@ -1952,11 +1958,6 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname, lock->lk = xcalloc(1, sizeof(struct lock_file)); - lflags = 0; - if (flags & REF_NODEREF) { - refname = orig_refname; - lflags |= LOCK_NO_DEREF; - } lock->ref_name = xstrdup(refname); lock->orig_ref_name = xstrdup(orig_refname); strbuf_git_path(&ref_file, "%s", refname); diff --git a/t/t1401-symbolic-ref.sh b/t/t1401-symbolic-ref.sh index 5db876c6a..a713766cc 100755 --- a/t/t1401-symbolic-ref.sh +++ b/t/t1401-symbolic-ref.sh @@ -122,4 +122,11 @@ test_expect_success 'symbolic-ref does not create ref d/f conflicts' ' test_must_fail git symbolic-ref refs/heads/df/conflict refs/heads/df ' +test_expect_success 'symbolic-ref handles existing pointer to invalid name' ' + head=$(git rev-parse HEAD) && + git symbolic-ref HEAD refs/heads/outer && + git update-ref refs/heads/outer/inner $head && + git symbolic-ref HEAD refs/heads/unrelated +' + test_done diff --git a/t/t2011-checkout-invalid-head.sh b/t/t2011-checkout-invalid-head.sh index d444d5ee4..c5501b008 100755 --- a/t/t2011-checkout-invalid-head.sh +++ b/t/t2011-checkout-invalid-head.sh @@ -25,4 +25,37 @@ test_expect_success 'checkout notices failure to lock HEAD' ' test_must_fail git checkout -b other ' +test_expect_success 'create ref directory/file conflict scenario' ' + git update-ref refs/heads/outer/inner master && + + # do not rely on symbolic-ref to get a known state, + # as it may use the same code we are testing + reset_to_df () { + echo "ref: refs/heads/outer" >.git/HEAD + } +' + +test_expect_success 'checkout away from d/f HEAD (unpacked, to branch)' ' + reset_to_df && + git checkout master +' + +test_expect_success 'checkout away from d/f HEAD (unpacked, to detached)' ' + reset_to_df && + git checkout --detach master +' + +test_expect_success 'pack refs' ' + git pack-refs --all --prune +' + +test_expect_success 'checkout away from d/f HEAD (packed, to branch)' ' + reset_to_df && + git checkout master +' + +test_expect_success 'checkout away from d/f HEAD (packed, to detached)' ' + reset_to_df && + git checkout --detach master +' test_done -- cgit v1.2.1