Re: [RFC PATCH] gc --auto: don't lie about running in background on Windows
From: Junio C Hamano <hidden>
Date: 2016-06-16 02:19:11
SZEDER Gábor [off-list ref] writes:
Arguably this helper function could be just a simple variable. I
opted for a function because:
- I preferred a single '#ifdef NO_POSIX_GOODIES', and putting a
static variable so near to EOF felt just wrong. (And this is why
it's not an inline-able function defined in a header file.)
- currently we know already at compile time that Windows can't
daemonize, but in the future we might want to extend this helper
function to perform some runtime checks, too. But this is perhaps
like preparing for crossing a bridge where we'll never get to.
Alternatively, the implementation of daemonize() and can_daemonize()
can live in compat/ and have the #ifdef switch in git-compat-util.h,
e.g. something along the lines of these:
<< git-compat-util.h >>
... after conditional inclusion of compat/mingw.h ...
#ifndef can_daemonize
#define can_daemonize() 1
#endif
<< compat/mingw.h >>
#define can_daemonize() 0
#define daemonize() mingw_daemonize()
<< setup.c >>
...
#ifndef NO_POSIX_GOODIES
int daemonize(void)
{
... no ifdef around here ...
}
#endif
We can be even more purist and move the daemonize() implementation
for POSIX to compat/posix.c to keep the generic part even more
platform agnostic, which would remove the only #ifdef in setup.c,
but that might be going a bit too far.
These possible implementation choices aside, the goal of this patch
is a worthwhile thing to do, I would think.
Thanks.
quoted hunk
builtin/gc.c | 1 + cache.h | 1 + setup.c | 17 +++++++++++++++-- 3 files changed, 17 insertions(+), 2 deletions(-)diff --git a/builtin/gc.c b/builtin/gc.c index c583aad6ec28..79a0f6dc1126 100644 --- a/builtin/gc.c +++ b/builtin/gc.c@@ -368,6 +368,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix) */ if (!need_to_gc()) return 0; + detach_auto &= can_daemonize(); if (!quiet) { if (detach_auto) fprintf(stderr, _("Auto packing the repository in background for optimum performance.\n"));diff --git a/cache.h b/cache.h index fd728f079320..3662e5aabb98 100644 --- a/cache.h +++ b/cache.h@@ -524,6 +524,7 @@ extern int set_git_dir_init(const char *git_dir, const char *real_git_dir, int); extern int init_db(const char *template_dir, unsigned int flags); extern void sanitize_stdfds(void); +extern int can_daemonize(void); extern int daemonize(void); #define alloc_nr(x) (((x)+16)*3/2)diff --git a/setup.c b/setup.c index c86bf5c9fabe..6187a4ad9c47 100644 --- a/setup.c +++ b/setup.c@@ -1033,12 +1033,25 @@ void sanitize_stdfds(void) close(fd); } +#ifdef NO_POSIX_GOODIES +int can_daemonize(void) +{ + return 0; +} + int daemonize(void) { -#ifdef NO_POSIX_GOODIES errno = ENOSYS; return -1; +} #else +int can_daemonize(void) +{ + return 1; +} + +int daemonize(void) +{ switch (fork()) { case 0: break;@@ -1054,5 +1067,5 @@ int daemonize(void) close(2); sanitize_stdfds(); return 0; -#endif } +#endif /* #ifdef NO_POSIX_GOODIES */