[PATCH] gc: save log from daemonized gc --auto and print it next time

[PATCH] gc: save log from daemonized gc --auto and print it next time

[Nguyễn Thái Ngọc Duy]

While commit 9f673f9 (gc: config option for running --auto in
background - 2014-02-08) helps reduce some complaints about 'gc
--auto' hogging the terminal, it creates another set of problems.

The latest in this set is, as the result of daemonizing, stderr is
closed and all warnings are lost. This warning at the end of cmd_gc()
is particularly important because it tells the user how to avoid "gc
--auto" running repeatedly. Because stderr is closed, the user does
not know, naturally they complain about 'gc --auto' wasting CPU.

Besides reverting 9f673f9 and looking at the problem from another
angle, we could save the stderr in $GIT_DIR/gc.log. Next time, 'gc
--auto' will print the saved warnings, delete gc.log and exit.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/gc.c  | 37 ++++++++++++++++++++++++++++++++++++-
 t/t6500-gc.sh | 20 ++++++++++++++++++++
 2 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index 5c634af..07769a9 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -32,6 +32,8 @@ static int aggressive_window = 250;
 static int gc_auto_threshold = 6700;
 static int gc_auto_pack_limit = 50;
 static int detach_auto = 1;
+static struct strbuf log_filename = STRBUF_INIT;
+static int daemonized;
 static const char *prune_expire = "2.weeks.ago";
 
 static struct argv_array pack_refs_cmd = ARGV_ARRAY_INIT;
@@ -44,6 +46,15 @@ static char *pidfile;
 
 static void remove_pidfile(void)
 {
+	if (daemonized && log_filename.len) {
+		struct stat st;
+
+		close(2);
+		if (stat(log_filename.buf, &st) ||
+		    !st.st_size ||
+		    rename(log_filename.buf, git_path("gc.log")))
+			unlink(log_filename.buf);
+	}
 	if (pidfile)
 		unlink(pidfile);
 }
@@ -324,13 +335,25 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 			fprintf(stderr, _("See \"git help gc\" for manual housekeeping.\n"));
 		}
 		if (detach_auto) {
+			struct strbuf sb = STRBUF_INIT;
+
+			if (strbuf_read_file(&sb, git_path("gc.log"), 0) > 0) {
+				warning(_("Last gc run reported the following, gc skipped"));
+				fputs(sb.buf, stderr);
+				strbuf_release(&sb);
+				/* let the next gc --auto run as usual */
+				unlink(git_path("gc.log"));
+				return 0;
+			}
+
 			if (gc_before_repack())
 				return -1;
 			/*
 			 * failure to daemonize is ok, we'll continue
 			 * in foreground
 			 */
-			daemonize();
+			if (!daemonize())
+				daemonized = 1;
 		}
 	} else
 		add_repack_all_option();
@@ -343,6 +366,18 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 		    name, (uintmax_t)pid);
 	}
 
+	if (daemonized) {
+		int fd;
+
+		strbuf_addstr(&log_filename, git_path("gc.log_XXXXXX"));
+		fd = xmkstemp(log_filename.buf);
+		if (fd >= 0) {
+			dup2(fd, 2);
+			close(fd);
+		} else
+			strbuf_release(&log_filename);
+	}
+
 	if (gc_before_repack())
 		return -1;
 
diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
index 63194d8..54bc9c4 100755
--- a/t/t6500-gc.sh
+++ b/t/t6500-gc.sh
@@ -30,4 +30,24 @@ test_expect_success 'gc -h with invalid configuration' '
 	test_i18ngrep "[Uu]sage" broken/usage
 '
 
+test_expect_success !MINGW 'gc --auto and logging' '
+	git init abc &&
+	(
+		cd abc &&
+		# These create blobs starting with the magic number "17"
+		for i in 901 944; do
+			echo $i >test && git hash-object -w test >/dev/null
+		done &&
+		git config gc.auto 1 &&
+		LANG=C git gc --auto &&
+		sleep 1 && # give it time to daemonize
+		while test -f .git/gc.pid; do sleep 1; done &&
+		grep "too many unreachable loose objects" .git/gc.log &&
+		LANG=C git gc --auto 2>error &&
+		grep skipped error &&
+		grep "too many unreachable loose objects" error &&
+		! test -f .git/gc.log
+	)
+'
+
 test_done
-- 
2.3.0.rc1.137.g477eb31

Re: [PATCH] gc: save log from daemonized gc --auto and print it next time

[Junio C Hamano]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> While commit 9f673f9 (gc: config option for running --auto in
> background - 2014-02-08) helps reduce some complaints about 'gc
> --auto' hogging the terminal, it creates another set of problems.
>
> The latest in this set is, as the result of daemonizing, stderr is
> closed and all warnings are lost. This warning at the end of cmd_gc()
> is particularly important because it tells the user how to avoid "gc
> --auto" running repeatedly. Because stderr is closed, the user does
> not know, naturally they complain about 'gc --auto' wasting CPU.
>
> Besides reverting 9f673f9 and looking at the problem from another
> angle, we could save the stderr in $GIT_DIR/gc.log. Next time, 'gc
> --auto' will print the saved warnings, delete gc.log and exit.

I wonder what this buys us if multiple auto-gc's are triggered
because the user is running a long svn import or something similar.

> diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
> index 63194d8..54bc9c4 100755
> --- a/t/t6500-gc.sh
> +++ b/t/t6500-gc.sh
> @@ -30,4 +30,24 @@ test_expect_success 'gc -h with invalid configuration' '
>  	test_i18ngrep "[Uu]sage" broken/usage
>  '
>  
> +test_expect_success !MINGW 'gc --auto and logging' '
> +	git init abc &&
> +	(
> +		cd abc &&
> +		# These create blobs starting with the magic number "17"
> +		for i in 901 944; do

There are numbers smaller than these, like 263 and 410 ;-)

> +			echo $i >test && git hash-object -w test >/dev/null

"hash-object --stdin"?

> +		done &&
> +		git config gc.auto 1 &&

test_config?

> +		LANG=C git gc --auto &&
> +		sleep 1 && # give it time to daemonize
> +		while test -f .git/gc.pid; do sleep 1; done &&

Yuck...

> +		grep "too many unreachable loose objects" .git/gc.log &&
> +		LANG=C git gc --auto 2>error &&
> +		grep skipped error &&
> +		grep "too many unreachable loose objects" error &&
> +		! test -f .git/gc.log
> +	)
> +'

For that "17/ has very many loose objects that are still young and
unreachable" issue, I wonder if the right solution is somehow to
flag the repository and prevent "gc --auto" from running until the
situation improves.  "I checked at this time and found too many in
17/"; upon finding that flag file (with a timestamp), if there are
new files in 17/ or if there are other reasons to do a gc (perhaps
there are too many packfiles to be consolidated?), then do the gc
but otherwise quite silently before spending too many cycles on it,
or something along that line?

Re: [PATCH] gc: save log from daemonized gc --auto and print it next time

[Eric Sunshine]

On Tue, Mar 24, 2015 at 8:17 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> While commit 9f673f9 (gc: config option for running --auto in
> background - 2014-02-08) helps reduce some complaints about 'gc
> --auto' hogging the terminal, it creates another set of problems.
>
> The latest in this set is, as the result of daemonizing, stderr is
> closed and all warnings are lost. This warning at the end of cmd_gc()
> is particularly important because it tells the user how to avoid "gc
> --auto" running repeatedly. Because stderr is closed, the user does
> not know, naturally they complain about 'gc --auto' wasting CPU.
>
> Besides reverting 9f673f9 and looking at the problem from another
> angle, we could save the stderr in $GIT_DIR/gc.log. Next time, 'gc
> --auto' will print the saved warnings, delete gc.log and exit.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  builtin/gc.c  | 37 ++++++++++++++++++++++++++++++++++++-
>  t/t6500-gc.sh | 20 ++++++++++++++++++++
>  2 files changed, 56 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/gc.c b/builtin/gc.c
> index 5c634af..07769a9 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -32,6 +32,8 @@ static int aggressive_window = 250;
>  static int gc_auto_threshold = 6700;
>  static int gc_auto_pack_limit = 50;
>  static int detach_auto = 1;
> +static struct strbuf log_filename = STRBUF_INIT;
> +static int daemonized;
>  static const char *prune_expire = "2.weeks.ago";
>
>  static struct argv_array pack_refs_cmd = ARGV_ARRAY_INIT;
> @@ -44,6 +46,15 @@ static char *pidfile;
>
>  static void remove_pidfile(void)
>  {
> +       if (daemonized && log_filename.len) {
> +               struct stat st;
> +
> +               close(2);
> +               if (stat(log_filename.buf, &st) ||
> +                   !st.st_size ||
> +                   rename(log_filename.buf, git_path("gc.log")))
> +                       unlink(log_filename.buf);
> +       }
>         if (pidfile)
>                 unlink(pidfile);
>  }
> @@ -324,13 +335,25 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
>                         fprintf(stderr, _("See \"git help gc\" for manual housekeeping.\n"));
>                 }
>                 if (detach_auto) {
> +                       struct strbuf sb = STRBUF_INIT;
> +
> +                       if (strbuf_read_file(&sb, git_path("gc.log"), 0) > 0) {
> +                               warning(_("Last gc run reported the following, gc skipped"));

When I read this message, it makes me think that the previous gc was
skipped, even though it's actually skipping the current one. Perhaps
rephrase as "skipping gc; last gc reported:"?

> +                               fputs(sb.buf, stderr);
> +                               strbuf_release(&sb);
> +                               /* let the next gc --auto run as usual */
> +                               unlink(git_path("gc.log"));
> +                               return 0;
> +                       }
> +
>                         if (gc_before_repack())
>                                 return -1;
>                         /*
>                          * failure to daemonize is ok, we'll continue
>                          * in foreground
>                          */
> -                       daemonize();
> +                       if (!daemonize())
> +                               daemonized = 1;
>                 }
>         } else
>                 add_repack_all_option();
> @@ -343,6 +366,18 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
>                     name, (uintmax_t)pid);
>         }
>
> +       if (daemonized) {
> +               int fd;
> +
> +               strbuf_addstr(&log_filename, git_path("gc.log_XXXXXX"));
> +               fd = xmkstemp(log_filename.buf);
> +               if (fd >= 0) {
> +                       dup2(fd, 2);
> +                       close(fd);
> +               } else
> +                       strbuf_release(&log_filename);
> +       }
> +
>         if (gc_before_repack())
>                 return -1;
>
> diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
> index 63194d8..54bc9c4 100755
> --- a/t/t6500-gc.sh
> +++ b/t/t6500-gc.sh
> @@ -30,4 +30,24 @@ test_expect_success 'gc -h with invalid configuration' '
>         test_i18ngrep "[Uu]sage" broken/usage
>  '
>
> +test_expect_success !MINGW 'gc --auto and logging' '
> +       git init abc &&
> +       (
> +               cd abc &&
> +               # These create blobs starting with the magic number "17"
> +               for i in 901 944; do
> +                       echo $i >test && git hash-object -w test >/dev/null
> +               done &&
> +               git config gc.auto 1 &&
> +               LANG=C git gc --auto &&
> +               sleep 1 && # give it time to daemonize
> +               while test -f .git/gc.pid; do sleep 1; done &&
> +               grep "too many unreachable loose objects" .git/gc.log &&
> +               LANG=C git gc --auto 2>error &&
> +               grep skipped error &&
> +               grep "too many unreachable loose objects" error &&
> +               ! test -f .git/gc.log
> +       )
> +'
> +
>  test_done
> --
> 2.3.0.rc1.137.g477eb31
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] gc: save log from daemonized gc --auto and print it next time

[Duy Nguyen]

On Wed, Mar 25, 2015 at 5:07 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> +             LANG=C git gc --auto &&
>> +             sleep 1 && # give it time to daemonize
>> +             while test -f .git/gc.pid; do sleep 1; done &&
>
> Yuck...

Yeah.. it's hard to test daemon things. I'm not even sure if we should
add a test, but I tried anyway.

>> +             grep "too many unreachable loose objects" .git/gc.log &&
>> +             LANG=C git gc --auto 2>error &&
>> +             grep skipped error &&
>> +             grep "too many unreachable loose objects" error &&
>> +             ! test -f .git/gc.log
>> +     )
>> +'
>
> For that "17/ has very many loose objects that are still young and
> unreachable" issue, I wonder if the right solution is somehow to
> flag the repository and prevent "gc --auto" from running until the
> situation improves.  "I checked at this time and found too many in
> 17/"; upon finding that flag file (with a timestamp), if there are
> new files in 17/ or if there are other reasons to do a gc (perhaps
> there are too many packfiles to be consolidated?), then do the gc
> but otherwise quit silently before spending too many cycles on it,
> or something along that line?

That's a separate problem that's being discussed in another thread. I
think Jeff's idea of storing the number of estimated loose objects may
be more reliable than timestamps..
-- 
Duy

[PATCH v2] gc: save log from daemonized gc --auto and print it next time

[Nguyễn Thái Ngọc Duy]

While commit 9f673f9 (gc: config option for running --auto in
background - 2014-02-08) helps reduce some complaints about 'gc
--auto' hogging the terminal, it creates another set of problems.

The latest in this set is, as the result of daemonizing, stderr is
closed and all warnings are lost. This warning at the end of cmd_gc()
is particularly important because it tells the user how to avoid "gc
--auto" running repeatedly. Because stderr is closed, the user does
not know, naturally they complain about 'gc --auto' wasting CPU.

Besides reverting 9f673f9 and looking at the problem from another
angle, we could save the stderr in $GIT_DIR/gc.log. Next time, 'gc
--auto' will print the saved warnings, delete gc.log and exit.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 The test is dropped. It does not feel good enough.. The rest of
 changes is minor
 
  diff --git a/builtin/gc.c b/builtin/gc.c
  index 07769a9..3886937 100644
  --- a/builtin/gc.c
  +++ b/builtin/gc.c
  @@ -32,8 +32,6 @@ static int aggressive_window = 250;
   static int gc_auto_threshold = 6700;
   static int gc_auto_pack_limit = 50;
   static int detach_auto = 1;
  -static struct strbuf log_filename = STRBUF_INIT;
  -static int daemonized;
   static const char *prune_expire = "2.weeks.ago";
   
   static struct argv_array pack_refs_cmd = ARGV_ARRAY_INIT;
  @@ -43,6 +41,8 @@ static struct argv_array prune = ARGV_ARRAY_INIT;
   static struct argv_array rerere = ARGV_ARRAY_INIT;
   
   static char *pidfile;
  +static struct strbuf log_filename = STRBUF_INIT;
  +static int daemonized;
   
   static void remove_pidfile(void)
   {
  @@ -338,7 +338,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
   			struct strbuf sb = STRBUF_INIT;
   
   			if (strbuf_read_file(&sb, git_path("gc.log"), 0) > 0) {
  -				warning(_("Last gc run reported the following, gc skipped"));
  +				warning(_("skipping gc; last gc reported:"));
   				fputs(sb.buf, stderr);
   				strbuf_release(&sb);
   				/* let the next gc --auto run as usual */
  @@ -352,8 +352,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
   			 * failure to daemonize is ok, we'll continue
   			 * in foreground
   			 */
  -			if (!daemonize())
  -				daemonized = 1;
  +			daemonized = !daemonize();
   		}
   	} else
   		add_repack_all_option();

 builtin/gc.c | 36 +++++++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index 5c634af..3886937 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -41,9 +41,20 @@ static struct argv_array prune = ARGV_ARRAY_INIT;
 static struct argv_array rerere = ARGV_ARRAY_INIT;
 
 static char *pidfile;
+static struct strbuf log_filename = STRBUF_INIT;
+static int daemonized;
 
 static void remove_pidfile(void)
 {
+	if (daemonized && log_filename.len) {
+		struct stat st;
+
+		close(2);
+		if (stat(log_filename.buf, &st) ||
+		    !st.st_size ||
+		    rename(log_filename.buf, git_path("gc.log")))
+			unlink(log_filename.buf);
+	}
 	if (pidfile)
 		unlink(pidfile);
 }
@@ -324,13 +335,24 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 			fprintf(stderr, _("See \"git help gc\" for manual housekeeping.\n"));
 		}
 		if (detach_auto) {
+			struct strbuf sb = STRBUF_INIT;
+
+			if (strbuf_read_file(&sb, git_path("gc.log"), 0) > 0) {
+				warning(_("skipping gc; last gc reported:"));
+				fputs(sb.buf, stderr);
+				strbuf_release(&sb);
+				/* let the next gc --auto run as usual */
+				unlink(git_path("gc.log"));
+				return 0;
+			}
+
 			if (gc_before_repack())
 				return -1;
 			/*
 			 * failure to daemonize is ok, we'll continue
 			 * in foreground
 			 */
-			daemonize();
+			daemonized = !daemonize();
 		}
 	} else
 		add_repack_all_option();
@@ -343,6 +365,18 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 		    name, (uintmax_t)pid);
 	}
 
+	if (daemonized) {
+		int fd;
+
+		strbuf_addstr(&log_filename, git_path("gc.log_XXXXXX"));
+		fd = xmkstemp(log_filename.buf);
+		if (fd >= 0) {
+			dup2(fd, 2);
+			close(fd);
+		} else
+			strbuf_release(&log_filename);
+	}
+
 	if (gc_before_repack())
 		return -1;
 
-- 
2.3.0.rc1.137.g477eb31

Re: [PATCH v2] gc: save log from daemonized gc --auto and print it next time

[Junio C Hamano]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> While commit 9f673f9 (gc: config option for running --auto in
> background - 2014-02-08) helps reduce some complaints about 'gc
> --auto' hogging the terminal, it creates another set of problems.
>
> The latest in this set is, as the result of daemonizing, stderr is
> closed and all warnings are lost. This warning at the end of cmd_gc()
> is particularly important because it tells the user how to avoid "gc
> --auto" running repeatedly. Because stderr is closed, the user does
> not know, naturally they complain about 'gc --auto' wasting CPU.
>
> Besides reverting 9f673f9 and looking at the problem from another
> angle, we could save the stderr in $GIT_DIR/gc.log. Next time, 'gc
> --auto' will print the saved warnings, delete gc.log and exit.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  The test is dropped. It does not feel good enough.. The rest of
>  changes is minor

I still wonder what this buys us if multiple auto-gc's are triggered,
because the user is running a long svn import or something similar.

I cannot tell what problem this is trying to solve.

 (1) If the error output from the previous "gc --auto" indicates a
     grave error condition that further re-running of "gc --auto" is
     pointless, why is it a good idea to blindly remove the log and
     "let the next one run as usual"?

 (2) If the problem it is trying to solve is that "gc --auto"
     sometimes gives a good suggestion but that is lost when
     daemonized, why not address that problem in a more direct way,
     e.g. introduce a new option "gc --probe-auto" that only checks
     and reports if "gc --auto" would spend actual cycles to do its
     work, and then run the daemonized version of "gc --auto" when
     necessary?  Either "gc --probe-auto" itself or the caller of
     "gc --probe-auto" can give the "you should do this to avoid
     repeated auto-gc" when this happens.

     Also the same issue I have with (1) above applies; I do not see
     much linkage with solving this issue with halving the frequency
     of running "gc --auto" which is what this patch does.

In other words, I would understand what the change is trying to
achieve if it were to always stop, instruct the user to take
corrective action, and never remove the .log file itself (naturally,
the corrective action would include "remove the .log when you are
done").  I do not necessarily agree that it would be a good change
for the overall system, but at least such a change is internally
consistent between its perception of a problem and its approach to
solve it.

I would also understand, and I suspect I would prefer it if I see
the result, if the change were to allow "gc --auto" to report
severity of its findings (i.e. "nothing wrong in your repository but
you should do X to avoid triggering me too often" vs "no point
running me again and again"), do something similar to what this
patch does and after showing the message and (1) stop without
removing the log but give instruction when the situation is grave,
or (2) show the message, remove the .log, and continue to go ahead
with "gc --auto" when the situation is *not* grave.

But the approach taken by this patch looks very confused about its
own purpose to me and I cannot quite tell what it is trying to
solve.

>   diff --git a/builtin/gc.c b/builtin/gc.c
>   index 07769a9..3886937 100644
>   --- a/builtin/gc.c
>   +++ b/builtin/gc.c
>   @@ -32,8 +32,6 @@ static int aggressive_window = 250;
>    static int gc_auto_threshold = 6700;
>    static int gc_auto_pack_limit = 50;
>    static int detach_auto = 1;
>   -static struct strbuf log_filename = STRBUF_INIT;
>   -static int daemonized;
>    static const char *prune_expire = "2.weeks.ago";
>    
>    static struct argv_array pack_refs_cmd = ARGV_ARRAY_INIT;
>   @@ -43,6 +41,8 @@ static struct argv_array prune = ARGV_ARRAY_INIT;
>    static struct argv_array rerere = ARGV_ARRAY_INIT;
>    
>    static char *pidfile;
>   +static struct strbuf log_filename = STRBUF_INIT;
>   +static int daemonized;
>    
>    static void remove_pidfile(void)
>    {
>   @@ -338,7 +338,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
>    			struct strbuf sb = STRBUF_INIT;
>    
>    			if (strbuf_read_file(&sb, git_path("gc.log"), 0) > 0) {
>   -				warning(_("Last gc run reported the following, gc skipped"));
>   +				warning(_("skipping gc; last gc reported:"));
>    				fputs(sb.buf, stderr);
>    				strbuf_release(&sb);
>    				/* let the next gc --auto run as usual */
>   @@ -352,8 +352,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
>    			 * failure to daemonize is ok, we'll continue
>    			 * in foreground
>    			 */
>   -			if (!daemonize())
>   -				daemonized = 1;
>   +			daemonized = !daemonize();
>    		}
>    	} else
>    		add_repack_all_option();
>
>  builtin/gc.c | 36 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/gc.c b/builtin/gc.c
> index 5c634af..3886937 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -41,9 +41,20 @@ static struct argv_array prune = ARGV_ARRAY_INIT;
>  static struct argv_array rerere = ARGV_ARRAY_INIT;
>  
>  static char *pidfile;
> +static struct strbuf log_filename = STRBUF_INIT;
> +static int daemonized;
>  
>  static void remove_pidfile(void)
>  {
> +	if (daemonized && log_filename.len) {
> +		struct stat st;
> +
> +		close(2);
> +		if (stat(log_filename.buf, &st) ||
> +		    !st.st_size ||
> +		    rename(log_filename.buf, git_path("gc.log")))
> +			unlink(log_filename.buf);
> +	}
>  	if (pidfile)
>  		unlink(pidfile);
>  }
> @@ -324,13 +335,24 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
>  			fprintf(stderr, _("See \"git help gc\" for manual housekeeping.\n"));
>  		}
>  		if (detach_auto) {
> +			struct strbuf sb = STRBUF_INIT;
> +
> +			if (strbuf_read_file(&sb, git_path("gc.log"), 0) > 0) {
> +				warning(_("skipping gc; last gc reported:"));
> +				fputs(sb.buf, stderr);
> +				strbuf_release(&sb);
> +				/* let the next gc --auto run as usual */
> +				unlink(git_path("gc.log"));
> +				return 0;
> +			}
> +
>  			if (gc_before_repack())
>  				return -1;
>  			/*
>  			 * failure to daemonize is ok, we'll continue
>  			 * in foreground
>  			 */
> -			daemonize();
> +			daemonized = !daemonize();
>  		}
>  	} else
>  		add_repack_all_option();
> @@ -343,6 +365,18 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
>  		    name, (uintmax_t)pid);
>  	}
>  
> +	if (daemonized) {
> +		int fd;
> +
> +		strbuf_addstr(&log_filename, git_path("gc.log_XXXXXX"));
> +		fd = xmkstemp(log_filename.buf);
> +		if (fd >= 0) {
> +			dup2(fd, 2);
> +			close(fd);
> +		} else
> +			strbuf_release(&log_filename);
> +	}
> +
>  	if (gc_before_repack())
>  		return -1;