Annoyed with the interference of the python formatting of
generated code (see #72964), I took matters into my own hands
as maintainer of dockerTools.
Afterwards, I've created a PR, hoping to unstuck the discussion.
@aszlig took notice and thanks to his python ecosystem knowledge,
the testing efforts of @blaggacao and @Ma27, and a sense of
shared suffering and comraderie we were able to change the
situation for the better in #122201.
Now, we have a proper linter that actually helps contributors,
so it's time to turn it back on again.
I'm glad we could make it happen this quickly!
Thanks!
This reverts commit 4035049af3.
This switches the linting of the NixOS test driver script from Black
(which is a code *formatter*) to PyFlakes. Contrary to Black, which only
does formatting and a basic syntax check, PyFlakes actually performs
useful checks early on before spinning up VMs and evaluating the actual
test script and thus becomes actually useful in development rather than
an annoyance.
One of the reasons why Black has been an annoyance[1] was because it
assumed that the files that it's formatting aren't inlined inside
another programming language.
With NixOS VM tests however, we inline these Python scripts in the
testScript attribute. With some of them using string antiquotations,
things are getting rather ugly because Black now no longer formats
static code but generated code from Nix being used as a macro language.
This becomes especially annoying when an antiquotation contains an
option definition from the NixOS module system, since an unrelated
change might change its length or contents (eg. suddenly containing a
double quote) for which Black will report an error.
While issue #72964 has been sitting around for a while (and probably
annoyed everyone involved), nobody has actually proposed an
implementation until @roberth did a first pull request[2] yesterday
which added a "skipFormatter" attribute, which contrary to skipLint
silently disabled Black.
This has led to very legitimate opposition[3] from @flokli:
> As of now, this only adds an option that does exactly the same as the
> already existing one.
>
> black does more than linting, yes. Last September it was proposed to
> switch from black to to a more permissive (only-)linter.
>
> I don't think adding another option (skipFormatter) that currently
> does exactly the same as skipLint will help out of this confusion.
>
> IMHO, we should keep skipLint, but use a linter that doesn't format,
> at least not enforce the line length (due to the nix interpolation we
> do).
This was written while I was doing an alternative implementation and
pretty much sums up the work I'm merging here, which switches to
PyFlakes, which only checks for various errors in the code (eg.
undefined variables, shadowing, wrong comparisons and more) but does not
do *any* formatting.
Since Black didn't do any of the checks performed by PyFlakes (except a
basic syntax check), the existing test scripts needed to be fixed.
Thanks to @blaggacao, @Ma27 and @roberth for helping with testing and
fixing those scripts.
[1]: https://github.com/NixOS/nixpkgs/issues/72964
[2]: https://github.com/NixOS/nixpkgs/pull/122197
[3]: https://github.com/NixOS/nixpkgs/pull/122197#pullrequestreview-654997723
There were a bunch of unnecessary f-strings in there and I also removed
the "# fmt: on/off" comments, because we no longer use Black and thus
won't need those comments anymore.
Signed-off-by: aszlig <aszlig@nix.build>
The new linter basically does
def testScript
# ...
before calling `pyflakes`. As this test-script is empty, it would lead
to a syntax-error unless `pass` is added.
