Re: [PATCH 2/2] gc: Remove garbage .idx files from pack dir

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 17, 2015, 16:35 UTC
Message-ID: <xmqq7fotg9fk.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1439488973-11522-2-git-send-email-dougk.ff7@gmail.com>

Doug Kelly <dougk.ff7@gmail.com> writes:

> Add a custom report_garbage handler to collect and remove garbage
> .idx files from the pack directory.

You need to explain "why" here. Why do we want to remove them? And the definition of what is "garbage" depends on that exact reason why we want to remove them.

You and I may remember the discussion we had that started with your initial problem description right now. IIRC, it had to do with something with the performance when there are many lone .idx files without corresponding .pack file in the repository, or something?

But those who will be reading "git log" output later will not know, and we would forget, too.

As discussed elsewhere, the removal needs to be protected with grace period, probably controlled via prune_expire.

Perhaps along this line on top of your patch you can squash but this is not even compile tested yet, so take it with a grain of salt.

Thanks.

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

diff --git a/builtin/gc.c b/builtin/gc.c
index 4a459f3..a652773 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -49,8 +49,21 @@ static char *pidfile;
 static void clean_pack_garbage(void)
 {
 	int i;
-	for (i = 0; i < pack_garbage.nr; i++)
+	unsigned long expire = approxidate(prune_expire);
+
+	for (i = 0; i < pack_garbage.nr; i++) {
+		const char *path = pack_garbage.items[i].string;
+		struct stat st;
+
+		if (lstat(path, &st)) {
+			error("could not stat '%s'", path);
+			continue; /* don't risk */
+		}
+		if (st.st_mtime > expire)
+			continue; /* too young */
+
 		unlink_or_warn(pack_garbage.items[i].string);
+	}
 	string_list_clear(&pack_garbage, 0);
 }