Subject: Re: [PATCHv9 3/4] ref namespaces: Support remote repositories via upload-pack and receive-pack

From: Josh Triplett <[email-masked]>
Date: Wed, 6 Jul 2011 22:31:22 -0700

On Wed, Jul 06, 2011 at 11:55:36AM -0700, Junio C Hamano wrote:
> Josh Triplett <[email-masked]> writes:
> 
> > Change upload-pack and receive-pack to use the namespace-prefixed refs
> > when working with the repository, and use the unprefixed refs when
> > talking to the client, maintaining the masquerade.  This allows
> > clone, pull, fetch, and push to work with a suitably configured
> > GIT_NAMESPACE.
> >
> > With appropriate configuration, this also allows http-backend to expose
> > namespaces as multiple repositories with different paths.  This only
> > requires setting GIT_NAMESPACE, which http-backend passes through to
> > upload-pack and receive-pack.
> >
> > Signed-off-by: Josh Triplett <[email-masked]>
> > Signed-off-by: Jamey Sharp <[email-masked]>
> > ---
> >  builtin/receive-pack.c |   37 +++++++++++++++++++++++++++++++------
> >  upload-pack.c          |   15 ++++++++-------
> >  2 files changed, 39 insertions(+), 13 deletions(-)
> >
> > diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> > index e1a687a..2d36378 100644
> > --- a/builtin/receive-pack.c
> > +++ b/builtin/receive-pack.c
> > @@ -120,9 +120,17 @@ static int show_ref(const char *path, const unsigned char *sha1, int flag, void
> >  	return 0;
> >  }
> >  
> > +static int show_ref_cb(const char *path, const unsigned char *sha1, int flag, void *cb_data)
> > +{
> > +	path = strip_namespace(path);
> > +	if (!path)
> > +		path = ".have";
> > +	return show_ref(path, sha1, flag, cb_data);
> 
> At the first glance, this felt somewhat unoptimal as it forbids us from
> stuffing fake "ref" entries other than ".have" via the add_extra_ref()
> interface, and I wondered if it would make sense to do something like this
> instead:
> 
> 	const char *ns_path = strip_namespace(path);
>         if (!ns_path)
>         	ns_path = path;
> 	return show_ref(ns_path, sha1, flag, cb_data);
> 
> But that is flawed, and I think your patch does the right thing.  The
> justification is a bit subtle [*1*] and I think it needs to be explained
> in a comment around here, not just in the 0/4 cover letter message.

Will do.  How does the following coment sound?

/* We advertise refs outside our current namespace as .have refs, so
 * that the client can use them to minimize data transfer but will
 * otherwise ignore them. */

- Josh Triplett