Merge branch 'nd/clone-detached'

* nd/clone-detached:
  clone: fix up delay cloning conditions
  push: do not let configured foreign-vcs permanently clobbered
  clone: print advice on checking out detached HEAD
  clone: allow --branch to take a tag
  clone: refuse to clone if --branch points to bogus ref
  clone: --branch=<branch> always means refs/heads/<branch>
  clone: delay cloning until after remote HEAD checking
  clone: factor out remote ref writing
  clone: factor out HEAD update code
  clone: factor out checkout code
  clone: write detached HEAD in bare repositories
  t5601: add missing && cascade

Documentation/git-clone.txt

@@ -147,8 +147,9 @@ objects from the source repository into a pack in the cloned repository.
 -b <name>::
 	Instead of pointing the newly created HEAD to the branch pointed
 	to by the cloned repository's HEAD, point to `<name>` branch
-	instead. In a non-bare repository, this is the branch that will
+	instead. `--branch` can also take tags and treat them like
-	be checked out.
+	detached HEAD. In a non-bare repository, this is the branch
+	that will be checked out.
 
 --upload-pack <upload-pack>::
 -u <upload-pack>::

advice.c

@@ -73,3 +73,17 @@ void NORETURN die_resolve_conflict(const char *me)
 	error_resolve_conflict(me);
 	die("Exiting because of an unresolved conflict.");
 }
+
+void detach_advice(const char *new_name)
+{
+	const char fmt[] =
+	"Note: checking out '%s'.\n\n"
+	"You are in 'detached HEAD' state. You can look around, make experimental\n"
+	"changes and commit them, and you can discard any commits you make in this\n"
+	"state without impacting any branches by performing another checkout.\n\n"
+	"If you want to create a new branch to retain commits you create, you may\n"
+	"do so (now or later) by using -b with the checkout command again. Example:\n\n"
+	"  git checkout -b new_branch_name\n\n";
+
+	fprintf(stderr, fmt, new_name);
+}

advice.h

@@ -14,5 +14,6 @@ int git_default_advice_config(const char *var, const char *value);
 void advise(const char *advice, ...);
 int error_resolve_conflict(const char *me);
 extern void NORETURN die_resolve_conflict(const char *me);
+void detach_advice(const char *new_name);
 
 #endif /* ADVICE_H */

builtin/checkout.c

@@ -514,20 +514,6 @@ static void report_tracking(struct branch_info *new)
 	strbuf_release(&sb);
 }
 
-static void detach_advice(const char *old_path, const char *new_name)
-{
-	const char fmt[] =
-	"Note: checking out '%s'.\n\n"
-	"You are in 'detached HEAD' state. You can look around, make experimental\n"
-	"changes and commit them, and you can discard any commits you make in this\n"
-	"state without impacting any branches by performing another checkout.\n\n"
-	"If you want to create a new branch to retain commits you create, you may\n"
-	"do so (now or later) by using -b with the checkout command again. Example:\n\n"
-	"  git checkout -b new_branch_name\n\n";
-
-	fprintf(stderr, fmt, new_name);
-}
-
 static void update_refs_for_switch(struct checkout_opts *opts,
 				   struct branch_info *old,
 				   struct branch_info *new)
@@ -575,7 +561,7 @@ static void update_refs_for_switch(struct checkout_opts *opts,
 			   REF_NODEREF, DIE_ON_ERR);
 		if (!opts->quiet) {
 			if (old->path && advice_detached_head)
-				detach_advice(old->path, new->name);
+				detach_advice(new->name);
 			describe_detached_head(_("HEAD is now at"), new->commit);
 		}
 	} else if (new->path) {	/* Switch branches. */

builtin/clone.c

@@ -48,7 +48,6 @@ static int option_verbosity;
 static int option_progress;
 static struct string_list option_config;
 static struct string_list option_reference;
-static const char *src_ref_prefix = "refs/heads/";
 
 static int opt_parse_reference(const struct option *opt, const char *arg, int unset)
 {
@@ -364,13 +363,8 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,
 	closedir(dir);
 }
 
-static const struct ref *clone_local(const char *src_repo,
+static void clone_local(const char *src_repo, const char *dest_repo)
-				     const char *dest_repo)
 {
-	const struct ref *ret;
-	struct remote *remote;
-	struct transport *transport;
-
 	if (option_shared) {
 		struct strbuf alt = STRBUF_INIT;
 		strbuf_addf(&alt, "%s/objects", src_repo);
@@ -386,13 +380,8 @@ static const struct ref *clone_local(const char *src_repo,
 		strbuf_release(&dest);
 	}
 
-	remote = remote_get(src_repo);
-	transport = transport_get(remote, src_repo);
-	ret = transport_get_remote_refs(transport);
-	transport_disconnect(transport);
 	if (0 <= option_verbosity)
 		printf(_("done.\n"));
-	return ret;
 }
 
 static const char *junk_work_tree;
@@ -423,6 +412,26 @@ static void remove_junk_on_signal(int signo)
 	raise(signo);
 }
 
+static struct ref *find_remote_branch(const struct ref *refs, const char *branch)
+{
+	struct ref *ref;
+	struct strbuf head = STRBUF_INIT;
+	strbuf_addstr(&head, "refs/heads/");
+	strbuf_addstr(&head, branch);
+	ref = find_ref_by_name(refs, head.buf);
+	strbuf_release(&head);
+
+	if (ref)
+		return ref;
+
+	strbuf_addstr(&head, "refs/tags/");
+	strbuf_addstr(&head, branch);
+	ref = find_ref_by_name(refs, head.buf);
+	strbuf_release(&head);
+
+	return ref;
+}
+
 static struct ref *wanted_peer_refs(const struct ref *refs,
 		struct refspec *refspec)
 {
@@ -435,19 +444,18 @@ static struct ref *wanted_peer_refs(const struct ref *refs,
 
 		if (!option_branch)
 			remote_head = guess_remote_head(head, refs, 0);
-		else {
+		else
-			struct strbuf sb = STRBUF_INIT;
+			remote_head = find_remote_branch(refs, option_branch);
-			strbuf_addstr(&sb, src_ref_prefix);
-			strbuf_addstr(&sb, option_branch);
-			remote_head = find_ref_by_name(refs, sb.buf);
-			strbuf_release(&sb);
-		}
 
 		if (!remote_head && option_branch)
 			warning(_("Could not find remote branch %s to clone."),
 				option_branch);
-		else
+		else {
 			get_fetch_map(remote_head, refspec, &tail, 0);
+
+			/* if --branch=tag, pull the requested tag explicitly */
+			get_fetch_map(remote_head, tag_refspec, &tail, 0);
+		}
 	} else
 		get_fetch_map(refs, refspec, &tail, 0);
 
@@ -485,6 +493,116 @@ static void write_followtags(const struct ref *refs, const char *msg)
 	}
 }
 
+static void update_remote_refs(const struct ref *refs,
+			       const struct ref *mapped_refs,
+			       const struct ref *remote_head_points_at,
+			       const char *branch_top,
+			       const char *msg)
+{
+	if (refs) {
+		clear_extra_refs();
+		write_remote_refs(mapped_refs);
+		if (option_single_branch)
+			write_followtags(refs, msg);
+	}
+
+	if (remote_head_points_at && !option_bare) {
+		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);
+	}
+}
+
+static void update_head(const struct ref *our, const struct ref *remote,
+			const char *msg)
+{
+	if (our && !prefixcmp(our->name, "refs/heads/")) {
+		/* Local default branch link */
+		create_symref("HEAD", our->name, NULL);
+		if (!option_bare) {
+			const char *head = skip_prefix(our->name, "refs/heads/");
+			update_ref(msg, "HEAD", our->old_sha1, NULL, 0, DIE_ON_ERR);
+			install_branch_config(0, head, option_origin, our->name);
+		}
+	} else if (our) {
+		struct commit *c = lookup_commit_reference(our->old_sha1);
+		/* --branch specifies a non-branch (i.e. tags), detach HEAD */
+		update_ref(msg, "HEAD", c->object.sha1,
+			   NULL, REF_NODEREF, DIE_ON_ERR);
+	} else if (remote) {
+		/*
+		 * We know remote HEAD points to a non-branch, or
+		 * HEAD points to a branch but we don't know which one.
+		 * Detach HEAD in all these cases.
+		 */
+		update_ref(msg, "HEAD", remote->old_sha1,
+			   NULL, REF_NODEREF, DIE_ON_ERR);
+	}
+}
+
+static int checkout(void)
+{
+	unsigned char sha1[20];
+	char *head;
+	struct lock_file *lock_file;
+	struct unpack_trees_options opts;
+	struct tree *tree;
+	struct tree_desc t;
+	int err = 0, fd;
+
+	if (option_no_checkout)
+		return 0;
+
+	head = resolve_refdup("HEAD", sha1, 1, NULL);
+	if (!head) {
+		warning(_("remote HEAD refers to nonexistent ref, "
+			  "unable to checkout.\n"));
+		return 0;
+	}
+	if (!strcmp(head, "HEAD")) {
+		if (advice_detached_head)
+			detach_advice(sha1_to_hex(sha1));
+	} else {
+		if (prefixcmp(head, "refs/heads/"))
+			die(_("HEAD not found below refs/heads!"));
+	}
+	free(head);
+
+	/* We need to be in the new work tree for the checkout */
+	setup_work_tree();
+
+	lock_file = xcalloc(1, sizeof(struct lock_file));
+	fd = hold_locked_index(lock_file, 1);
+
+	memset(&opts, 0, sizeof opts);
+	opts.update = 1;
+	opts.merge = 1;
+	opts.fn = oneway_merge;
+	opts.verbose_update = (option_verbosity > 0);
+	opts.src_index = &the_index;
+	opts.dst_index = &the_index;
+
+	tree = parse_tree_indirect(sha1);
+	parse_tree(tree);
+	init_tree_desc(&t, tree->buffer, tree->size);
+	unpack_trees(1, &t, &opts);
+
+	if (write_cache(fd, active_cache, active_nr) ||
+	    commit_locked_index(lock_file))
+		die(_("unable to write new index file"));
+
+	err |= run_hook(NULL, "post-checkout", sha1_to_hex(null_sha1),
+			sha1_to_hex(sha1), "1", NULL);
+
+	if (!err && option_recursive)
+		err = run_command_v_opt(argv_submodule, RUN_GIT_CMD);
+
+	return err;
+}
+
 static int write_one_config(const char *key, const char *value, void *data)
 {
 	return git_config_set_multivar(key, value ? value : "true", "^$", 0);
@@ -512,10 +630,13 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	const struct ref *remote_head_points_at;
 	const struct ref *our_head_points_at;
 	struct ref *mapped_refs;
+	const struct ref *ref;
 	struct strbuf key = STRBUF_INIT, value = STRBUF_INIT;
 	struct strbuf branch_top = STRBUF_INIT, reflog_msg = STRBUF_INIT;
 	struct transport *transport = NULL;
-	int err = 0;
+	const char *src_ref_prefix = "refs/heads/";
+	struct remote *remote;
+	int err = 0, complete_refs_before_fetch = 1;
 
 	struct refspec *refspec;
 	const char *fetch_pattern;
@@ -669,13 +790,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 
 	strbuf_reset(&value);
 
-	if (is_local) {
+	remote = remote_get(option_origin);
-		refs = clone_local(path, git_dir);
-		mapped_refs = wanted_peer_refs(refs, refspec);
-	} else {
-		struct remote *remote = remote_get(option_origin);
 	transport = transport_get(remote, remote->url[0]);
 
+	if (!is_local) {
 		if (!transport->get_refs_list || !transport->fetch)
 			die(_("Don't know how to clone %s"), transport->url);
 
@@ -692,39 +810,42 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		if (option_upload_pack)
 			transport_set_option(transport, TRANS_OPT_UPLOADPACK,
 					     option_upload_pack);
+	}
 
 	refs = transport_get_remote_refs(transport);
-		if (refs) {
+	mapped_refs = refs ? wanted_peer_refs(refs, refspec) : NULL;
-			mapped_refs = wanted_peer_refs(refs, refspec);
+
+	/*
+	 * transport_get_remote_refs() may return refs with null sha-1
+	 * in mapped_refs (see struct transport->get_refs_list
+	 * comment). In that case we need fetch it early because
+	 * remote_head code below relies on it.
+	 *
+	 * for normal clones, transport_get_remote_refs() should
+	 * return reliable ref set, we can delay cloning until after
+	 * remote HEAD check.
+	 */
+	for (ref = refs; ref; ref = ref->next)
+		if (is_null_sha1(ref->old_sha1)) {
+			complete_refs_before_fetch = 0;
+			break;
+		}
+
+	if (!is_local && !complete_refs_before_fetch && refs)
 		transport_fetch_refs(transport, mapped_refs);
-		}
-	}
 
 	if (refs) {
-		clear_extra_refs();
-
-		write_remote_refs(mapped_refs);
-		if (option_single_branch)
-			write_followtags(refs, reflog_msg.buf);
-
 		remote_head = find_ref_by_name(refs, "HEAD");
 		remote_head_points_at =
 			guess_remote_head(remote_head, mapped_refs, 0);
 
 		if (option_branch) {
-			struct strbuf head = STRBUF_INIT;
-			strbuf_addstr(&head, src_ref_prefix);
-			strbuf_addstr(&head, option_branch);
 			our_head_points_at =
-				find_ref_by_name(mapped_refs, head.buf);
+				find_remote_branch(mapped_refs, option_branch);
-			strbuf_release(&head);
 
-			if (!our_head_points_at) {
+			if (!our_head_points_at)
-				warning(_("Remote branch %s not found in "
+				die(_("Remote branch %s not found in upstream %s"),
-					"upstream %s, using HEAD instead"),
 				    option_branch, option_origin);
-				our_head_points_at = remote_head_points_at;
-			}
 		}
 		else
 			our_head_points_at = remote_head_points_at;
@@ -740,84 +861,20 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 					      "refs/heads/master");
 	}
 
-	if (remote_head_points_at && !option_bare) {
+	if (is_local)
-		struct strbuf head_ref = STRBUF_INIT;
+		clone_local(path, git_dir);
-		strbuf_addstr(&head_ref, branch_top.buf);
+	else if (refs && complete_refs_before_fetch)
-		strbuf_addstr(&head_ref, "HEAD");
+		transport_fetch_refs(transport, mapped_refs);
-		create_symref(head_ref.buf,
-			      remote_head_points_at->peer_ref->name,
-			      reflog_msg.buf);
-	}
 
-	if (our_head_points_at) {
+	update_remote_refs(refs, mapped_refs, remote_head_points_at,
-		/* Local default branch link */
+			   branch_top.buf, reflog_msg.buf);
-		create_symref("HEAD", our_head_points_at->name, NULL);
+
-		if (!option_bare) {
+	update_head(our_head_points_at, remote_head, reflog_msg.buf);
-			const char *head = skip_prefix(our_head_points_at->name,
-						       "refs/heads/");
-			update_ref(reflog_msg.buf, "HEAD",
-				   our_head_points_at->old_sha1,
-				   NULL, 0, DIE_ON_ERR);
-			install_branch_config(0, head, option_origin,
-					      our_head_points_at->name);
-		}
-	} else if (remote_head) {
-		/* Source had detached HEAD pointing somewhere. */
-		if (!option_bare) {
-			update_ref(reflog_msg.buf, "HEAD",
-				   remote_head->old_sha1,
-				   NULL, REF_NODEREF, DIE_ON_ERR);
-			our_head_points_at = remote_head;
-		}
-	} else {
-		/* Nothing to checkout out */
-		if (!option_no_checkout)
-			warning(_("remote HEAD refers to nonexistent ref, "
-				"unable to checkout.\n"));
-		option_no_checkout = 1;
-	}
 
-	if (transport) {
 	transport_unlock_pack(transport);
 	transport_disconnect(transport);
-	}
 
-	if (!option_no_checkout) {
+	err = checkout();
-		struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
-		struct unpack_trees_options opts;
-		struct tree *tree;
-		struct tree_desc t;
-		int fd;
-
-		/* We need to be in the new work tree for the checkout */
-		setup_work_tree();
-
-		fd = hold_locked_index(lock_file, 1);
-
-		memset(&opts, 0, sizeof opts);
-		opts.update = 1;
-		opts.merge = 1;
-		opts.fn = oneway_merge;
-		opts.verbose_update = (option_verbosity > 0);
-		opts.src_index = &the_index;
-		opts.dst_index = &the_index;
-
-		tree = parse_tree_indirect(our_head_points_at->old_sha1);
-		parse_tree(tree);
-		init_tree_desc(&t, tree->buffer, tree->size);
-		unpack_trees(1, &t, &opts);
-
-		if (write_cache(fd, active_cache, active_nr) ||
-		    commit_locked_index(lock_file))
-			die(_("unable to write new index file"));
-
-		err |= run_hook(NULL, "post-checkout", sha1_to_hex(null_sha1),
-				sha1_to_hex(our_head_points_at->old_sha1), "1",
-				NULL);
-
-		if (!err && option_recursive)
-			err = run_command_v_opt(argv_submodule, RUN_GIT_CMD);
-	}
 
 	strbuf_release(&reflog_msg);
 	strbuf_release(&branch_top);

t/t5500-fetch-pack.sh

@@ -282,13 +282,6 @@ test_expect_success 'clone shallow object count' '
 	test_cmp count3.expected count3.actual
 '
 
-test_expect_success 'clone shallow with nonexistent --branch' '
-	git clone --depth 1 --branch Z "file://$(pwd)/." shallow4 &&
-	GIT_DIR=shallow4/.git git rev-parse HEAD >actual &&
-	git rev-parse HEAD >expected &&
-	test_cmp expected actual
-'
-
 test_expect_success 'clone shallow with detached HEAD' '
 	git checkout HEAD^ &&
 	git clone --depth 1 "file://$(pwd)/." shallow5 &&
@@ -318,4 +311,19 @@ EOF
 	test_cmp count6.expected count6.actual
 '
 
+test_expect_success 'shallow cloning single tag' '
+	git clone --depth 1 --branch=TAGB1 "file://$(pwd)/." shallow7 &&
+	cat >taglist.expected <<\EOF &&
+TAGB1
+TAGB2
+EOF
+	GIT_DIR=shallow7/.git git tag -l >taglist.actual &&
+	test_cmp taglist.expected taglist.actual &&
+
+	echo "in-pack: 7" > count7.expected &&
+	GIT_DIR=shallow7/.git git count-objects -v |
+		grep "^in-pack" > count7.actual &&
+	test_cmp count7.expected count7.actual
+'
+
 test_done

t/t5601-clone.sh

@@ -9,10 +9,13 @@ test_expect_success setup '
 	rm -fr .git &&
 	test_create_repo src &&
 	(
-		cd src
+		cd src &&
-		>file
+		>file &&
-		git add file
+		git add file &&
-		git commit -m initial
+		git commit -m initial &&
+		echo 1 >file &&
+		git add file &&
+		git commit -m updated
 	)
 
 '
@@ -88,6 +91,26 @@ test_expect_success 'clone --mirror' '
 
 '
 
+test_expect_success 'clone --mirror with detached HEAD' '
+
+	( cd src && git checkout HEAD^ && git rev-parse HEAD >../expected ) &&
+	git clone --mirror src mirror.detached &&
+	( cd src && git checkout - ) &&
+	GIT_DIR=mirror.detached git rev-parse HEAD >actual &&
+	test_cmp expected actual
+
+'
+
+test_expect_success 'clone --bare with detached HEAD' '
+
+	( cd src && git checkout HEAD^ && git rev-parse HEAD >../expected ) &&
+	git clone --bare src bare.detached &&
+	( cd src && git checkout - ) &&
+	GIT_DIR=bare.detached git rev-parse HEAD >actual &&
+	test_cmp expected actual
+
+'
+
 test_expect_success 'clone --bare names the local repository <name>.git' '
 
 	git clone --bare src &&
@@ -248,4 +271,13 @@ test_expect_success 'clone from original with relative alternate' '
 	grep /src/\\.git/objects target-10/objects/info/alternates
 '
 
+test_expect_success 'clone checking out a tag' '
+	git clone --branch=some-tag src dst.tag &&
+	GIT_DIR=src/.git git rev-parse some-tag >expected &&
+	test_cmp expected dst.tag/.git/HEAD &&
+	GIT_DIR=dst.tag/.git git config remote.origin.fetch >fetch.actual &&
+	echo "+refs/heads/*:refs/remotes/origin/*" >fetch.expected &&
+	test_cmp fetch.expected fetch.actual
+'
+
 test_done

t/t5706-clone-branch.sh

@@ -57,12 +57,8 @@ test_expect_success 'clone -b does not munge remotes/origin/HEAD' '
 	)
 '
 
-test_expect_success 'clone -b with bogus branch chooses HEAD' '
+test_expect_success 'clone -b with bogus branch' '
-	git clone -b bogus parent clone-bogus &&
+	test_must_fail git clone -b bogus parent clone-bogus
-	(cd clone-bogus &&
-	 check_HEAD master &&
-	 check_file one
-	)
 '
 
 test_done