Re: [PATCH 03/17] commit-graph: use chunk-format write API
flat view
From: Taylor Blau <hidden>
Date: 2021-01-27 04:17:50
On Tue, Jan 26, 2021 at 04:01:12PM +0000, Derrick Stolee via GitGitGadget wrote:
From: Derrick Stolee <redacted> The commit-graph write logic is ready to make use of the chunk-format write API. Each chunk write method is already in the correct prototype. We only need to use the 'struct chunkfile' pointer and the correct API calls. Signed-off-by: Derrick Stolee <redacted>
Nicely done. The majority of this patch was remarkably easy to read, which I attribute to you doing the necessary prep work to make the callbacks usable by the new API. Thank you.
quoted hunk ↗ jump to hunk
@@ -1941,6 +1896,7 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx) close_commit_graph(ctx->r->objects); finalize_hashfile(f, file_hash.hash, CSUM_HASH_IN_STREAM | CSUM_FSYNC); + free_chunkfile(cf);
Since chunkfiles are so tightly coupled to hashfiles (i.e., you can only
"construct" a chunkfile given a 'struct hashfile*'), I wonder whether
this should be:
finalize_chunkfile(cf, ...)
instead. It seems kind of weird to give up ownership of 'f' down to the
chunkfile API only to reach down into it again.
I could even buy that you'd always want to finalize and free a chunkfile
at the same time, and so perhaps the calls could be combined, but that
may be a step too far.
Thanks,
Taylor