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

["Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>]

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.

Daemonized gc now saves stderr to $GIT_DIR/gc.log. Following gc --auto
will not run and gc.log printed out until the user removes gc.log.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 The lock-based version has an advantage that the following gc runs
 will never see partial gc.log. But it requires some more hook at
 atexit() and maybe signal handler.
 
 This version avoids that, and gc.log can be kept even if gc is
 SIGKILL'd (unlikely because gc itself does not do anything that can
 upset the kernel), but then it's racy.

 I think I perfer the lock-based version.

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

diff --git a/builtin/gc.c b/builtin/gc.c
index bcc75d9..3d42ef7 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -43,6 +43,8 @@ static struct argv_array prune_worktrees = 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)
 {
@@ -253,6 +255,41 @@ static const char *lock_repo_for_gc(int force, pid_t* ret_pid)
 	return NULL;
 }
 
+static int report_last_gc_error(void)
+{
+	struct strbuf sb = STRBUF_INIT;
+	int ret;
+
+	if (!access(git_path("gc.pid"), R_OK))
+		/*
+		 * Either background gc is still running, or we have a stale
+		 * gc.pid.
+		 *
+		 * If the former, it's not the right time to look at gc.log
+		 * yet. We'll exit shortly after lock_repo_for_gc().
+		 *
+		 * If the latter, we'll run gc --auto one more time. But
+		 * because gc.log is appended, the old log (if exists) won't
+		 * be lost. One extra gc run is not that big a deal to avoid.
+		 */
+		return 0;
+
+	ret = strbuf_read_file(&sb, git_path("gc.log"), 0);
+	if (ret > 0)
+		return error(_("The last gc run reported the following. "
+			       "Please correct the root cause\n"
+			       "and remove %s. Automatic cleanup will not "
+			       "be performed\n"
+			       "until the file is removed.\n\n"
+			       "%s"),
+			     git_path("gc.log"), sb.buf);
+	else if (!ret)
+		/* racy, but in the worst case we waste one more gc run */
+		unlink(git_path("gc.log"));
+	strbuf_release(&sb);
+	return 0;
+}
+
 static int gc_before_repack(void)
 {
 	if (pack_refs && run_command_v_opt(pack_refs_cmd.argv, RUN_GIT_CMD))
@@ -330,13 +367,16 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 			fprintf(stderr, _("See \"git help gc\" for manual housekeeping.\n"));
 		}
 		if (detach_auto) {
+			if (report_last_gc_error())
+				return -1;
+
 			if (gc_before_repack())
 				return -1;
 			/*
 			 * failure to daemonize is ok, we'll continue
 			 * in foreground
 			 */
-			daemonize();
+			daemonized = !daemonize();
 		}
 	} else
 		add_repack_all_option();
@@ -349,6 +389,18 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 		    name, (uintmax_t)pid);
 	}
 
+	if (daemonized) {
+		int fd;
+
+		strbuf_git_path(&log_filename, "gc.log");
+		fd = open(log_filename.buf, O_WRONLY | O_CREAT | O_APPEND, 0644);
+		if (fd >= 0) {
+			dup2(fd, 2);
+			close(fd);
+		} else
+			strbuf_release(&log_filename);
+	}
+
 	if (gc_before_repack())
 		return -1;
 
@@ -376,6 +428,17 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 		warning(_("There are too many unreachable loose objects; "
 			"run 'git prune' to remove them."));
 
+	/*
+	 * Opportunistic cleanup in the good case (which most likely
+	 * produce empty gc.log), otherwise empty gc.log will be
+	 * deleted at the next auto gc run.
+	 */
+	if (daemonized && log_filename.len) {
+		struct stat st;
+
+		fflush(stderr);
+		if (!stat(log_filename.buf, &st) && !st.st_size)
+			unlink(log_filename.buf);
+	}
 	return 0;
 }
-- 
2.3.0.rc1.137.g477eb31