Re: [PATCH] fetch: Only call a new ref a "branch" if it's under refs/heads/.

From: Jeff King <peff@peff.net>
Date: Apr 13, 2012, 07:04 UTC
Message-ID: <20120413070423.GA16292@sigill.intra.peff.net>
In-Reply-To: <20120412210542.GD21018@sigill.intra.peff.net>

On Thu, Apr 12, 2012 at 05:05:42PM -0400, Jeff King wrote:

Show 15 quoted lines

> > Am 12.04.2012 22:36, schrieb Marc Branchaud:
> > > I assumed it was an optimization of some sort -- that since tags are normally
> > > only fetched when they're part of a requested branch's history (right?),
> > > there was no point in doing submodule recursion on the fetched tags since
> > > those tagged tree-ishes had already been submodule-recursed.
> > 
> > If that is the case the patch I just sent is pointless, but adding a comment
> > there explaining that would be a good thing ;-)
> 
> That is often the case, but not always (I might explicitly fetch a tag,
> or have refs/tags/ in my refspec). So I think you want to handle both to
> cover all cases. I haven't looked at the submodule recursion code, but I
> would hope that it would eliminate duplicates (so if I get a branch and
> a tag that is on the branch, we would look at the commit in question
> only once).

OK, I checked. Yes, we handle this just fine. After my 6859de4, we collect the ref tips before and after the fetch and run only a single rev-list. So processing the tags will result in just an extra interesting commit, which was either:

  1. accessible by another fetched branch, in which case it was going to
     be processed as interesting anyway

  2. not accessible, in which case we have fixed a bug. :)

So I think we should check all incoming refs, including tags.

-Peff