Jeff King <peff@xxxxxxxx> writes:
> diff --git a/builtin/clone.c b/builtin/clone.c
> index f579794..a99f1c8 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -343,8 +343,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 = get_remote_ref(refs, "HEAD");
The rest of the patch looked quite sane but I wonder if this should be
using get_remote_ref() that calls find_ref_by_name_abbrev() which in turn
would hit "refs/heads/HEAD" if the remote side didn't give you "HEAD".
Shouldn't it be using find_ref_by_name() directly?
> @@ -357,8 +358,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;
As this is part of this patch, I presume this test reliably catch "HEAD"
and only "HEAD", but what is it that gives us this guarantee? Is it that
in all three possible configurations (i.e. traditional no-separate remote
layout, separate remote layout, or mirrored layout), we never map anything
outside refs/heads/* and refs/tags/* namespace, hence things like HEAD
will never have peer_ref defined?
This is not a complaint but is an honest question. I am wondering how
future possible enhancements to "clone" (like the rumored "track only this
branch") will affect codepaths around this area.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html