From: Johan Herland <johan@herland.net>
Date: Sun, 01 Feb 2009 02:20:28 GMT
Subject: Re: [RFC PATCH 3/3] Support fetching from foreign VCSes
Message-ID: <200902010320.28128.johan@herland.net>
In-Reply-To: <alpine.LNX.1.00.0901110335520.19665@iabervon.org>

On Sunday 11 January 2009, Daniel Barkalow wrote:
> This supports a useful subset of the usual fetch logic, mostly in the
> config file.

Hi,

I love the idea of this patch series, and have started working on a CVS 
backend for this. I have a question though...

> Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
> ---
>  builtin-fetch.c |  135
> +++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed,
> 132 insertions(+), 3 deletions(-)
>
> diff --git a/builtin-fetch.c b/builtin-fetch.c
> index 7b46f8f..14e037e 100644
> --- a/builtin-fetch.c
> +++ b/builtin-fetch.c
> @@ -614,6 +614,136 @@ static void set_option(const char *name, const char
> *value) name, transport->url);
>  }
>
> +static struct ref *list_foreign(struct remote *remote)
> +{

[...]

> +}
> +
> +static int import_foreign(struct remote *remote, struct ref *refs)
> +{

[...]

> +}
> +
> +static int fetch_foreign(struct remote *remote)
> +{
> +	struct ref *remote_refs = list_foreign(remote);

We retrieve a list of all refs available at the given remote...

> +	struct ref *ref_map = NULL;
> +	struct ref *rm;
> +	struct ref **tail = &ref_map;
> +	struct branch *branch;
> +	int i;
> +
> +	int exit_code = import_foreign(remote, remote_refs);

...and then we start fetching _all_ the refs returned by list_foreign().

When I call "git fetch <vcs-remote> <ref>", I expect _only_ <ref> to be 
fetched from the remote, but it seems the above code doesn't even concern 
itself with the ref(s) specified on the "git fetch" command-line

I'm not even sure why list_foreign() should be called in this case (except, 
maybe, to verify the existence of <ref> before attempting to fetch it).

AFAICS list_foreign() is only needed by "git fetch" if <ref> contains a 
wildcard (like the default refspec: +refs/heads/*:refs/remotes/<remote>/*).

...or have I misunderstood something fundamental about how this is going to 
work?


Have fun! :)

...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net