diff options
author | Junio C Hamano <gitster@pobox.com> | 2019-10-07 11:32:55 +0900 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2019-10-07 11:32:55 +0900 |
commit | 098e8c6716b8c6abc29b6788acdb3ac8725f835d (patch) | |
tree | 1380bdbd3b2c5623c20fb2c5a8064bbeeb0fe7be | |
parent | Merge branch 'mr/complete-more-for-log-etc' (diff) | |
parent | upload-pack: disable commit graph more gently for shallow traversal (diff) | |
download | tgif-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.c | 18 | ||||
-rw-r--r-- | commit-graph.h | 6 | ||||
-rw-r--r-- | repository.h | 3 | ||||
-rwxr-xr-x | t/t5500-fetch-pack.sh | 38 | ||||
-rw-r--r-- | upload-pack.c | 2 |
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); |