From 3a1ea94a491e3e0bcbc2d55b4cfb0b9eacace608 Mon Sep 17 00:00:00 2001 From: Han Xin Date: Fri, 1 Jul 2022 09:34:30 +0800 Subject: [PATCH 1/2] commit-graph.c: no lazy fetch in lookup_commit_in_graph() The commit-graph is used to opportunistically optimize accesses to certain pieces of information on commit objects, and lookup_commit_in_graph() tries to say "no" when the requested commit does not locally exist by returning NULL, in which case the caller can ask for (which may result in on-demand fetching from a promisor remote) and parse the commit object itself. However, it uses a wrong helper, repo_has_object_file(), to do so. This helper not only checks if an object is mmediately available in the local object store, but also tries to fetch from a promisor remote. But the fetch machinery calls lookup_commit_in_graph(), thus causing an infinite loop. We should make lookup_commit_in_graph() expect that a commit given to it can be legitimately missing from the local object store, by using the has_object_file() helper instead. Signed-off-by: Han Xin Signed-off-by: Junio C Hamano --- commit-graph.c | 2 +- t/t5330-no-lazy-fetch-with-commit-graph.sh | 70 ++++++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) create mode 100755 t/t5330-no-lazy-fetch-with-commit-graph.sh diff --git a/commit-graph.c b/commit-graph.c index 2b52818731..2dd9bcc7ea 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -907,7 +907,7 @@ struct commit *lookup_commit_in_graph(struct repository *repo, const struct obje return NULL; if (!search_commit_pos_in_graph(id, repo->objects->commit_graph, &pos)) return NULL; - if (!repo_has_object_file(repo, id)) + if (!has_object(repo, id, 0)) return NULL; commit = lookup_commit(repo, id); diff --git a/t/t5330-no-lazy-fetch-with-commit-graph.sh b/t/t5330-no-lazy-fetch-with-commit-graph.sh new file mode 100755 index 0000000000..be33334229 --- /dev/null +++ b/t/t5330-no-lazy-fetch-with-commit-graph.sh @@ -0,0 +1,70 @@ +#!/bin/sh + +test_description='test for no lazy fetch with the commit-graph' + +. ./test-lib.sh + +run_with_limited_processses () { + # bash and ksh use "ulimit -u", dash uses "ulimit -p" + if test -n "$BASH_VERSION" + then + ulimit_max_process="-u" + elif test -n "$KSH_VERSION" + then + ulimit_max_process="-u" + fi + (ulimit ${ulimit_max_process-"-p"} 512 && "$@") +} + +test_lazy_prereq ULIMIT_PROCESSES ' + run_with_limited_processses true +' + +if ! test_have_prereq ULIMIT_PROCESSES +then + skip_all='skipping tests for no lazy fetch with the commit-graph, ulimit processes not available' + test_done +fi + +test_expect_success 'setup: prepare a repository with a commit' ' + git init with-commit && + test_commit -C with-commit the-commit && + oid=$(git -C with-commit rev-parse HEAD) +' + +test_expect_success 'setup: prepare a repository with commit-graph contains the commit' ' + git init with-commit-graph && + echo "$(pwd)/with-commit/.git/objects" \ + >with-commit-graph/.git/objects/info/alternates && + # create a ref that points to the commit in alternates + git -C with-commit-graph update-ref refs/ref_to_the_commit "$oid" && + # prepare some other objects to commit-graph + test_commit -C with-commit-graph something && + git -c gc.writeCommitGraph=true -C with-commit-graph gc && + test_path_is_file with-commit-graph/.git/objects/info/commit-graph +' + +test_expect_success 'setup: change the alternates to what without the commit' ' + git init --bare without-commit && + git -C with-commit-graph cat-file -e $oid && + echo "$(pwd)/without-commit/objects" \ + >with-commit-graph/.git/objects/info/alternates && + test_must_fail git -C with-commit-graph cat-file -e $oid +' + +test_expect_success 'fetch any commit from promisor with the usage of the commit graph' ' + # setup promisor and prepare any commit to fetch + git -C with-commit-graph remote add origin "$(pwd)/with-commit" && + git -C with-commit-graph config remote.origin.promisor true && + git -C with-commit-graph config remote.origin.partialclonefilter blob:none && + test_commit -C with-commit any-commit && + anycommit=$(git -C with-commit rev-parse HEAD) && + + run_with_limited_processses env GIT_TRACE="$(pwd)/trace.txt" \ + git -C with-commit-graph fetch origin $anycommit 2>err && + ! grep "fatal: promisor-remote: unable to fork off fetch subprocess" err && + grep "git fetch origin" trace.txt >actual && + test_line_count = 1 actual +' + +test_done From cb88b37cb9f711f702ea3d1ec3226db44081b454 Mon Sep 17 00:00:00 2001 From: Han Xin Date: Tue, 12 Jul 2022 16:01:43 +0800 Subject: [PATCH 2/2] t5330: remove run_with_limited_processses() run_with_limited_processses() is used to end the loop faster when an infinite loop happen. But "ulimit" is tied to the entire development station, and the test will fail due to too many other processes or using "--stress". Without run_with_limited_processses() the infinite loop can also be stopped due to global configrations or quotas, and the verification still works fine. So let's remove run_with_limited_processses(). Signed-off-by: Han Xin Signed-off-by: Junio C Hamano --- t/t5330-no-lazy-fetch-with-commit-graph.sh | 25 +--------------------- 1 file changed, 1 insertion(+), 24 deletions(-) diff --git a/t/t5330-no-lazy-fetch-with-commit-graph.sh b/t/t5330-no-lazy-fetch-with-commit-graph.sh index be33334229..2cc7fd7a47 100755 --- a/t/t5330-no-lazy-fetch-with-commit-graph.sh +++ b/t/t5330-no-lazy-fetch-with-commit-graph.sh @@ -4,28 +4,6 @@ test_description='test for no lazy fetch with the commit-graph' . ./test-lib.sh -run_with_limited_processses () { - # bash and ksh use "ulimit -u", dash uses "ulimit -p" - if test -n "$BASH_VERSION" - then - ulimit_max_process="-u" - elif test -n "$KSH_VERSION" - then - ulimit_max_process="-u" - fi - (ulimit ${ulimit_max_process-"-p"} 512 && "$@") -} - -test_lazy_prereq ULIMIT_PROCESSES ' - run_with_limited_processses true -' - -if ! test_have_prereq ULIMIT_PROCESSES -then - skip_all='skipping tests for no lazy fetch with the commit-graph, ulimit processes not available' - test_done -fi - test_expect_success 'setup: prepare a repository with a commit' ' git init with-commit && test_commit -C with-commit the-commit && @@ -59,8 +37,7 @@ test_expect_success 'fetch any commit from promisor with the usage of the commit git -C with-commit-graph config remote.origin.partialclonefilter blob:none && test_commit -C with-commit any-commit && anycommit=$(git -C with-commit rev-parse HEAD) && - - run_with_limited_processses env GIT_TRACE="$(pwd)/trace.txt" \ + GIT_TRACE="$(pwd)/trace.txt" \ git -C with-commit-graph fetch origin $anycommit 2>err && ! grep "fatal: promisor-remote: unable to fork off fetch subprocess" err && grep "git fetch origin" trace.txt >actual &&