Re: [PATCH 4/8] git-repack --max-pack-size: add fixup_header_footer()

From: Shawn O. Pearce <spearce@spearce.org>
Date: May 1, 2007, 05:06 UTC
Message-ID: <20070501050633.GZ5942@spearce.org>
In-Reply-To: <463679EB.2010301@gmail.com>

Dana How <danahow@gmail.com> wrote:

> Add our own version of the one in fast-import.c here.
> Needed later to correct bad object count in header for split pack.

...

Show 11 quoted lines

> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
> index bc45ca6..98066bf 100644
> --- a/builtin-pack-objects.c
> +++ b/builtin-pack-objects.c
> @@ -562,6 +562,42 @@ static off_t write_one(struct sha1file *f,
>  	return offset + size;
>  }
>  
> +static void fixup_header_footer(int pack_fd, unsigned char *pack_file_sha1,
> +				char *pack_name, uint32_t object_count)
> +{

This looks a *lot* like the code in fast-import.c. Why not refactor both to use the same implementation and stuff it away in say pack-check.c (for lack of a better place), or start a new file (pack-write.c)?

There is a *lot* of code in fast-import.c (over 2,000 lines) that was half-copied from other core code, and that was half created on its own. This is also true in index-pack.c and pack-objects.c; I'd like to see these implementations unify more rather than copy code from each other.

I know git-blame will identify the original author quite well, but I'd really like to avoid adding lots more code to maintain if we can avoid it.

-- 
Shawn.