Subject: [PATCH] Update "gc" behavior in commit, merge, am, rebase and index-pack

From: Nguyễn Thái Ngọc Duy <[email-masked]>
Date: Sat, 12 May 2012 15:08:54 +0700

A few attempts have been made in the past to change 'gc --auto' [1]
[2]. This is another such attempt.

Commit d4bb43e (Invoke "git gc --auto" from commit, merge, am and
rebase. - 2007-09-05) used the rule to put "gc --auto" is "where
update-ref occurs". I would argue that this is not a good condition to
run gc, because (at least current) gc is slow. We encourage commit
often and rebase to make all patches in good shape and this workflow
should not be interrupted/slowed down by random "gc --auto".

Instead, we could just inform users that "gc" should be run soon in
commonly used commands (this patch also reinstates "gc" check in
commit, which was lost at the sh->C conversion). [1] and [2] can annoy
users constantly with warnings. This patch shows the warning at most
once a day.

There are commands that are not expected to return immediately. These
are more suitable for "gc --auto". One of them is receive-pack, which
already calls "gc --auto". The other one is index-pack, which "gc
--auto" is added in by this patch.

In short, after this patch:

 - receive-pack and index-pack (or push, pull/fetch at high level) can
   do "gc --auto".
 - commmit, merge, am, rebase warns once a day, no actual gc.

[1] http://thread.gmane.org/gmane.comp.version-control.git/184848
   adds an config option to turn --auto to warning only

[2] http://thread.gmane.org/gmane.comp.version-control.git/187711
   make it warn early before actual do the house keeping

Signed-off-by: Nguyễn Thái Ngọc Duy <[email-masked]>
---
 builtin/commit.c           |    2 ++
 builtin/gc.c               |   21 ++++++++++++++++++++-
 builtin/index-pack.c       |   16 ++++++++++++++--
 builtin/merge.c            |    2 +-
 git-am.sh                  |    2 +-
 git-rebase--interactive.sh |    2 +-
 6 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index a2ec73d..4ba5677 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1418,6 +1418,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 
 		OPT_END()
 	};
+	const char *argv_gc_auto[] = { "gc", "--check", NULL };
 
 	struct strbuf sb = STRBUF_INIT;
 	struct strbuf author_ident = STRBUF_INIT;
@@ -1589,6 +1590,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		     "new_index file. Check that disk is not full or quota is\n"
 		     "not exceeded, and then \"git reset HEAD\" to recover."));
 
+	run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
 	rerere(0);
 	run_hook(get_index_file(), "post-commit", NULL);
 	if (amend && !no_post_rewrite) {
diff --git a/builtin/gc.c b/builtin/gc.c
index 9b4232c..a85f71c 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -171,6 +171,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 {
 	int aggressive = 0;
 	int auto_gc = 0;
+	int check_gc = 0;
 	int quiet = 0;
 
 	struct option builtin_gc_options[] = {
@@ -180,12 +181,16 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 			PARSE_OPT_OPTARG, NULL, (intptr_t)prune_expire },
 		OPT_BOOLEAN(0, "aggressive", &aggressive, "be more thorough (increased runtime)"),
 		OPT_BOOLEAN(0, "auto", &auto_gc, "enable auto-gc mode"),
+		OPT_BOOLEAN(0, "check", &check_gc, "enable auto-gc mode"),
 		OPT_END()
 	};
 
 	if (argc == 2 && !strcmp(argv[1], "-h"))
 		usage_with_options(builtin_gc_usage, builtin_gc_options);
 
+	if (auto_gc && check_gc)
+		die(_("--auto and --check are incompatible"));
+
 	argv_array_pushl(&pack_refs_cmd, "pack-refs", "--all", "--prune", NULL);
 	argv_array_pushl(&reflog, "reflog", "expire", "--all", NULL);
 	argv_array_pushl(&repack, "repack", "-d", "-l", NULL);
@@ -211,12 +216,26 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 	if (quiet)
 		argv_array_push(&repack, "-q");
 
-	if (auto_gc) {
+	if (auto_gc || check_gc) {
 		/*
 		 * Auto-gc should be least intrusive as possible.
 		 */
 		if (!need_to_gc())
 			return 0;
+		if (check_gc) {
+			struct stat st;
+			if (stat(git_path("gc_needed"), &st))
+				open(git_path("gc_needed"), O_CREAT | O_RDWR);
+			else {
+				/* do not bother users more than once a day */
+				if (time(NULL) - st.st_mtime < 86400)
+					return 0;
+				utime(git_path("gc_needed"), NULL);
+			}
+			warning(_("This repository needs maintenance. "
+				  "Please run \"git gc\" as soon as possible."));
+			return 0;
+		}
 		if (quiet)
 			fprintf(stderr, _("Auto packing the repository for optimum performance.\n"));
 		else
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 83555e5..438245c 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -9,6 +9,7 @@
 #include "progress.h"
 #include "fsck.h"
 #include "exec_cmd.h"
+#include "run-command.h"
 
 static const char index_pack_usage[] =
 "git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--verify] [--strict] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
@@ -63,6 +64,7 @@ static int nr_resolved_deltas;
 static int from_stdin;
 static int strict;
 static int verbose;
+static int auto_gc;
 
 static struct progress *progress;
 
@@ -968,6 +970,10 @@ static int git_index_pack_config(const char *k, const char *v, void *cb)
 			die("bad pack.indexversion=%"PRIu32, opts->version);
 		return 0;
 	}
+	if (strcmp(k, "receive.autogc") == 0) {
+		auto_gc = git_config_bool(k, v);
+		return 0;
+	}
 	return git_default_config(k, v, cb);
 }
 
@@ -1254,12 +1260,18 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)
 	curr_index = write_idx_file(index_name, idx_objects, nr_objects, &opts, pack_sha1);
 	free(idx_objects);
 
-	if (!verify)
+	if (!verify) {
 		final(pack_name, curr_pack,
 		      index_name, curr_index,
 		      keep_name, keep_msg,
 		      pack_sha1);
-	else
+		if (auto_gc) {
+			const char *argv_gc_auto[] = {
+				"gc", "--auto", "--quiet", NULL,
+			};
+			run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
+		}
+	} else
 		close(input_fd);
 	free(objects);
 	free(index_name_buf);
diff --git a/builtin/merge.c b/builtin/merge.c
index 470fc57..8b8716e 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -385,7 +385,7 @@ static void finish(struct commit *head_commit,
 		if (verbosity >= 0 && !merge_msg.len)
 			printf(_("No merge message -- not updating HEAD\n"));
 		else {
-			const char *argv_gc_auto[] = { "gc", "--auto", NULL };
+			const char *argv_gc_auto[] = { "gc", "--check", NULL };
 			update_ref(reflog_message.buf, "HEAD",
 				new_head, head, 0,
 				DIE_ON_ERR);
diff --git a/git-am.sh b/git-am.sh
index f8b7a0c..0f2dbc4 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -902,4 +902,4 @@ if test -s "$dotest"/rewritten; then
 fi
 
 rm -fr "$dotest"
-git gc --auto
+git gc --check
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 0c19b7c..89996a3 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -573,7 +573,7 @@ do_next () {
 		true # we don't care if this hook failed
 	fi &&
 	rm -rf "$state_dir" &&
-	git gc --auto &&
+	git gc --check &&
 	warn "Successfully rebased and updated $head_name."
 
 	exit
-- 
1.7.8.36.g69ee2