Quoting Johannes Schindelin <Johannes.Schindelin@xxxxxx>:
Hi Gábor,
On Tue, 3 May 2016, SZEDER Gábor wrote:
Quoting SZEDER Gábor <szeder@xxxxxxxxxx>:
>Quoting Johannes Schindelin <Johannes.Schindelin@xxxxxx>:
>
> >Hi Gábor,
> >
> >On Sun, 1 May 2016, SZEDER Gábor wrote:
> >
> > >diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
> > >index 38321d19efbe..454d896390c0 100755
> > >--- a/t/t5510-fetch.sh
> > >+++ b/t/t5510-fetch.sh
> > >@@ -682,6 +682,7 @@ test_expect_success 'fetching with auto-gc does
> > >not lock up' '
> > > (
> > > cd auto-gc &&
> > > git config gc.autoPackLimit 1 &&
> > >+ git config gc.autoDetach false &&
> > > GIT_ASK_YESNO="$D/askyesno" git fetch >fetch.out 2>&1 &&
> > > ! grep "Should I try again" fetch.out
> > > )
> >
> >Sounds good to me.
>
>There is something still bothering me, though.
>
>I take this was a Windows-specific issue; deleting open files on Linux is
>no brainer. According to a comment on the original Git for Windows issue
>at github[1], 'git gc' runs in the background by default on
Windows as well.
Ok, having slept on it, it was a false alarm.
Though 'git gc --auto' claims "Auto packing the repository in background for
optimum performance." on Windows, it does in fact runs in the foreground.
Thanks for checking. I ran out of time yesterday.
'git gc --auto' first prints that message, unless gc.autoDetach is disabled,
and then calls daemonize() to go to the background. However, daemonize() is
basically a no-op on Windows, thus 'git gc' will remain in the
foreground and
the sequence I described below is impossible. Good.
Oh, right. I think this will take a lot of energy to fix: daemonize()'s
functionality is not really available, indeed. What *is* available is a
spawn() that detaches the new process.
Perhaps it would be worth updating 'git gc' to not lie about going to the
background when we can already know in advance that it won't.
Hmm. https://github.com/git/git/blob/master/builtin/gc.c#L372-L373
already looks correct (should it really know that we cannot daemonize()?
if (detach_auto)
fprintf(stderr, _("Auto packing the repository in background for
optimum performance.\n"));
else
fprintf(stderr, _("Auto packing the repository for optimum
performance.\n"));
It is only correct as far as the value of the 'gc.autoDetach' config
variable is concerned, which is enabled by default, even on Windows, where
it can't go to the background.
(should it really know that we cannot daemonize()?
What about other code paths using that function?):
daemonize() is currently only used in the 'git gc --auto' code path and in
'git daemon', which doesn't announce that it is about to go in the
background. Then there is the WIP index-helper, which will use daemonize()
as well, but it won't announce that either.
Being the sole callsite that makes such an announcement, I think it should
know or at least should check whether "daemonizing" is possible. (as
opposed to e.g. teaching daemonize() to print such messages)
Gábor
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html