On Fri, Dec 15, 2023 at 1:04 AM Patrick Steinhardt <ps@xxxxxx> wrote:
> [...]
> Instead of improving the detection logic, fix our ".expect" files so
> that we do not need any post-processing at all anymore. This allows us
> to drop the `-w` flag when diffing so that we can always use diff(1)
> now.
>
> Note that we leave the post-processing of `chainlint.pl` output intact.
> All we do here is to strip leading line numbers that it would otherwise
> generate.
Hmm, okay, but... (see below)
> Having these would cause a rippling effect whenever we add a
> new test that sorts into the middle of existing tests and would require
> us to renumerate all subsequent lines, which seems rather pointless.
Just an aside, not strictly relevant at this time: Ævar has proposed
that check-chainlint should not be creating conglomerate "test",
"expect", and "actual" files, but should instead let `make` run
chainlint.pl separately on each chainlint self-test file, thus
benefiting from `make`'s innate parallelism rather than baking
parallelism into chainlint.pl itself. More importantly, `make`'s
dependency tracking would ensure that a chainlint self-test file only
gets rechecked if its timestamp changes. That differs from the current
situation in which _all_ of the chainlint self-test files are checked
on _every_ `make test` which is wasteful if none of them have changed.
Anyhow, with his proposed approach, there wouldn't be cascading line
number changes just because a new self-test file was added.
> Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
> ---
> diff --git a/t/Makefile b/t/Makefile
> @@ -103,20 +103,12 @@ check-chainlint:
> $(CHAINLINT) --emit-all '$(CHAINLINTTMP_SQ)'/tests | \
> sed -e 's/^[1-9][0-9]* //;/^[ ]*$$/d' >'$(CHAINLINTTMP_SQ)'/actual && \
The commit message claims that this is only stripping the line numbers
which prefix each emitted line, but the `/^[ ]*$$/d` bit is also
deleting blank lines from the output of chainlint.pl. Thus, this ought
to be:
sed -e 's/^[1-9][0-9]* //' >'$(CHAINLINTTMP_SQ)'/actual && \
