Merge branch 'jk/disable-commit-graph-during-upload-pack'
The "upload-pack" (the counterpart of "git fetch") needs to disable
commit-graph when responding to a shallow clone/fetch request, but
the way this was done made Git panic, which has been corrected.
* jk/disable-commit-graph-during-upload-pack:
upload-pack: disable commit graph more gently for shallow traversal
commit-graph: bump DIE_ON_LOAD check to actual load-time
diff --git a/commit-graph.c b/commit-graph.c
index 5da0e16..394908b 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -468,14 +468,21 @@
{
struct object_directory *odb;
- if (git_env_bool(GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD, 0))
- die("dying as requested by the '%s' variable on commit-graph load!",
- GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD);
+ /*
+ * This must come before the "already attempted?" check below, because
+ * we want to disable even an already-loaded graph file.
+ */
+ if (r->commit_graph_disabled)
+ return 0;
if (r->objects->commit_graph_attempted)
return !!r->objects->commit_graph;
r->objects->commit_graph_attempted = 1;
+ if (git_env_bool(GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD, 0))
+ die("dying as requested by the '%s' variable on commit-graph load!",
+ GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD);
+
prepare_repo_settings(r);
if (!git_env_bool(GIT_TEST_COMMIT_GRAPH, 0) &&
@@ -2101,3 +2108,8 @@
free(g->filename);
free(g);
}
+
+void disable_commit_graph(struct repository *r)
+{
+ r->commit_graph_disabled = 1;
+}
diff --git a/commit-graph.h b/commit-graph.h
index 486e64e..7f5c933 100644
--- a/commit-graph.h
+++ b/commit-graph.h
@@ -107,4 +107,10 @@
void close_commit_graph(struct raw_object_store *);
void free_commit_graph(struct commit_graph *);
+/*
+ * Disable further use of the commit graph in this process when parsing a
+ * "struct commit".
+ */
+void disable_commit_graph(struct repository *r);
+
#endif
diff --git a/repository.h b/repository.h
index fe0b5f5..fe42197 100644
--- a/repository.h
+++ b/repository.h
@@ -125,6 +125,9 @@
/* A unique-id for tracing purposes. */
int trace2_repo_id;
+ /* True if commit-graph has been disabled within this process. */
+ int commit_graph_disabled;
+
/* Configurations */
/* Indicate if a repository has a different 'commondir' from 'gitdir' */
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 5115711..6b97923 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -792,6 +792,44 @@
)
'
+# A few subtle things about the request in this test:
+#
+# - the server must have commit-graphs present and enabled
+#
+# - the history is such that our want/have share a common ancestor ("base"
+# here)
+#
+# - we send only a single have, which is fewer than a normal client would
+# send. This ensures that we don't parse "base" up front with
+# parse_object(), but rather traverse to it as a parent while deciding if we
+# can stop the "have" negotiation, and call parse_commit(). The former
+# sees the actual object data and so always loads the three oid, whereas the
+# latter will try to load it lazily.
+#
+# - we must use protocol v2, because it handles the "have" negotiation before
+# processing the shallow directives
+#
+test_expect_success 'shallow since with commit graph and already-seen commit' '
+ test_create_repo shallow-since-graph &&
+ (
+ cd shallow-since-graph &&
+ test_commit base &&
+ test_commit master &&
+ git checkout -b other HEAD^ &&
+ test_commit other &&
+ git commit-graph write --reachable &&
+ git config core.commitGraph true &&
+
+ GIT_PROTOCOL=version=2 git upload-pack . <<-EOF >/dev/null
+ 0012command=fetch
+ 00010013deepen-since 1
+ 0032want $(git rev-parse other)
+ 0032have $(git rev-parse master)
+ 0000
+ EOF
+ )
+'
+
test_expect_success 'shallow clone exclude tag two' '
test_create_repo shallow-exclude &&
(
diff --git a/upload-pack.c b/upload-pack.c
index 875db92..a00d7ec 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -721,7 +721,7 @@
{
struct commit_list *result;
- close_commit_graph(the_repository->objects);
+ disable_commit_graph(the_repository);
result = get_shallow_commits_by_rev_list(ac, av, SHALLOW, NOT_SHALLOW);
send_shallow(writer, result);
free_commit_list(result);
