git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Eric Sunshine" <sunshine@sunshineco.com>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: [PATCH v5] gc: save log from daemonized gc --auto and print it next time
Date: Sat, 19 Sep 2015 12:13:23 +0700	[thread overview]
Message-ID: <1442639603-25568-1-git-send-email-pclouds@gmail.com> (raw)
In-Reply-To: <1442108202-25746-1-git-send-email-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>
---
 This is the lock-based version. I'm sending an alternative that does
 not need atexit() with comparison between the two.

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

diff --git a/builtin/gc.c b/builtin/gc.c
index bcc75d9..4e738fa 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -43,6 +43,7 @@ static struct argv_array prune_worktrees = ARGV_ARRAY_INIT;
 static struct argv_array rerere = ARGV_ARRAY_INIT;
 
 static char *pidfile;
+static struct lock_file log_lock;
 
 static void remove_pidfile(void)
 {
@@ -57,6 +58,28 @@ static void remove_pidfile_on_signal(int signo)
 	raise(signo);
 }
 
+static void process_log_file(void)
+{
+	struct stat st;
+	if (!fstat(log_lock.fd, &st) && st.st_size)
+		commit_lock_file(&log_lock);
+	else
+		rollback_lock_file(&log_lock);
+}
+
+static void process_log_file_at_exit(void)
+{
+	fflush(stderr);
+	process_log_file();
+}
+
+static void process_log_file_on_signal(int signo)
+{
+	process_log_file();
+	sigchain_pop(signo);
+	raise(signo);
+}
+
 static void git_config_date_string(const char *key, const char **output)
 {
 	if (git_config_get_string_const(key, output))
@@ -253,6 +276,24 @@ 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;
+
+	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.\n"
+			       "Automatic cleanup will not be performed "
+			       "until the file is removed.\n\n"
+			       "%s"),
+			     git_path("gc.log"), sb.buf);
+	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))
@@ -274,6 +315,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 	int force = 0;
 	const char *name;
 	pid_t pid;
+	int daemonized = 0;
 
 	struct option builtin_gc_options[] = {
 		OPT__QUIET(&quiet, N_("suppress progress reporting")),
@@ -330,13 +372,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 +394,15 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 		    name, (uintmax_t)pid);
 	}
 
+	if (daemonized) {
+		hold_lock_file_for_update(&log_lock,
+					  git_path("gc.log"),
+					  LOCK_DIE_ON_ERROR);
+		dup2(log_lock.fd, 2);
+		sigchain_push_common(process_log_file_on_signal);
+		atexit(process_log_file_at_exit);
+	}
+
 	if (gc_before_repack())
 		return -1;
 
-- 
2.3.0.rc1.137.g477eb31

  parent reply	other threads:[~2015-09-19  5:13 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-08 12:28 Since gc.autodetach=1 you can end up with auto-gc on every command with no user notification Ævar Arnfjörð Bjarmason
2015-07-08 12:47 ` Duy Nguyen
2015-08-22  2:12   ` [PATCH v3] gc: save log from daemonized gc --auto and print it next time Nguyễn Thái Ngọc Duy
2015-08-25 17:49     ` Junio C Hamano
2015-08-31 10:17       ` Duy Nguyen
2015-09-13  1:36     ` [PATCH v4] " Nguyễn Thái Ngọc Duy
2015-09-14 17:24       ` Junio C Hamano
2015-09-14 17:37         ` Junio C Hamano
2015-09-16  9:28           ` Michael Haggerty
2015-09-16 16:00             ` Junio C Hamano
2015-09-17  9:40               ` Michael Haggerty
2015-09-17 13:08               ` Duy Nguyen
2015-09-17 14:48                 ` Junio C Hamano
2015-09-19  5:13       ` Nguyễn Thái Ngọc Duy [this message]
2015-09-21 16:43         ` [PATCH v5] " Junio C Hamano
2015-09-19  5:14       ` [Alt. PATCH " Nguyễn Thái Ngọc Duy
2015-09-21 16:19         ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1442639603-25568-1-git-send-email-pclouds@gmail.com \
    --to=pclouds@gmail.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).