Re: [PATCH] Refactor sha1_pack_index_name and sha1_pack_name to use a common backend

From: Junio C Hamano <junkio@cox.net>
Date: Aug 28, 2006, 04:40 UTC
Message-ID: <7vlkp9tquz.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <20060828001610.GC20904@diku.dk>

Jonas Fonseca <fonseca@diku.dk> writes:

> This adds another pair of static buffers, if that's a problem and the
> cleanup is still wanted I can change it to use malloc.

To be perfectly honest, I would rather get rid of these two functions, and also sha1[] member from struct packed_git.

The real core git does not impose any limit to the name of the packfiles, other than that the name of the datafile ends with ".pack" and that its corresponding pack index file has the same name ending with ".idx". It is only the packfile support which was later bolted on to commit walkers by commit bf592c5 that introduced this silly expectation that they are always of form "pack-[0-9a-f]{40}.pack" and "pack-[0-9a-f]{40}.idx".

For example, fetch_pack() in local-fetch.c is given an object name and tries to find the packfile that contains it by looking at ".idx" files downloaded from the other side. When it finds one, it fabricates a name for the pack file by what is in target->sha1 (where target is of type "struct packed_git").

However, struct packed_git already has perfectly good place that is meant to store the real name of the packfile, so this is totally unnecessary.

As a clean-up, I'd rather see a patch that removes the need for these two functions and one struct member, rather than keeping these two misguided functions and consolidating their implementations.