Re: [PATCH 5/9] builtin/repack.c: move `.idx` files into place last

Taylor Blau [off-list ref] writes:

In a similar spirit as the previous patch, fix the identical problem
from `git repack` (which invokes `pack-objects` with a temporary
location for output, and then moves the files into their final locations
itself).

OK.  This array is used in cmd_repack() to call rename() in the
order in which its elements appear, so moving the entry to the last
means it will be renamed the last.

And the .idx entry MUST stay the last.  I wonder if dropping the
trailing comma and replace it with a "/* must be at the end */"
comment would be an effective way to ensure that.

On Thu, Sep 09, 2021 at 12:38:52PM -0700, Junio C Hamano wrote:

Taylor Blau [off-list ref] writes:

quoted

diff --git a/builtin/repack.c b/builtin/repack.c
index 5f9bc74adc..c3e4771609 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c

@@ -208,10 +208,10 @@ static struct {
 	unsigned optional:1;
 } exts[] = {
 	{".pack"},
-	{".idx"},
 	{".rev", 1},
 	{".bitmap", 1},
 	{".promisor", 1},
+	{".idx"},
 };

And the .idx entry MUST stay the last.  I wonder if dropping the
trailing comma and replace it with a "/* must be at the end */"
comment would be an effective way to ensure that.

Heh, Ævar even wrote that up, and I responded that I did not think it
helped much. Again, I'm happy to add it back if anybody feels strongly.

Thanks,
Taylor