From 8f801804befa12a9c4ddff91275cf03612f1895d Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Thu, 8 Oct 2020 00:50:39 +0000 Subject: [PATCH 1/2] maintenance: test commit-graph auto condition The auto condition for the commit-graph maintenance task walks refs looking for commits that are not in the commit-graph file. This was added in 4ddc79b2 (maintenance: add auto condition for commit-graph task, 2020-09-17) but was left untested. The initial goal of this change was to demonstrate the feature works properly by adding tests. However, there was an off-by-one error that caused the basic tests around maintenance.commit-graph.auto=1 to fail when it should work. The subtlety is that if a ref tip is not in the commit-graph, then we were not adding that to the total count. In the test, we see that we have only added one commit since our last commit-graph write, so the auto condition would say there is nothing to do. The fix is simple: add the check for the commit-graph position to see that the tip is not in the commit-graph file before starting our walk. Since this happens before adding to the DFS stack, we do not need to clear our (currently empty) commit list. This does add some extra complexity for the test, because we also want to verify that the walk along the parents actually does some work. This means we need to add at least two commits in a row without writing the commit-graph. However, we also need to make sure no additional refs are pointing to the middle of this list or else the for_each_ref() in should_write_commit_graph() might visit these commits as tips instead of doing a DFS walk. Hence, the last two commits are added with "git commit" instead of "test_commit". Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano --- builtin/gc.c | 8 +++++++- t/t7900-maintenance.sh | 29 +++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/builtin/gc.c b/builtin/gc.c index 090959350e..12ddb68bba 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -737,9 +737,15 @@ static int dfs_on_ref(const char *refname, commit = lookup_commit(the_repository, oid); if (!commit) return 0; - if (parse_commit(commit)) + if (parse_commit(commit) || + commit_graph_position(commit) != COMMIT_NOT_FROM_GRAPH) return 0; + data->num_not_in_graph++; + + if (data->num_not_in_graph >= data->limit) + return 1; + commit_list_append(commit, &stack); while (!result && stack) { diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh index 53c883531e..ee1f4a7ae4 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -52,6 +52,35 @@ test_expect_success 'run --task=' ' test_subcommand git commit-graph write --split --reachable --no-progress err && test_i18ngrep "is not a valid task" err From d334107c5da27e5212e21e77da03e938ea6db976 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Mon, 12 Oct 2020 13:28:34 +0000 Subject: [PATCH 2/2] maintenance: core.commitGraph=false prevents writes Recently, a user had an issue due to combining fetch.writeCommitGraph=true with core.commitGraph=false. The root bug has been resolved by preventing commit-graph writes when core.commitGraph is disabled. This happens inside the 'git commit-graph write' command, but we can be more aware of this situation and prevent that process from ever starting in the 'commit-graph' maintenance task. Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano --- builtin/gc.c | 4 ++++ t/t7900-maintenance.sh | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/builtin/gc.c b/builtin/gc.c index 12ddb68bba..e80331c4e2 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -813,6 +813,10 @@ static int run_write_commit_graph(struct maintenance_run_opts *opts) static int maintenance_task_commit_graph(struct maintenance_run_opts *opts) { + prepare_repo_settings(the_repository); + if (!the_repository->settings.core_commit_graph) + return 0; + close_object_store(the_repository->objects); if (run_write_commit_graph(opts)) { error(_("failed to write commit-graph")); diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh index ee1f4a7ae4..9776154a2a 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -52,6 +52,14 @@ test_expect_success 'run --task=' ' test_subcommand git commit-graph write --split --reachable --no-progress /dev/null && + test_subcommand ! git commit-graph write --split --reachable --no-progress \ +