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

3 messages, 2 authors, 2016-06-15 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:54

Jeff King [off-list ref] writes:
Since get_headref is useful in contexts where "$remote" is not always
prepended (see patch 3/3), I think the best solution is:
...
+	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")?

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;
	}

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

From: Jeff King <hidden>
Date: 2016-06-15 22:43:54

On Wed, Nov 28, 2007 at 02:23:13PM -0800, Junio C Hamano wrote:
quoted
+	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".
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

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

From: Jeff King <hidden>
Date: 2016-06-15 22:43:54

On Wed, Nov 28, 2007 at 05:43:13PM -0500, Jeff King wrote:
quoted
	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.
BTW, one bad thing about the new get_headref compared to the original is
that it does not distinguish very well between "ref does not exist" and
"an error occurred." Is there a lookup command that makes such a
distinction possible?

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help