Re: [PATCH, fixed] builtin-clone: create remotes/origin/HEAD symref, if guessed

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mar 3, 2008, 17:10 UTC
Message-ID: <alpine.LSU.1.00.0803031709150.22527@racer.site>
In-Reply-To: <1204563913.4084.3.camel@gaara.boston.redhat.com>

Hi,

On Mon, 3 Mar 2008, Kristian Høgsberg wrote:

Show 74 quoted lines

> On Sun, 2008-03-02 at 06:25 +0000, Johannes Schindelin wrote:
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> > 
> > 	Sorry, my previous patch was broken in so many ways.  This one
> > 	is better, promise.
> > 
> > 	BTW this incidentally fixes the branch.<branch>.{remote,merge} 
> > 	setup: it used to strip all up-to and including a slash from the 
> > 	ref name.  This just _happened_ to work, because commonly HEAD is 
> > 	at "refs/heads/master".  However, if it is at "refs/heads/a/b", it 
> > 	would fail.
> > 
> >  builtin-clone.c |   35 ++++++++++++++++++++---------------
> >  1 files changed, 20 insertions(+), 15 deletions(-)
> > 
> > diff --git a/builtin-clone.c b/builtin-clone.c
> > index 056e8a3..f27d205 100644
> > --- a/builtin-clone.c
> > +++ b/builtin-clone.c
> > @@ -523,33 +523,38 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
> >  		git_config_set_multivar(key, value, "^$", 0);
> >  	}
> >  
> > +	if (head_points_at)
> > +		/* Local default branch */
> > +		create_symref("HEAD", head_points_at->name, NULL);
> > +
> >  	if (option_bare) {
> > -		if (head_points_at) {
> > -			/* Local default branch */
> > -			create_symref("HEAD", head_points_at->name, NULL);
> > -		}
> >  		junk_work_tree = NULL;
> >  		junk_git_dir = NULL;
> >  		return 0;
> >  	}
> >  
> >  	if (head_points_at) {
> > -		if (strrchr(head_points_at->name, '/'))
> > -			head = strrchr(head_points_at->name, '/') + 1;
> > -		else
> > -			head = head_points_at->name;
> > +		struct strbuf head_ref, real_ref;
> >  
> > -		/* Local default branch */
> > -		create_symref("HEAD", head_points_at->name, NULL);
> > +		head = head_points_at->name;
> > +		if (!prefixcmp(head, "refs/heads/"))
> > +			head += 11;
> >  
> >  		/* Tracking branch for the primary branch at the remote. */
> >  		update_ref(NULL, "HEAD", head_points_at->old_sha1,
> >  			   NULL, 0, DIE_ON_ERR);
> > -	/*
> > -		rm -f "refs/remotes/$origin/HEAD"
> > -		git symbolic-ref "refs/remotes/$origin/HEAD" \
> > -			"refs/remotes/$origin/$head_points_at" &&
> > -	*/
> > +
> > +		strbuf_init(&head_ref, 0);
> > +		strbuf_addstr(&head_ref, branch_top);
> > +		strbuf_addstr(&head_ref, "/HEAD");
> > +		delete_ref(head_ref.buf, head_points_at->old_sha1);
> > +		strbuf_init(&real_ref, 0);
> > +		strbuf_addstr(&real_ref, branch_top);
> > +		strbuf_addch(&real_ref, '/');
> > +		strbuf_addstr(&real_ref, head);
> 
> What about just using
> 
>   strbuf_addf(&real_ref, "%s/%s", branch_top, head);
> 
> Are you worried about performance? :-p

You know, just after sending, I thought the same.

Show 5 quoted lines

> Oh and I'm wondering if
> 
>   strbuf_initf(&real_ref, "%s/%s", branch_top, head);
> 
> would be a worthwhile addition to the strbuf API...

And exactly this was crossing my mind, too, as

static inline int strbuf_initf(struct strbuf *buf, const char *format, ...)
{
	strbuf_init(buf, strlen(format));
	return strbuf_addf(format, ...);
}

(just a sketch, but you get the idea...)

Ciao, Dscho