Re: [PATCH] t1410: fix breakage on case-insensitive filesystems
- From
Jeff King <peff@peff.net>
- Date
- Nov 12, 2014, 21:59 UTC
- Message-ID
- <20141112215923.GB6801@peff.net>
- In-Reply-To
- <5463C106.5090803@kdbg.org>
On Wed, Nov 12, 2014 at 09:20:22PM +0100, Johannes Sixt wrote:
Show 35 quoted lines
> 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.
Show 6 quoted lines
> 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