On Mon, Dec 13, 2021 at 11:17 AM Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> wrote: > On Mon, Dec 13 2021, Eric Sunshine wrote: > > On Mon, Dec 13, 2021 at 10:43 AM Fabian Stelzer <fs@xxxxxxxxxxxx> wrote: > >> 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. > > To add to that; > https://www.gnu.org/software/make/manual/html_node/Rules.html#Rules and > other relevant parts of the GNU make manual are very helpful here. And the Makefile variables $< and $@, in particular, are documented here: https://www.gnu.org/software/make/manual/html_node/Automatic-Variables.html > I don't really care about the details of whether it's invoked once or N > times, although I think the N times with proper dependencies tends to > give you better error messages, but maybe you'll be changing it > significantly enough that the current map between chainlint files and > approximately what sort of thing they check won't be there anymore. > > In any case, I'd think that a rule that used $< now (i.e. 1=1 file->out > prereq) would be better for the current state, and could just be changed > to use one of $^ or $+ later. > > I.e. you can declare a "check.done" that depends on {1..10}.todo, and > get a list of all of those {1..10}.todo files if one changes, or just > the ones whose mtime is newer than a "check.done". > > The reason I looked at this to begin with is that it takes it ~100-150ms > to run now, which adds up if you're e.g. using "make test T=<glob>" in > "git rebase -i --exec". Regarding this last point, one idea I strongly considered (and have not rejected) is to stop making `check-chainlin` a dependency of `test` and `prove`. Unlike most of the test suite, in which a change to any part of the Git source code could potentially cause any test to fail -- thus, it is important to run the full test suite for any source code change -- the `check-chainlint` target is completely isolated from everything else; it only checks whether `chainlint` itself functions correctly. The only time it really makes sense to run `check-chainlint` is when chainlint itself is changed in order to verify that it still functions as expected. Considering how infrequently (i.e. never) chainlint is modified, it seems somewhat silly for every `make test` or `make prove` invoked by anybody anywhere to repeatedly and forever validate chainlint[*]. Instead, it could be the responsibility of the person modifying chainlint to run the `check-chainlint` self-tests. [*]: There is at least one exception. Various implementations of `sed` could behave differently, thus impacting the behavior of chainlint.sed. This is not just a theoretical concern. I did all the development of this series on macOS, where everything worked as intended. Shortly before sending the series to the list, I subjected it to other platforms via CI and found that it failed on Linux due to minor behavioral differences in `sed` on Linux (though, very oddly, it worked just fine on Windows). I might not have caught this problem if `check-chainlint` had not been run automatically by `make test`.
