@matheusbsbs / matheus-bagliano / commits / c195ecda77

commit-graph.c: remove temporary graph layers on exit

Since the introduction of split commit graph layers in 92b1ea66b9a (Merge branch 'ds/commit-graph-incremental', 2019-07-19), the function write_commit_graph_file() has done the following when writing an incremental commit-graph layer: - used a lock_file to control access to the commit-graph-chain file - used an auxiliary file (whose descriptor was stored in 'fd') to write the new commit-graph layer itself Using a lock_file to control access to the commit-graph-chain is sensible, since only one writer may modify it at a time. Likewise, when the commit-graph machinery is writing out a single layer, the lock_file structure is used to modify the commit-graph itself. This is also sensible, since the non-incremental commit-graph may also have at most one writer. However, using an auxiliary temporary file without using the tempfile.h API means that writes that fail after the temporary graph layer has been created will leave around a file in $GIT_DIR/objects/info/commit-graphs/tmp_graph_XXXXXX The commit-graph-chain file and non-incremental commit-graph do not suffer from this problem as the lockfile.h API uses the tempfile.h API transparently, so processes that died before moving those finals into their final location cleaned up after themselves. Ensure that the temporary file used to write incremental commit-graphs is also managed with the tempfile.h API, to ensure that we do not ever leave tmp_graph_XXXXXX files laying around. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Taylor Blau committed Jun 6, 2024 at 18:19 UTC c195ecda7703098f28c160cc63306ca0e1488236

2 files changed +33 -12

commit-graph.c

+8 -11

@@ -2002,8 +2002,8 @@ static int write_graph_chunk_base(struct hashfile *f,

2002 static int write_commit_graph_file(struct write_commit_graph_context *ctx)

2003 {

2004 uint32_t i;

2005 - int fd;

2005 struct hashfile *f;

2006 + struct tempfile *graph_layer; /* when ctx->split is non-zero */

2007 struct lock_file lk = LOCK_INIT;

2008 const unsigned hashsz = the_hash_algo->rawsz;

2009 struct strbuf progress_title = STRBUF_INIT;

@@ -2035,24 +2035,23 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx)

2035 LOCK_DIE_ON_ERROR, 0444);

2036 free(lock_name);

2037

2038 - fd = git_mkstemp_mode(ctx->graph_name, 0444);

2039 - if (fd < 0) {

2038 + graph_layer = mks_tempfile_m(ctx->graph_name, 0444);

2039 + if (!graph_layer) {

2040 error(_("unable to create temporary graph layer"));

2041 return -1;

2042 }

2043

2044 - if (adjust_shared_perm(ctx->graph_name)) {

2044 + if (adjust_shared_perm(get_tempfile_path(graph_layer))) {

2045 error(_("unable to adjust shared permissions for '%s'"),

2046 - ctx->graph_name);

2046 + get_tempfile_path(graph_layer));

2047 return -1;

2048 }

2049

2050 - f = hashfd(fd, ctx->graph_name);

2050 + f = hashfd(get_tempfile_fd(graph_layer), get_tempfile_path(graph_layer));

2051 } else {

2052 hold_lock_file_for_update_mode(&lk, ctx->graph_name,

2053 LOCK_DIE_ON_ERROR, 0444);

2054 - fd = get_lock_file_fd(&lk);

2055 - f = hashfd(fd, get_lock_file_path(&lk));

2054 + f = hashfd(get_lock_file_fd(&lk), get_lock_file_path(&lk));

2055 }

2056

2057 cf = init_chunkfile(f);

@@ -2133,8 +2132,6 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx)

2132 char *final_graph_name;

2133 int result;

2134

2136 - close(fd);

2137 -

2135 if (!chainf) {

2136 error(_("unable to open commit-graph chain file"));

2137 return -1;

@@ -2169,7 +2166,7 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx)

2166 free(ctx->commit_graph_filenames_after[ctx->num_commit_graphs_after - 1]);

2167 ctx->commit_graph_filenames_after[ctx->num_commit_graphs_after - 1] = final_graph_name;

2168

2172 - result = rename(ctx->graph_name, final_graph_name);

2169 + result = rename_tempfile(&graph_layer, final_graph_name);

2170

2171 for (i = 0; i < ctx->num_commit_graphs_after; i++)

2172 fprintf(get_lock_file_fp(&lk), "%s\n", ctx->commit_graph_hash_after[i]);

t/t5324-split-commit-graph.sh

+25 -1

@@ -13,7 +13,8 @@ test_expect_success 'setup repo' '

13 git init &&

14 git config core.commitGraph true &&

15 git config gc.writeCommitGraph false &&

16 - infodir=".git/objects/info" &&

16 + objdir=".git/objects" &&

17 + infodir="$objdir/info" &&

18 graphdir="$infodir/commit-graphs" &&

19 test_oid_cache <<-EOM

20 shallow sha1:2132

@@ -718,4 +719,27 @@ test_expect_success 'write generation data chunk when commit-graph chain is repl

719 )

720 '

721

722 +test_expect_success 'temporary graph layer is discarded upon failure' '

723 + git init layer-discard &&

724 + (

725 + cd layer-discard &&

726 +

727 + test_commit A &&

728 + test_commit B &&

729 +

730 + # Intentionally remove commit "A" from the object store

731 + # so that the commit-graph machinery fails to parse the

732 + # parents of "B".

733 + #

734 + # This takes place after the commit-graph machinery has

735 + # initialized a new temporary file to store the contents

736 + # of the new graph layer, so will allow us to ensure

737 + # that the temporary file is discarded upon failure.

738 + rm $objdir/$(test_oid_to_path $(git rev-parse HEAD^)) &&

739 +

740 + test_must_fail git commit-graph write --reachable --split &&

741 + test_dir_is_empty $graphdir

742 + )

743 +'

744 +

745 test_done