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

From: Junio C Hamano <[email-masked]>
Date: Mon, 17 Aug 2015 09:35:11 -0700

Doug Kelly <[email-masked]> 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);
 }