Re: [PATCH 2/3] cvsimport: use show-ref to support packed refs

From: Jeff King <peff@peff.net>
Date: Nov 28, 2007, 22:43 UTC
Message-ID: <20071128224313.GB13298@coredump.intra.peff.net>
In-Reply-To: <7v4pf63t7y.fsf@gitster.siamese.dyndns.org>

On Wed, Nov 28, 2007 at 02:23:13PM -0800, Junio C Hamano wrote:

Show 8 quoted lines

> > +	if ($branch eq $opt_o && !$index{branch} &&
> > +		!get_headref("$remote/$branch")) {
> > +	my $parent = get_headref("$remote/$last_branch");
> > +		if (my $sha1 = get_headref("$remote/$mparent")) {
> 
> So the definition of get_headref() is to always take everything under
> but not including "refs/"?  IOW, the tip of the master branch is asked
> with get_headref("heads/master")?

No, the $remote variable is "refs/remotes/origin" or "refs/heads". See the old version of get_headref, which actually looked up "$git_dir/$remote/$branch".

Show 11 quoted lines

> I think show-ref can easily be confused by its tail matching behaviour,
> and is a bad command to use in a context like this.  To be safe, I think
> get_headref() should be:
> 
> 	sub get_headref {
>         	my ($it) = (@_);
>                 my $r = `git-rev-parse --verify "refs/$it"`;
> 		return undef unless $? == 0;
> 		chomp $r;
> 		return $r;
> 	}

I have no comment on which is the best command to use (you would know much better than I), but adding "refs/" is wrong.

-Peff