Re: [PATCH v5 2/2] tests(mingw): avoid very slow `mingw_test_cmp`
- From
Taylor Blau <me@ttaylorr.com>
- Date
- Dec 7, 2022, 01:31 UTC
- Message-ID
- <Y4/s1QcVFjjm31n7@nand.local>
- In-Reply-To
- <6a80fab7e3936ec56e1583d6136d47487327e907.1670339267.git.gitgitgadget@gmail.com>
Hi Johannes,
On Tue, Dec 06, 2022 at 03:07:47PM +0000, Johannes Schindelin via GitGitGadget wrote:
Show 30 quoted lines
> Learning the lesson to avoid shell scripting wherever possible, the Git > for Windows project implemented a minimal replacement for > `mingw_test_cmp` in the form of a `test-tool` subcommand that parses the > input files line by line, ignoring line endings, and compares them. > Essentially the same thing as `mingw_test_cmp`, but implemented in > C instead of Bash. This solution served the Git for Windows project > well, over years. > > However, when this solution was finally upstreamed, the conclusion was > reached that a change to use `git diff --no-index` instead of > `mingw_test_cmp` was more easily reviewed and hence should be used > instead. > > The reason why this approach was not even considered in Git for Windows > is that in 2007, there was already a motion on the table to use Git's > own diff machinery to perform comparisons in Git's test suite, but it > was dismissed in https://lore.kernel.org/git/xmqqbkrpo9or.fsf@gitster.g/ > as undesirable because tests might potentially succeed due to bugs in > the diff machinery when they should not succeed, and those bugs could > therefore hide regressions that the tests try to prevent. > > By the time Git for Windows' `mingw-test-cmp` in C was finally > contributed to the Git mailing list, reviewers agreed that the diff > machinery had matured enough and should be used instead. > > When the concern was raised that the diff machinery, due to its > complexity, would perform substantially worse than the test helper > originally implemented in the Git for Windows project, a test > demonstrated that these performance differences are well lost within the > 100+ minutes it takes to run Git's test suite on Windows.
Thanks for this update. I think these five paragraphs are an accurate and helpful record of the history behind this patch and approach. It is very much appreciated.
Thanks, Taylor