summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLibravatar Junio C Hamano <gitster@pobox.com>2019-10-07 11:32:55 +0900
committerLibravatar Junio C Hamano <gitster@pobox.com>2019-10-07 11:32:55 +0900
commit098e8c6716b8c6abc29b6788acdb3ac8725f835d (patch)
tree1380bdbd3b2c5623c20fb2c5a8064bbeeb0fe7be
parentMerge branch 'mr/complete-more-for-log-etc' (diff)
parentupload-pack: disable commit graph more gently for shallow traversal (diff)
downloadtgif-098e8c6716b8c6abc29b6788acdb3ac8725f835d.tar.xz
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
-rw-r--r--commit-graph.c18
-rw-r--r--commit-graph.h6
-rw-r--r--repository.h3
-rwxr-xr-xt/t5500-fetch-pack.sh38
-rw-r--r--upload-pack.c2
5 files changed, 63 insertions, 4 deletions
diff --git a/commit-graph.c b/commit-graph.c
index 5da0e16d12..394908ba9b 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -468,14 +468,21 @@ static int prepare_commit_graph(struct repository *r)
{
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 @@ void free_commit_graph(struct commit_graph *g)
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 486e64e591..7f5c933fa2 100644
--- a/commit-graph.h
+++ b/commit-graph.h
@@ -107,4 +107,10 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g, int flags)
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 fe0b5f5dc6..fe42197813 100644
--- a/repository.h
+++ b/repository.h
@@ -125,6 +125,9 @@ struct repository {
/* 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 5115711562..6b97923964 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -792,6 +792,44 @@ test_expect_success 'clone shallow since selects no commits' '
)
'
+# 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 875db92996..a00d7ece6b 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -721,7 +721,7 @@ static void deepen_by_rev_list(struct packet_writer *writer, int ac,
{
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);