Hi Taylor, On Mon, 14 Nov 2022, Taylor Blau wrote: > On Mon, Nov 14, 2022 at 02:06:52PM +0000, Johannes Schindelin via GitGitGadget wrote: > > From: Johannes Schindelin <johannes.schindelin@xxxxxx> > > > > It is more performant to run `git diff --no-index` than running the > > `mingw_test_cmp` code with MSYS2's Bash, i.e. the Bash that Git for > > Windows uses. And a lot more readable. > > > > The original reason why Git's test suite needs the `mingw_test_cmp` > > function at all (and why `cmp` is not good enough) is that Git's test > > suite is not actually trying to compare binary files when it calls > > `test_cmp`, but it compares text files. And those text files can contain > > CR/LF line endings depending on the circumstances. > > > > Note: The original fix in the Git for Windows project implemented a test > > helper that avoids the overhead of the diff machinery, in favor of > > implementing a behavior that is more in line with what `mingw_test_cmp` > > does now. This was done to minimize the risk in using something as > > complex as the diff machinery to perform something as simple as > > determining whether text output is identical to the expected output or > > not. This approach has served Git for Windows well for years, but the > > attempt to upstream this saw a lot of backlash and distractions during > > the review, was disliked by the Git maintainer and was therefore > > abandoned. For full details, see the thread at > > https://lore.kernel.org/git/pull.1309.git.1659106382128.gitgitgadget@xxxxxxxxx/t > > In the previous round, you wrote that this paragraph: > > It explains why we replace a relatively simple logic with a relatively > complex one. Yes. And I obviously got myself misunderstood. The simple logic I refer to is the `t/helper/test-cmp.c` that I abandoned. The complex logic is the diff machinery we now call, which is a lot more involved and goes through a lot more code paths. > But I'm not sure the rewritten version does what you claim, at least in > my own personal opinion. > > It is not helpful to say the original approach "saw a lot of backlash". It is the nicest thing I can say about it. And I want people who are curious what happened to Git for Windows' custom code to have an explanation in the commit messages. > It is helpful, on the other hand, to say what about the original > approach gave reviewers pause, and why that alternative approach isn't > pursued here. > > Maybe I'm splitting hairs here, but I really do stand by my original > suggestion from back in [1]. We can also keep hitting a dead horse, but I don't think that will make anything any better. Thanks, Johannes > > Thanks, > Taylor > > [1]: https://lore.kernel.org/git/Y3B36HjDJhIY5jNz@nand.local/ >
