On 13.12.2021 11:02, Eric Sunshine wrote:
On Mon, Dec 13, 2021 at 10:43 AM Fabian Stelzer <fs@xxxxxxxxxxxx> wrote:
Personally i find the initial for loop variant to be the most readable.
Ævars makefile targets could be very nice too, but especially:
+$(BUILT_CHAINLINTTESTS): | .build/chainlint
+$(BUILT_CHAINLINTTESTS): .build/%.actual: %
+ $(CHAINLINT) <$< | \
+ sed -e '/^# LINT: /d' >$@ && \
+ diff -u $(basename $<).expect $@
i find very hard to grasp :/
I have no idea what is going on here: `<$< |` ?
Ya, that line-noise is an unfortunate combination of shell and
Makefile gobbledygook. The `$<` is effectively the source file (the
file being passed into chainlint.sed), and the rest of it is just
normal shell. `<` is redirection (using the source file `$<` as
stdin), and `|` is the pipe operator (sending the output of
chainlint.sed to another `sed`), and capturing it all via shell `>`
redirection in `$@` which is the Makefile variable for the target
file.
Thanks, that explains it nicely. I'm not familiar enough with makefile
syntax.
Anyhow, although the commit message tries to sell this change as some
sort of optimization, it's really in preparation for the new chainlint
which wants to check all tests in all files with a single invocation
rather than being invoked over and over and over. The self-test files
also require more preprocessing to work with the new chainlint, so the
implementation of `check-chainlint` gets rather more complex once the
end state is reached. I'll think about it a bit, but at the moment,
I'm still leaning toward this intermediate step as being beneficial to
reaching the end state. However, my opinion could change since the way
this is done here was probably influenced by an earlier iteration of
the new chainlint, but now that the implementation of the new
chainlint is concrete, it may not be especially important to do it
this way.
I don't mind much either way and i tend to favor the efficient variant as
well. On the other hand i can also see some bug mixing up the contents of
these files producing a huge diff with no good indication for the developer
of what has happened.
