From 3b368546a0da81aa44d8c46eb40a7e7b955cab65 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 3 Jun 2011 01:10:10 -0400 Subject: [PATCH 1/4] t: add tests for cloning remotes with detached HEAD We didn't test this setup at all, and doing so reveals a few bugs: 1. Cloning a repository with an orphaned detached HEAD (i.e., one that points to history that is not referenced by any ref) will fail. 2. Cloning a repository with a detached HEAD that points to a tag will cause us to write a bogus "refs/tags/..." ref into the HEAD symbolic ref. We should probably detach instead. 3. Cloning a repository with a detached HEAD that points to a branch will cause us to checkout that branch. This is a known limitation of the git protocol (we have to guess at HEAD's destination, since the symref contents aren't shown to us). This test serves to document the desired behavior, which can only be achieved once the git protocol learns to share symref information. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t5707-clone-detached.sh | 76 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) create mode 100755 t/t5707-clone-detached.sh diff --git a/t/t5707-clone-detached.sh b/t/t5707-clone-detached.sh new file mode 100755 index 0000000000..6cecd4cd3c --- /dev/null +++ b/t/t5707-clone-detached.sh @@ -0,0 +1,76 @@ +#!/bin/sh + +test_description='test cloning a repository with detached HEAD' +. ./test-lib.sh + +head_is_detached() { + git --git-dir=$1/.git rev-parse --verify HEAD && + test_must_fail git --git-dir=$1/.git symbolic-ref HEAD +} + +test_expect_success 'setup' ' + echo one >file && + git add file && + git commit -m one && + echo two >file && + git commit -a -m two && + git tag two && + echo three >file && + git commit -a -m three +' + +test_expect_success 'clone repo (detached HEAD points to branch)' ' + git checkout master^0 && + git clone "file://$PWD" detached-branch +' +test_expect_success 'cloned HEAD matches' ' + echo three >expect && + git --git-dir=detached-branch/.git log -1 --format=%s >actual && + test_cmp expect actual +' +test_expect_failure 'cloned HEAD is detached' ' + head_is_detached detached-branch +' + +test_expect_success 'clone repo (detached HEAD points to tag)' ' + git checkout two^0 && + git clone "file://$PWD" detached-tag +' +test_expect_success 'cloned HEAD matches' ' + echo two >expect && + git --git-dir=detached-tag/.git log -1 --format=%s >actual && + test_cmp expect actual +' +test_expect_failure 'cloned HEAD is detached' ' + head_is_detached detached-tag +' + +test_expect_success 'clone repo (detached HEAD points to history)' ' + git checkout two^ && + git clone "file://$PWD" detached-history +' +test_expect_success 'cloned HEAD matches' ' + echo one >expect && + git --git-dir=detached-history/.git log -1 --format=%s >actual && + test_cmp expect actual +' +test_expect_success 'cloned HEAD is detached' ' + head_is_detached detached-history +' + +test_expect_failure 'clone repo (orphan detached HEAD)' ' + git checkout master^0 && + echo four >file && + git commit -a -m four && + git clone "file://$PWD" detached-orphan +' +test_expect_failure 'cloned HEAD matches' ' + echo four >expect && + git --git-dir=detached-orphan/.git log -1 --format=%s >actual && + test_cmp expect actual +' +test_expect_failure 'cloned HEAD is detached' ' + head_is_detached detached-orphan +' + +test_done From 61adfd30974a6c49b3d07275e9f2c9fc44bf779c Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 3 Jun 2011 01:11:13 -0400 Subject: [PATCH 2/4] consider only branches in guess_remote_head The guess_remote_head function tries to figure out where a remote's HEAD is pointing by comparing the sha1 of the remote's HEAD with the sha1 of various refs found on the remote. However, we were too liberal in matching refs, and would match tags or remote tracking branches, even though these things could not possibly be referenced by the HEAD symbolic ref (since git will detach when checking them out). As a result, a clone of a remote repository with a detached HEAD might write "refs/tags/*" into our local HEAD, which is bogus. The resulting HEAD should be detached. The other related code path is remote.c's get_head_names() (which is used for, among other things, "set-head -a"). This was not affected, however, as that function feeds only refs from refs/heads to guess_remote_head. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- remote.c | 4 +++- t/t5707-clone-detached.sh | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/remote.c b/remote.c index ca42a126ad..f073b1ecf5 100644 --- a/remote.c +++ b/remote.c @@ -1667,7 +1667,9 @@ struct ref *guess_remote_head(const struct ref *head, /* Look for another ref that points there */ for (r = refs; r; r = r->next) { - if (r != head && !hashcmp(r->old_sha1, head->old_sha1)) { + if (r != head && + !prefixcmp(r->name, "refs/heads/") && + !hashcmp(r->old_sha1, head->old_sha1)) { *tail = copy_ref(r); tail = &((*tail)->next); if (!all) diff --git a/t/t5707-clone-detached.sh b/t/t5707-clone-detached.sh index 6cecd4cd3c..d63b1e390e 100755 --- a/t/t5707-clone-detached.sh +++ b/t/t5707-clone-detached.sh @@ -41,7 +41,7 @@ test_expect_success 'cloned HEAD matches' ' git --git-dir=detached-tag/.git log -1 --format=%s >actual && test_cmp expect actual ' -test_expect_failure 'cloned HEAD is detached' ' +test_expect_success 'cloned HEAD is detached' ' head_is_detached detached-tag ' From 59a5775770e3e491b5af479b3bf72fb13ddd3dbf Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 7 Jun 2011 19:03:03 -0400 Subject: [PATCH 3/4] make copy_ref globally available This is a useful function, and we have already made the similar alloc_ref and copy_ref_list available. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- remote.c | 2 +- remote.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/remote.c b/remote.c index f073b1ecf5..b8ecfa5d95 100644 --- a/remote.c +++ b/remote.c @@ -896,7 +896,7 @@ struct ref *alloc_ref(const char *name) return alloc_ref_with_prefix("", 0, name); } -static struct ref *copy_ref(const struct ref *ref) +struct ref *copy_ref(const struct ref *ref) { struct ref *cpy; size_t len; diff --git a/remote.h b/remote.h index 888d7c15de..9a30a9dba6 100644 --- a/remote.h +++ b/remote.h @@ -70,7 +70,7 @@ struct refspec { extern const struct refspec *tag_refspec; struct ref *alloc_ref(const char *name); - +struct ref *copy_ref(const struct ref *ref); struct ref *copy_ref_list(const struct ref *ref); int check_ref_type(const struct ref *ref, int flags); From c1921c184c464e313308e4ef58e28ca78a5b2127 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 7 Jun 2011 19:03:22 -0400 Subject: [PATCH 4/4] clone: always fetch remote HEAD In most cases, fetching the remote HEAD explicitly is unnecessary. It's just a symref pointing to a branch which we are already fetching, so we will already ask for its sha1. However, if the remote has a detached HEAD, things are less certain. We do not ask for HEAD's sha1, but we do try to write it into a local detached HEAD. In most cases this is fine, as the remote HEAD is pointing to some part of the history graph that we will fetch via the refs. But if the remote HEAD points to an "orphan" commit (one which was is not an ancestor of any refs), then we will not have the object, and update_ref will complain when we try to write the detached HEAD, aborting the whole clone. This patch makes clone always explicitly ask the remote for the sha1 of its HEAD commit. In the non-detached case, this is a no-op, as we were going to ask for that sha1 anyway. In the regular detached case, this will add an extra "want" to the protocol negotiation, but will not change the history that gets sent. And in the detached orphan case, we will fetch the orphaned history so that we can write it into our local detached HEAD. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/clone.c | 10 +++++++--- t/t5707-clone-detached.sh | 6 +++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/builtin/clone.c b/builtin/clone.c index 2ee1fa9846..20496bdad0 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -340,8 +340,9 @@ static void remove_junk_on_signal(int signo) static struct ref *wanted_peer_refs(const struct ref *refs, struct refspec *refspec) { - struct ref *local_refs = NULL; - struct ref **tail = &local_refs; + struct ref *head = copy_ref(find_ref_by_name(refs, "HEAD")); + struct ref *local_refs = head; + struct ref **tail = head ? &head->next : &local_refs; get_fetch_map(refs, refspec, &tail, 0); if (!option_mirror) @@ -354,8 +355,11 @@ static void write_remote_refs(const struct ref *local_refs) { const struct ref *r; - for (r = local_refs; r; r = r->next) + for (r = local_refs; r; r = r->next) { + if (!r->peer_ref) + continue; add_extra_ref(r->peer_ref->name, r->old_sha1, 0); + } pack_refs(PACK_REFS_ALL); clear_extra_refs(); diff --git a/t/t5707-clone-detached.sh b/t/t5707-clone-detached.sh index d63b1e390e..8b0d607df1 100755 --- a/t/t5707-clone-detached.sh +++ b/t/t5707-clone-detached.sh @@ -58,18 +58,18 @@ test_expect_success 'cloned HEAD is detached' ' head_is_detached detached-history ' -test_expect_failure 'clone repo (orphan detached HEAD)' ' +test_expect_success 'clone repo (orphan detached HEAD)' ' git checkout master^0 && echo four >file && git commit -a -m four && git clone "file://$PWD" detached-orphan ' -test_expect_failure 'cloned HEAD matches' ' +test_expect_success 'cloned HEAD matches' ' echo four >expect && git --git-dir=detached-orphan/.git log -1 --format=%s >actual && test_cmp expect actual ' -test_expect_failure 'cloned HEAD is detached' ' +test_expect_success 'cloned HEAD is detached' ' head_is_detached detached-orphan '