On Wed, Dec 13, 2023 at 10:22:48PM -0500, Eric Sunshine wrote:

From: Eric Sunshine <redacted>

The "check-chainlint" target runs automatically when running tests and
performs self-checks to verify that the chainlinter itself produces the
expected output. Originally, the chainlinter was implemented via sed,
but the infrastructure has been rewritten in fb41727b7e (t: retire
unused chainlint.sed, 2022-09-01) to use a Perl script instead.

The rewrite caused some slight whitespace changes in the output that are
ultimately not of much importance. In order to be able to assert that
the actual chainlinter errors match our expectations we thus have to
ignore whitespace characters when diffing them. As the `-w` flag is not
in POSIX we try to use `git diff -w --no-index` before we fall back to
non-standard `diff -w -u`.

To accommodate for cases where the host system has no Git installation
we use the locally-compiled version of Git. This can result in problems
though when the Git project's repository is using extensions that the
locally-compiled version of Git doesn't understand, in which case `git`
may refuse to run and thus cause the checks to fail.

Work around this issue by normalizing whitespace via sed before invoking
diff, which allows any platform diff implementation to be used, thus
eliminating the dependency upon `git diff` and the non-POSIX `-w` flag.

Reported-by: Patrick Steinhardt <redacted>
Signed-off-by: Eric Sunshine <redacted>
---

This is an alternative solution to the issue Patrick's patch[1]
addresses. Hopefully, this approach should avoid the sort of push-back
Patrick's patch received[2].

Thanks for chiming in!

I shamelessly stole most of Patrick's commit message.

The sed expressions for normalizing whitespace prior to `diff` may look
a bit hairy, but they are simple enough in concept:

* collapse runs of whitespace to a single SP
* drop blank lines (this step is not new)
* fold out possible SP at beginning and end of each line
* fold out SP surrounding common punctuation characters used in shell
  scripts, such as `>`, `|`, `;`, etc.

By the way, I'm somewhat surprised that this issue crops up at all
considering that --no-index is being used with git-diff. As such, I
would have thought that the local repository's format would not have
been interrogated at all. If that's a bug in `git diff --no-index`, then
fixing that could be considered yet another alternative solution to the
issue raised here.

This strongly reminds me of the thread at [1], where a similar issue was
discussed for git-grep(1). Quoting Junio: 

I actually do not think these "we are allowing Git tools to be used
on random garbage" is a good idea to begin with X-<.  If we invented
something nice for our variant in "git grep" and wish we can use it
outside the repository, contributing the feature to implementations
of "grep" would have been the right way to move forward, instead of
contaminating the codebase with things that are not related to Git.

So this might not be the best way to go.

These sed expressions do look hairy indeed. I have to wonder: all that
we're doing here is to munge the expected files we already have in our
tree. Can't we fix those to look exactly like the actual results instead
and then avoid any kind of post processing altogether? If I understand
correctly the only reason we do this post processing is because the
original implementation of the chainlinter produced slightly different
whitespace.

Patrick

[1]: https://lore.kernel.org/git/xmqq7cnnpy3z.fsf@gitster.g/ (local)