On Wed, Nov 14, 2018 at 02:08:48PM -0800, Stefan Beller wrote:
> On Wed, Nov 14, 2018 at 1:43 PM Martin Ågren <martin.agren@xxxxxxxxx> wrote:
> >
> > On Wed, 14 Nov 2018 at 16:26, Gaël Lhez via GitGitGadget
> > <gitgitgadget@xxxxxxxxx> wrote:
> > > However, the `.lock` file was still open and on Windows that means
> > > that it could not be deleted properly. This patch fixes that issue.
> >
> > Hmmm, doesn't the tempfile machinery remove the lock file when we die?
>
> On Windows this seems not to be the case. (Open files cannot be deleted
> as the open file is not kept by inode or similar but by the file path there?)
>
> Rewording your concern: Could the tempfile machinery be taught to
> work properly on Windows, e.g. by first closing all files and then deleting
> them afterwards?
It already tries to do so. See delete_tempfile(), or more likely in the
die() case, the remove_tempfiles() handler which is called at exit.
Are we sure this is still a problem?
I looked at the test to see if it would pass, but it is not even
checking anything about lockfiles! It just checks that we exit 1 by
returning up the callstack instead of calling die(). And of course it
would not have a problem under Linux either way. But if I run something
similar under strace, I see:
$ strace ./git bundle create foobar.bundle HEAD..HEAD
[...]
openat(AT_FDCWD, "/home/peff/compile/git/foobar.bundle.lock", O_RDWR|O_CREAT|O_EXCL|O_CLOEXEC, 0666) = 3
[...]
close(3) = 0
unlink("/home/peff/compile/git/foobar.bundle.lock") = 0
exit_group(128) = ?
which seems right.
-Peff
