Re: [PATCH] t1410: fix breakage on case-insensitive filesystems

[Jeff King <peff@peff.net>]

On Wed, Nov 12, 2014 at 09:20:22PM +0100, Johannes Sixt wrote:

> Am 09.11.2014 um 02:59 schrieb Jeff King:
> >  test_expect_success 'stale dirs do not cause d/f conflicts (reflogs off)' '
> > -	test_when_finished "git branch -d a || git branch -d a/b" &&
> > +	test_when_finished "git branch -d one || git branch -d one/two" &&
> >  
> > -	git branch a/b master &&
> > -	echo "a/b@{0} branch: Created from master" >expect &&
> > -	git log -g --format="%gd %gs" a/b >actual &&
> > +	git branch one/two master &&
> > +	echo "one/two@{0} branch: Created from master" >expect &&
> > +	git log -g --format="%gd %gs" one/two >actual &&
> >  	test_cmp expect actual &&
> > -	git branch -d a/b &&
> > +	git branch -d one/two &&
> >  
> > -	# same as before, but we only create a reflog for "a" if
> > +	# same as before, but we only create a reflog for "one" if
> >  	# it already exists, which it does not
> > -	git -c core.logallrefupdates=false branch a master &&
> > +	git -c core.logallrefupdates=false branch one master &&
> >  	: >expect &&
> > -	git log -g --format="%gd %gs" a >actual &&
> > +	git log -g --format="%gd %gs" one >actual &&
> >  	test_cmp expect actual
> >  '
> >  
> 
> On Linux I observe
> 
> Deleted branch one/two (was b60a214).
> warning: unable to unlink .git/logs/refs/heads/one: Is a directory
> Deleted branch one (was b60a214).
> ok 15 - stale dirs do not cause d/f conflicts (reflogs off)
> 
> (notice the warning)

Yes, this is expected. The warning actually comes from the "git branch
-d" run during cleanup. Branch "one" exists but has no reflog. Instead
there is a crufty dir call "logs/refs/heads/one". The deletion process
blindly calls "unlink" on it and complains when it fails for reasons
other than ENOENT.

We could suppress that warning, but I didn't think it was really worth
the bother. This is such a funny setup (reflogs on, then off, then on,
_and_ a d/f conflict in the branches) that I doubt it will come up much.

I seem to recall that ancient versions of SunOS used to do bad things
when you called "unlink" on a directory, but I do not know if that is
still the case (I also would doubt this is the only place in git where
we blindly do an "unlink if you can" without actually checking the file.

> but on Windows the test fails with
> 
> Deleted branch one/two (was b60a214).
> error: Unable to append to .git/logs/refs/heads/one: Permission denied
> fatal: Cannot update the ref 'refs/heads/one'.
> not ok 15 - stale dirs do not cause d/f conflicts (reflogs off)

That looks more like it is failing the actual test (i.e., the creation
of branch "one" when there is cruft in the reflog). My guess is that
calling open() on a directory is giving us EACCES instead of EISDIR. Can
you verify that?

If that is the case, then this isn't a new breakage, I think, but just
code we weren't previously exercising. It would be interesting to know
whether:

  git config core.logallrefupdates true
  git branch one/two
  git branch -d one/two
  git branch one

works (even without my patch). If so, then there's probably something
else going on.

The relevant bits are in log_ref_setup. We try to open() once, and
accept EISDIR as a hint that we may need to remove an empty directory
and try again. If Windows gives us EACCES, then we may need to have that
trigger the same code.

-Peff