[PATCH 3/3] Fix checkout not to clobber the branch when using symlinked HEAD upon detaching
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:45:30
Subsystem:
the rest · Maintainer:
Linus Torvalds
When you are using core.prefersymlinkrefs (i.e. your ".git/HEAD" is a
symlink to "refs/heads/$current_branch"), attempt to detach HEAD resulted
in clobbering the tip of the current branch.
The offending callchain is:
update_ref(..., "HEAD", REF_NODEREF, ...);
-> lock_any_ref_for_update("HEAD", ..., REF_NODEREF);
-> lock_ref_sha1_basic("HEAD", ..., REF_NODEREF, ...);
. calls resolve_ref() to read HEAD to arrive at
refs/heads/master
. however, it notices REF_NODEREF and adjusts the ref to be updated
back to "HEAD";
-> hold_lock_file_for_update(..., "HEAD", 1);
-> lock_file(..., "HEAD");
. resolves symlink "HEAD" to "refs/heads/master", and
locks it! This creates "refs/heads/master.lock", that is
then renamed to "refs/heads/master" when unlocked.
The behaviour of lock_file() to resolve symlink at this point in the code
comes from d58e8d3 (When locking in a symlinked repository, try to lock
the original, 2007-07-25), and as explained in the log message of that
commit, we cannot unconditionally remove it.
This patch fixes this. It teaches lock_file() not to dereference the
symbolic link when LOCK_NODEREF is given, and uses this new flag in
lock_ref_sha1_basic() when it is operating directly on HEAD (iow when
REF_NODEREF was given to it).
Signed-off-by: Junio C Hamano <redacted>
---
I haven't tested it beyond running the full testsuite, which it does pass,
but I can't give any more guarantee than that. Testing is for wimps ;-)
cache.h | 1 +
lockfile.c | 3 ++-
refs.c | 10 ++++++----
t/t7201-co.sh | 2 +-
4 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/cache.h b/cache.h
index 941a9dc..8ab2fd8 100644
--- a/cache.h
+++ b/cache.h@@ -412,6 +412,7 @@ struct lock_file { char filename[PATH_MAX]; }; #define LOCK_DIE_ON_ERROR 1 +#define LOCK_NODEREF 2 extern int hold_lock_file_for_update(struct lock_file *, const char *path, int); extern int hold_lock_file_for_append(struct lock_file *, const char *path, int); extern int commit_lock_file(struct lock_file *);
diff --git a/lockfile.c b/lockfile.c
index bc1b585..6d75608 100644
--- a/lockfile.c
+++ b/lockfile.c@@ -130,7 +130,8 @@ static int lock_file(struct lock_file *lk, const char *path, int flags) * subtract 5 from size to make sure there's room for adding * ".lock" for the lock file name */ - resolve_symlink(lk->filename, sizeof(lk->filename)-5); + if (!(flags & LOCK_NODEREF)) + resolve_symlink(lk->filename, sizeof(lk->filename)-5); strcat(lk->filename, ".lock"); lk->fd = open(lk->filename, O_RDWR | O_CREAT | O_EXCL, 0666); if (0 <= lk->fd) {
diff --git a/refs.c b/refs.c
index 5467e98..9e422dc 100644
--- a/refs.c
+++ b/refs.c@@ -790,7 +790,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char struct ref_lock *lock; struct stat st; int last_errno = 0; - int type; + int type, lflags; int mustexist = (old_sha1 && !is_null_sha1(old_sha1)); lock = xcalloc(1, sizeof(struct ref_lock));
@@ -830,8 +830,11 @@ static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char lock->lk = xcalloc(1, sizeof(struct lock_file)); - if (flags & REF_NODEREF) + lflags = LOCK_DIE_ON_ERROR; + if (flags & REF_NODEREF) { ref = orig_ref; + lflags |= LOCK_NODEREF; + } lock->ref_name = xstrdup(ref); lock->orig_ref_name = xstrdup(orig_ref); ref_file = git_path("%s", ref);
@@ -845,9 +848,8 @@ static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char error("unable to create directory for %s", ref_file); goto error_return; } - lock->lock_fd = hold_lock_file_for_update(lock->lk, ref_file, - LOCK_DIE_ON_ERROR); + lock->lock_fd = hold_lock_file_for_update(lock->lk, ref_file, lflags); return old_sha1 ? verify_lock(lock, old_sha1, mustexist) : lock; error_return:
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 01304d7..d9a80aa 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh@@ -339,7 +339,7 @@ test_expect_success 'checkout w/--track from non-branch HEAD fails' ' test "z$(git rev-parse master^0)" = "z$(git rev-parse HEAD)" ' -test_expect_failure 'detch a symbolic link HEAD' ' +test_expect_success 'detch a symbolic link HEAD' ' git checkout master && git config --bool core.prefersymlinkrefs yes && git checkout side &&
--
1.6.0.2.734.gae0be