Re: Regression in 'git branch -m'?

From: Jeff King <peff@peff.net>
Date: Oct 6, 2017, 10:06 UTC
Message-ID: <20171006100649.rrbsw5dqf5gionzb@sigill.intra.peff.net>
In-Reply-To: <xmqqzi94vcd7.fsf@gitster.mtv.corp.google.com>

On Fri, Oct 06, 2017 at 06:45:08PM +0900, Junio C Hamano wrote:

Show 17 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > So this patch fixes the problem:
> >
> > diff --git a/refs.c b/refs.c
> > index df075fcd06..2ba74720c8 100644
> > --- a/refs.c
> > +++ b/refs.c
> > @@ -1435,7 +1435,8 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,
> >  		if (refs_read_raw_ref(refs, refname,
> >  				      sha1, &sb_refname, &read_flags)) {
> >  			*flags |= read_flags;
> > -			if (errno != ENOENT || (resolve_flags & RESOLVE_REF_READING))
> > +			if ((errno != ENOENT && errno != EISDIR) ||
> > +			    (resolve_flags & RESOLVE_REF_READING))
> 
> Ooo, good find--is_missing_file_error() strikes back...

Almost. That uses ENOTDIR, so that looking for "foo/bar" handles the case where "foo" is a regular file.

But this is the opposite: we ask about "foo", but "foo/bar" exists. The answer isn't "it's not there" in the general case, but "it's not the thing you were expecting".

But in the case of refs, the filesystem is just a representation of the abstract namespace. In asking for "refs/heads/foo", if "refs/heads/foo/bar" exists, then answer is still "no, it's not a ref".

So EISDIR is needed for this case, though I suspect the opposite case would need ENOTDIR. I actually wonder if the files-backend read_raw_ref ought to be normalizing all of those to ENOENT.

Show 7 quoted lines

> >  				return NULL;
> >  			hashclr(sha1);
> >  			if (*flags & REF_BAD_NAME)
> >
> > but seems to stimulate a test failure in t3308. I have a suspicion that
> > I've just uncovered another bug, but I'll dig in that. In the meantime I
> > wanted to post this update in case anybody else was looking into it.

That failure indeed turned out to be a red herring. So I think I'm definitely onto the right track.

I want to play with the ENOTDIR case, and then I'll write up the whole thing and send it in later today.

-Peff