Re: [PATCH 12/15] chunk-format: create write_chunks()

[Taylor Blau <me@ttaylorr.com>]

On Thu, Dec 03, 2020 at 04:16:51PM +0000, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <dstolee@microsoft.com>
>
> The commit-graph and multi-pack-index files both use a chunk-based file
> format. They have already unified on using write_table_of_contents(),
> but we expand upon that by unifying their chunk writing loop.
>
> This takes the concepts already present in the commit-graph that were
> dropped in the multi-pack-index code during refactoring, including:
>
> * Check the hashfile for how much data was written by each write_fn.
>
> * Allow write_fn() to report an error that results in a failure
>   without using die() in the low-level commands.
>
> This simplifies the code in commit-graph.c and midx.c while laying the
> foundation for future formats using similar ideas.
>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
>  chunk-format.c | 23 +++++++++++++++++++++++
>  chunk-format.h | 13 +++++++++++++
>  commit-graph.c | 13 ++-----------
>  midx.c         |  3 +--
>  4 files changed, 39 insertions(+), 13 deletions(-)
>
> diff --git a/chunk-format.c b/chunk-format.c
> index 771b6d98d0..a6643a4fc8 100644
> --- a/chunk-format.c
> +++ b/chunk-format.c
> @@ -24,3 +24,26 @@ void write_table_of_contents(struct hashfile *f,
>  	hashwrite_be32(f, 0);
>  	hashwrite_be64(f, cur_offset);
>  }
> +
> +int write_chunks(struct hashfile *f,
> +		 struct chunk_info *chunks,
> +		 int nr,
> +		 void *data)
> +{

Serves me right for thinking that a function like write_chunks() would
be a good addition to this series without... actually reading the whole
series first ;-).

I'm glad to see that you're adding such a function, but I think that I
prefer my version which moves these four parameters into the chunkfile
struct. That allows for other functions to be created which can, for
e.g., manage the chunks themselves (like my chunkfile_push_chunk()).

Thanks,
Taylor