Re: [PATCH v2 1/2] t5510-fetch.sh: demonstrate fetch.writeCommitGraph bug
From: SZEDER Gábor <hidden>
Date: 2019-10-23 14:18:22
On Wed, Oct 23, 2019 at 01:01:34PM +0000, Derrick Stolee via GitGitGadget wrote:
From: Derrick Stolee <redacted> While dogfooding, Johannes found a bug in the fetch.writeCommitGraph config behavior. His example initially happened during a clone with --recurse-submodules, we found that this happens with the first fetch after cloning a repository that contains a submodule: $ git clone <url> test $ cd test $ git -c fetch.writeCommitGraph=true fetch origin Computing commit graph generation numbers: 100% (12/12), done. BUG: commit-graph.c:886: missing parent <hash1> for commit <hash2> Aborted (core dumped) In the repo I had cloned, there were really 60 commits to scan, but only 12 were in the list to write when calling compute_generation_numbers(). A commit in the list expects to see a parent, but that parent is not in the list. A follow-up will fix the bug, but first we create a test that demonstrates the problem. I used "test_expect_failure" for the entire test instead of "test_must_fail" only on the command that I expect to fail. This is because the BUG() returns an exit code so test_must_fail complains.
I don't think this paragraph is necessary; using 'test_expect_failure' is the way to demonstrate a known breakage. 'test_must_fail' should only be used when the failure is the desired behavior of a git command. (I used the word "desired" here, because you just used the word "expect" above in the sense that "I expect it to fail, because I know it's buggy, and I want to demonstrate that bug")
quoted hunk ↗ jump to hunk
Helped-by: Jeff King [off-list ref] Helped-by: Johannes Schindelin [off-list ref] Helped-by: Szeder Gábor [off-list ref] Signed-off-by: Derrick Stolee <redacted> --- t/t5510-fetch.sh | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index ecabbe1616..e8ae3af0b6 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh@@ -583,6 +583,23 @@ test_expect_success 'fetch.writeCommitGraph' ' ) ' +test_expect_failure 'fetch.writeCommitGraph with submodules' ' + pwd="$(pwd)" && + git clone dups super && + ( + cd super && + git submodule add "file://$pwd/three" &&
Nits: couldn't the URL simply be file://$TRASH_DIRECTORY/three ?
+ git commit -m "add submodule" + ) && + git clone "super" writeError &&
There is a write error now, because we have a bug, but after the next patch the bug will be fixed and we won't have a write error anymore.
+ (
+ cd writeError &&
+ test_path_is_missing .git/objects/info/commit-graphs/commit-graph-chain &&
+ git -c fetch.writeCommitGraph=true fetch origin &&
+ test_path_is_file .git/objects/info/commit-graphs/commit-graph-chain
+ )
+'
+
# configured prune tests
set_config_tristate () {
--
gitgitgadget