git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH] DO-NOT-MERGE: write and read commit-graph always

From
Derrick Stolee <dstolee@microsoft.com>
Date
Jul 18, 2018, 15:22 UTC
Message-ID
<20180718152244.45513-1-dstolee@microsoft.com>
In-Reply-To
<pull.11.git.gitgitgadget@gmail.com>

This commit is not intended to be merged, but is only to create a test environment to see what works with the commit-graph feature and what does not. The tests that fail are primarily related to corrupting the object store to remove a commit from visibility and testing that rev-list fails -- except it doesn't when there is a commit-graph that prevents parsing from the object database. The following tests will fail with this commit, but are not "real" bugs:

t0410-partial-clone.sh, Test 9 t5307-pack-missing-commit.sh, Tests 3-4 t6011-rev-list-with-bad-commit.sh, Test 4

The following test fails because the repo has ambiguous merge-bases, and the commit-graph changes the walk order so we select a different one. This alters the resulting merge from the expected result.

t6024-recursive-merge.sh, Test 4

The tests above are made to pass by deleting the commit-graph file before the necessary steps.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 builtin/commit.c                    |  2 ++
 builtin/gc.c                        |  3 +--
 commit-graph.c                      | 11 -----------
 t/t0410-partial-clone.sh            |  1 +
 t/t5307-pack-missing-commit.sh      |  2 ++
 t/t6011-rev-list-with-bad-commit.sh |  1 +
 t/t6024-recursive-merge.sh          |  1 +
 7 files changed, 8 insertions(+), 13 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 158e3f843a..acc31252a9 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -33,6 +33,7 @@
 #include "sequencer.h"
 #include "mailmap.h"
 #include "help.h"
+#include "commit-graph.h"
 
 static const char * const builtin_commit_usage[] = {
 	N_("git commit [<options>] [--] <pathspec>..."),
@@ -1652,6 +1653,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		     "not exceeded, and then \"git reset HEAD\" to recover."));
 
 	rerere(0);
+	write_commit_graph_reachable(get_object_directory(), 1);
 	run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
 	run_commit_hook(use_editor, get_index_file(), "post-commit", NULL);
 	if (amend && !no_post_rewrite) {
diff --git a/builtin/gc.c b/builtin/gc.c
index e103f0f85d..60ab773087 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -41,7 +41,7 @@ static int aggressive_depth = 50;
 static int aggressive_window = 250;
 static int gc_auto_threshold = 6700;
 static int gc_auto_pack_limit = 50;
-static int gc_write_commit_graph;
+static int gc_write_commit_graph = 1;
 static int detach_auto = 1;
 static timestamp_t gc_log_expire_time;
 static const char *gc_log_expire = "1.day.ago";
@@ -131,7 +131,6 @@ static void gc_config(void)
 	git_config_get_int("gc.aggressivedepth", &aggressive_depth);
 	git_config_get_int("gc.auto", &gc_auto_threshold);
 	git_config_get_int("gc.autopacklimit", &gc_auto_pack_limit);
-	git_config_get_bool("gc.writecommitgraph", &gc_write_commit_graph);
 	git_config_get_bool("gc.autodetach", &detach_auto);
 	git_config_get_expiry("gc.pruneexpire", &prune_expire);
 	git_config_get_expiry("gc.worktreepruneexpire", &prune_worktrees_expire);
diff --git a/commit-graph.c b/commit-graph.c
index 237d4e7d1b..ed0d27c12e 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -227,22 +227,11 @@ static int prepare_commit_graph(struct repository *r)
 {
 	struct alternate_object_database *alt;
 	char *obj_dir;
-	int config_value;
 
 	if (r->objects->commit_graph_attempted)
 		return !!r->objects->commit_graph;
 	r->objects->commit_graph_attempted = 1;
 
-	if (repo_config_get_bool(r, "core.commitgraph", &config_value) ||
-	    !config_value)
-		/*
-		 * This repository is not configured to use commit graphs, so
-		 * do not load one. (But report commit_graph_attempted anyway
-		 * so that commit graph loading is not attempted again for this
-		 * repository.)
-		 */
-		return 0;
-
 	if (!commit_graph_compatible(r))
 		return 0;
 
diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh
index 4984ca583d..c235672b03 100755
--- a/t/t0410-partial-clone.sh
+++ b/t/t0410-partial-clone.sh
@@ -181,6 +181,7 @@ test_expect_success 'rev-list stops traversal at missing and promised commit' '
 
 	git -C repo config core.repositoryformatversion 1 &&
 	git -C repo config extensions.partialclone "arbitrary string" &&
+	rm -rf repo/.git/objects/info/commit-graph &&
 	git -C repo rev-list --exclude-promisor-objects --objects bar >out &&
 	grep $(git -C repo rev-parse bar) out &&
 	! grep $FOO out
diff --git a/t/t5307-pack-missing-commit.sh b/t/t5307-pack-missing-commit.sh
index ae52a1882d..0bb54ae227 100755
--- a/t/t5307-pack-missing-commit.sh
+++ b/t/t5307-pack-missing-commit.sh
@@ -24,10 +24,12 @@ test_expect_success 'check corruption' '
 '
 
 test_expect_success 'rev-list notices corruption (1)' '
+	rm -rf .git/objects/info/commit-graph &&
 	test_must_fail git rev-list HEAD
 '
 
 test_expect_success 'rev-list notices corruption (2)' '
+	rm -rf .git/objects/info/commit-graph &&
 	test_must_fail git rev-list --objects HEAD
 '
 
diff --git a/t/t6011-rev-list-with-bad-commit.sh b/t/t6011-rev-list-with-bad-commit.sh
index e51eb41f4b..9c9cc4d540 100755
--- a/t/t6011-rev-list-with-bad-commit.sh
+++ b/t/t6011-rev-list-with-bad-commit.sh
@@ -43,6 +43,7 @@ test_expect_success 'corrupt second commit object' \
 
 test_expect_success 'rev-list should fail' \
    '
+   rm -rf .git/objects/info/commit-graph &&
    test_must_fail git rev-list --all > /dev/null
    '
 
diff --git a/t/t6024-recursive-merge.sh b/t/t6024-recursive-merge.sh
index 3f59e58dfb..cec10983cd 100755
--- a/t/t6024-recursive-merge.sh
+++ b/t/t6024-recursive-merge.sh
@@ -61,6 +61,7 @@ GIT_AUTHOR_DATE="2006-12-12 23:00:08" git commit -m F
 '
 
 test_expect_success "combined merge conflicts" "
+	rm -rf .git/objects/info/commit-graph &&
 	test_must_fail git merge -m final G
 "
 
-- 
2.18.0.118.gd4f65b8d14
Previous: Jakub NarebskiNext: Jakub Narebski
Message 25 of 47 in “Clarify commit-graph and grafts/replace/shallow incompatibilities”
  1. 0/8 Clarify commit-graph and grafts/replace/shallow incompatibilitiesDerrick Stolee via GitGitGadget, Jul 18, 2018
  2. 1/8 refs.c: migrate internal ref iteration to pass thru repository argumentStefan Beller via GitGitGadget, Jul 18, 2018
  3. 2/8 refs.c: upgrade for_each_replace_ref to be a each_repo_ref_fn callbackStefan Beller via GitGitGadget, Jul 18, 2018
  4. Junio C HamanoJul 18, 2018
  5. Stefan BellerJul 18, 2018
  6. Junio C HamanoJul 18, 2018
  7. 3/8 commit-graph: update design documentDerrick Stolee via GitGitGadget, Jul 18, 2018
  8. Jakub NarebskiJul 29, 2018
  9. 4/8 test-repository: properly init repoDerrick Stolee via GitGitGadget, Jul 18, 2018
  10. Jakub NarebskiJul 29, 2018
  11. 5/8 commit-graph: not compatible with replace objectsDerrick Stolee via GitGitGadget, Jul 18, 2018
  12. Jeff KingJul 18, 2018
  13. Jeff KingJul 18, 2018
  14. Derrick StoleeJul 18, 2018
  15. Derrick StoleeJul 20, 2018
  16. Stefan BellerJul 20, 2018
  17. Derrick StoleeJul 20, 2018
  18. Jakub NarebskiJul 29, 2018
  19. 6/8 commit-graph: not compatible with graftsDerrick Stolee via GitGitGadget, Jul 18, 2018
  20. Jakub NarebskiJul 29, 2018
  21. 7/8 commit-graph: not compatible with uninitialized repoDerrick Stolee via GitGitGadget, Jul 18, 2018
  22. Jakub NarebskiJul 29, 2018
  23. 8/8 commit-graph: close_commit_graph before shallow walkDerrick Stolee via GitGitGadget, Jul 18, 2018
  24. Jakub NarebskiJul 30, 2018
  25. DO-NOT-MERGE: write and read commit-graph alwaysDerrick Stolee, Jul 18, 2018
  26. Jakub NarebskiJul 30, 2018
  27. Stefan BellerJul 30, 2018
  28. Jakub NarebskiJul 31, 2018
  29. Elijah NewrenJul 31, 2018
  30. Jakub NarebskiJul 29, 2018
  31. Eric SunshineJul 29, 2018
  32. 0/8 Clarify commit-graph and grafts/replace/shallow incompatibilitiesDerrick Stolee, Aug 20, 2018
  33. 6/8 commit-graph: not compatible with graftsDerrick Stolee, Aug 20, 2018
  34. 7/8 commit-graph: not compatible with uninitialized repoDerrick Stolee, Aug 20, 2018
  35. 2/8 refs.c: upgrade for_each_replace_ref to be a each_repo_ref_fn callbackDerrick Stolee, Aug 20, 2018
  36. 3/8 commit-graph: update design documentDerrick Stolee, Aug 20, 2018
  37. 4/8 test-repository: properly init repoDerrick Stolee, Aug 20, 2018
  38. 5/8 commit-graph: not compatible with replace objectsDerrick Stolee, Aug 20, 2018
  39. Junio C HamanoAug 21, 2018
  40. Derrick StoleeAug 21, 2018
  41. 8/8 commit-graph: close_commit_graph before shallow walkDerrick Stolee, Aug 20, 2018
  42. 1/8 refs.c: migrate internal ref iteration to pass thru repository argumentDerrick Stolee, Aug 20, 2018
  43. Stefan BellerAug 20, 2018
  44. Junio C HamanoAug 21, 2018
  45. Derrick StoleeAug 21, 2018
  46. Stefan BellerAug 20, 2018
  47. Derrick StoleeAug 20, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.