Errors reported only from the first failing linter #316

jaen · 2024-06-11T11:19:38Z

Describe the bug

I decided to try using go-based treefmt to see if it doesn't have issues with formatting hidden files and I noticed that while it now works with hidden files, it seems to report failures for only the first formatter that fails.

To Reproduce

Steps to reproduce the behaviour:

use this contrived treefmt.toml (this was generated by treefmt-nix, so you'd have to adjust paths for your system):

[formatter]
[formatter.actionlint]
command =  "/nix/store/r9h133c9m8f6jnlsqzwf89zg9w0w78s8-bash-5.2-p15/bin/bash"
excludes = []
includes = [".github/workflows/*.yml", "./github/workflows/*.yml"]
options = ["-eucx", "sleep 2; /nix/store/gksvgsiwzwc69lm7rm54wgfll26c2dyp-actionlint-1.6.26/bin/actionlint"]

[formatter.ruff-check]
command = "/nix/store/r9h133c9m8f6jnlsqzwf89zg9w0w78s8-bash-5.2-p15/bin/bash"
excludes = ["apks/chromium/*"]
includes = ["*.py", "*.pyi"]
options = ["-eucx", "sleep 1; /nix/store/372fyydc1xp2mjj1wwd8n28qlp0fp5dg-ruff-0.1.6/bin/ruff check ."]

run treefmt and observe you see errors from ruff only,
switch the sleep durations around,
run treefmt and observe you see errors from actionlint only.

Expected behaviour

Errors from all failing formatters/linters are reported.

System information

I used nixpkgs-unstable#treefmt2 which at this point in time points to 2.0.0-rc4.

The text was updated successfully, but these errors were encountered:

jaen · 2024-06-11T12:24:16Z

After some more experimentation I've discovered those two things:

when one of the formatters is shfmt it seems to leave some intermediary files (e.g. .update.sh2364291851 for update.sh) non-deterministically. This might mean that other formatters have their processes killed too early when another fails (or might be a separate issue),

after some further experimentation, it seems that is in fact the case – only the first formatter to finish using the contrived config below outputs it's .done.formatter file, the other are killed as soon as sleep is finished (because it's blocking, I guess):

[formatter]
[formatter.actionlint]
command =  "/nix/store/r9h133c9m8f6jnlsqzwf89zg9w0w78s8-bash-5.2-p15/bin/bash"
excludes = []
includes = [".github/workflows/*.yml", "./github/workflows/*.yml"]
options = ["-eucx", "shopt -s globstar; sleep 1 && /nix/store/gksvgsiwzwc69lm7rm54wgfll26c2dyp-actionlint-1.6.26/bin/actionlint .github/workflows/*.yml || export exitcode=$? && date +%s > .done.actionlint && echo exitcode=${exitcode:-0}  >> .done.actionlint && exit ${exitcode:-0}"]

[formatter.ruff-check]
command = "/nix/store/r9h133c9m8f6jnlsqzwf89zg9w0w78s8-bash-5.2-p15/bin/bash"
excludes = ["apks/chromium/*"]
includes = ["*.py", "*.pyi"]
options = ["-eucx", "shopt -s globstar; sleep 2 && /nix/store/372fyydc1xp2mjj1wwd8n28qlp0fp5dg-ruff-0.1.6/bin/ruff check **/*.py || export exitcode=$? && date +%s > .done.ruff-check && echo exitcode=${exitcode:-0} >> .done.ruff-check && exit ${exitcode:-0}"]

[formatter.shfmt]
command = "/nix/store/r9h133c9m8f6jnlsqzwf89zg9w0w78s8-bash-5.2-p15/bin/bash"
excludes = []
includes = ["*.sh"]
options = ["-eucx", "shopt -s globstar; sleep 3 && /nix/store/29ch5nlxyj31v36syqsl9ylgn6h6p9yb-shfmt-3.7.0/bin/shfmt -i 2 -s -w **/*.sh || export exitcode=$? && date +%s > .done.shfmt && echo exitcode=${exitcode:-0} >> .done.shfmt && exit ${exitcode:-0}"]

[formatter.nixpkgs-fmt]
command = "/nix/store/r9h133c9m8f6jnlsqzwf89zg9w0w78s8-bash-5.2-p15/bin/bash"
excludes = []
includes = ["*.nix"]
options = ["-eucx", "shopt -s globstar; sleep 10 && /nix/store/7714qjhzws6rcq5227w1859g3kl5ma62-nixfmt-unstable-2024-05-28/bin/nixfmt . || export exitcode=$? && date +%s > .done.nixpkgs-fmt && echo exitcode=${exitcode:-0} >> .done.nixpkgs-fmt && exit ${exitcode:-0}"]

brianmcgee · 2024-06-11T12:37:11Z

The errgroup will cancel any outstanding tasks on first error. As you've described, it's currently designed to abort when the first formatter fails. At the very least this should be more graceful, allowing any formatters in flight to complete rather than killing them. I think I'm passing the context into exec, perhaps I shouldn't be.

I never considered letting all the formatting run it's course, reporting errors as they occur on a best-effort basis. I'd like to think through the implications of that a bit more before making a change like this.

@zimbatm I'd like to get your input too.

jaen · 2024-06-11T13:21:44Z

FWIW as far as I can tell, the Rust-based treefmt runs all formatters/linters to completion and returns all errors — at least in the actual setup I get failures for both mypy and ruff-check in a single run and all the other files get formatted as well.

So at the very least keeping this behaviour will be a breaking workflow change to users of the current treefmt version. I don't particularly care as I've only started trying treefmt out (though I think the existing behaviour is better, personally), but existing users might.

brianmcgee · 2024-06-11T13:25:05Z

To be honest that settles it then.

One of the primary aims with 2.0 is to avoid breaking anything for existing users. We can diverge in behaviour and feature set in 2.{1-} onwards. So if treefmt runs everything to completion and spits out all errors, then the go version needs to as well.

jaen · 2024-06-15T19:11:31Z

Seems to work better now, thanks!

jaen · 2024-06-17T18:52:47Z

@brianmcgee ok, looks like it's not quite perfect – if you take the code from the PR linked above, reset to the first commit (the one adding treefmt config only) and run treefmt --no-cache then Python files won't get formatted with Ruff. But when I comment out the Ruff lint configuration, they will suddenly get formatted (I assume it might be due to sharing the formatter exectutable, but didn't test that assumption yet).

So it seems there's still some cancelling going on. Not sure if you want to re-open this, treat it as a separate issue or need a simpler reproducer.

brianmcgee · 2024-06-18T08:23:11Z

@jaen re-opened this issue so we retain all the context.

@zimbatm can you have a look?

jaen · 2024-07-22T11:22:19Z

@brianmcgee @zimbatm any update on plans with regard to this? I'm not in a very big rush here, but would be helpful to know where this is going.

brianmcgee · 2024-07-22T11:24:25Z

@jaen I haven't had any time to look at it yet, but I've made a note to look at it later this week when I should have some time.

brianmcgee · 2024-07-29T09:11:16Z

@jaen I tried the steps above, and I don't think I'm seeing what you were seeing:

❯ nix fmt -- --no-cache 
ruff-check error:
flavors/lineageos/update_device_dirs.py:196:82: E703 [*] Statement ends with an unnecessary semicolon
flavors/lineageos/update_device_metadata.py:7:8: F401 [*] `urllib.request` imported but unused
Found 2 errors.
[*] 2 fixable with the `--fix` option.

actionlint error:
.github/workflows/grapheneos-update.yml:16:7: shellcheck reported issue in this script: SC2086:info:5:52: Double quote to prevent globbing and word splitting [shellcheck]
   |
16 |       run: |
   |       ^~~~
.github/workflows/grapheneos-update.yml:16:7: shellcheck reported issue in this script: SC2086:info:10:52: Double quote to prevent globbing and word splitting [shellcheck]
   |
16 |       run: |
   |       ^~~~
.github/workflows/instantiate.yml:17:7: shellcheck reported issue in this script: SC2046:warning:1:98: Quote this to prevent word splitting [shellcheck]
   |
17 |     - run: |
   |       ^~~~
.github/workflows/instantiate.yml:17:7: shellcheck reported issue in this script: SC2071:error:3:32: > is for string comparisons. Use -gt instead [shellcheck]
   |
17 |     - run: |
   |       ^~~~
.github/workflows/instantiate.yml:26:7: shellcheck reported issue in this script: SC2086:info:4:8: Double quote to prevent globbing and word splitting [shellcheck]
   |
26 |     - run: |
   |       ^~~~
.github/workflows/instantiate.yml:56:7: shellcheck reported issue in this script: SC2046:warning:3:4: Quote this to prevent word splitting [shellcheck]
   |
56 |     - run: |
   |       ^~~~
.github/workflows/instantiate.yml:56:7: shellcheck reported issue in this script: SC2086:info:4:19: Double quote to prevent globbing and word splitting [shellcheck]
   |
56 |     - run: |
   |       ^~~~
.github/workflows/instantiate.yml:56:7: shellcheck reported issue in this script: SC2086:info:5:43: Double quote to prevent globbing and word splitting [shellcheck]
   |
56 |     - run: |
   |       ^~~~

robotnix on  HEAD (d8a35e1) [!?] on ☁️   took 1m1s 
❯ nix fmt -- --no-cache
robotnix on  HEAD (d8a35e1) [!?] on ☁️   took 1m1s 
❯ nix fmt -- --no-cache 
WARN format: no formatter for path: flavors/lineageos/update_device_dirs.py
WARN format: no formatter for path: flavors/lineageos/update_device_metadata.py
WARN format: no formatter for path: flavors/waydroid/extract-patch-metadata.py
WARN format: no formatter for path: modules/apps/chromium-trichrome-patcher.py
WARN format: no formatter for path: modules/apv/autogenerate.py
WARN format: no formatter for path: scripts/mk_repo_file.py
WARN format: no formatter for path: scripts/robotnix_common.py
WARN format: no formatter for path: scripts/test_mk_repo_file.py
actionlint error:
.github/workflows/grapheneos-update.yml:16:7: shellcheck reported issue in this script: SC2086:info:5:52: Double quote to prevent globbing and word splitting [shellcheck]
   |
16 |       run: |
   |       ^~~~
.github/workflows/grapheneos-update.yml:16:7: shellcheck reported issue in this script: SC2086:info:10:52: Double quote to prevent globbing and word splitting [shellcheck]
   |
16 |       run: |
   |       ^~~~
.github/workflows/instantiate.yml:17:7: shellcheck reported issue in this script: SC2046:warning:1:98: Quote this to prevent word splitting [shellcheck]
   |
17 |     - run: |
   |       ^~~~
.github/workflows/instantiate.yml:17:7: shellcheck reported issue in this script: SC2071:error:3:32: > is for string comparisons. Use -gt instead [shellcheck]
   |
17 |     - run: |
   |       ^~~~
.github/workflows/instantiate.yml:26:7: shellcheck reported issue in this script: SC2086:info:4:8: Double quote to prevent globbing and word splitting [shellcheck]
   |
26 |     - run: |
   |       ^~~~
.github/workflows/instantiate.yml:56:7: shellcheck reported issue in this script: SC2046:warning:3:4: Quote this to prevent word splitting [shellcheck]
   |
56 |     - run: |
   |       ^~~~
.github/workflows/instantiate.yml:56:7: shellcheck reported issue in this script: SC2086:info:4:19: Double quote to prevent globbing and word splitting [shellcheck]
   |
56 |     - run: |
   |       ^~~~
.github/workflows/instantiate.yml:56:7: shellcheck reported issue in this script: SC2086:info:5:43: Double quote to prevent globbing and word splitting [shellcheck]
   |
56 |     - run: |
   |       ^~~~

treefmt: error: formatting failure: formatter '/nix/store/gksvgsiwzwc69lm7rm54wgfll26c2dyp-actionlint-1.6.26/bin/actionlint' with options '[]' failed to apply: exit status 1

I think you were right and this needs a simpler reproducer.

jaen · 2024-07-29T09:34:45Z

Okay, I'll try to look at it in the evening

jaen · 2024-07-29T19:32:02Z

Ok, so here are the exact steps I used with my branch to reproduce it:

╭─jaen@glados ~/Projects/homelab/robotnix2 <HEAD>
╰─$ git switch --detach d8a35e1812a9dde788bfdd65be171c73ab217fd6
HEAD is now at d8a35e1 flake: add treefmt

╭─jaen@glados ~/Projects/homelab/robotnix2 <HEAD>
╰─$ nix fmt -- --no-cache
ruff-check error:
flavors/lineageos/update_device_dirs.py:196:82: E703 [*] Statement ends with an unnecessary semicolon
flavors/lineageos/update_device_metadata.py:7:8: F401 [*] `urllib.request` imported but unused
Found 2 errors.
[*] 2 fixable with the `--fix` option.

actionlint error:
.github/workflows/grapheneos-update.yml:16:7: shellcheck reported issue in this script: SC2086:info:5:52: Double quote to prevent globbing and word splitting [shellcheck]
   |
16 |       run: |
   |       ^~~~
.github/workflows/grapheneos-update.yml:16:7: shellcheck reported issue in this script: SC2086:info:10:52: Double quote to prevent globbing and word splitting [shellcheck]
   |
16 |       run: |
   |       ^~~~
.github/workflows/instantiate.yml:17:7: shellcheck reported issue in this script: SC2046:warning:1:98: Quote this to prevent word splitting [shellcheck]
   |
17 |     - run: |
   |       ^~~~
.github/workflows/instantiate.yml:17:7: shellcheck reported issue in this script: SC2071:error:3:32: > is for string comparisons. Use -gt instead [shellcheck]
   |
17 |     - run: |
   |       ^~~~
.github/workflows/instantiate.yml:26:7: shellcheck reported issue in this script: SC2086:info:4:8: Double quote to prevent globbing and word splitting [shellcheck]
   |
26 |     - run: |
   |       ^~~~
.github/workflows/instantiate.yml:56:7: shellcheck reported issue in this script: SC2046:warning:3:4: Quote this to prevent word splitting [shellcheck]
   |
56 |     - run: |
   |       ^~~~
.github/workflows/instantiate.yml:56:7: shellcheck reported issue in this script: SC2086:info:4:19: Double quote to prevent globbing and word splitting [shellcheck]
   |
56 |     - run: |
   |       ^~~~
.github/workflows/instantiate.yml:56:7: shellcheck reported issue in this script: SC2086:info:5:43: Double quote to prevent globbing and word splitting [shellcheck]
   |
56 |     - run: |
   |       ^~~~

treefmt: error: formatting failure: formatter '/nix/store/372fyydc1xp2mjj1wwd8n28qlp0fp5dg-ruff-0.1.6/bin/ruff' with options '[check]' failed to apply: exit status 1

╭─jaen@glados ~/Projects/homelab/robotnix2 <HEAD*>
╰─$ git status | grep py

╭─jaen@glados ~/Projects/homelab/robotnix2 <HEAD*>
╰─$ sed -i '/ruff.check = true;/s/^/#/g' treefmt.nix

╭─jaen@glados ~/Projects/homelab/robotnix2 <HEAD*>
╰─$ nix fmt -- --no-cache
warning: Git tree '/home/jaen/Projects/homelab/robotnix2' is dirty
actionlint error:
.github/workflows/grapheneos-update.yml:16:7: shellcheck reported issue in this script: SC2086:info:5:52: Double quote to prevent globbing and word splitting [shellcheck]
   |
16 |       run: |
   |       ^~~~
.github/workflows/grapheneos-update.yml:16:7: shellcheck reported issue in this script: SC2086:info:10:52: Double quote to prevent globbing and word splitting [shellcheck]
   |
16 |       run: |
   |       ^~~~
.github/workflows/instantiate.yml:17:7: shellcheck reported issue in this script: SC2046:warning:1:98: Quote this to prevent word splitting [shellcheck]
   |
17 |     - run: |
   |       ^~~~
.github/workflows/instantiate.yml:17:7: shellcheck reported issue in this script: SC2071:error:3:32: > is for string comparisons. Use -gt instead [shellcheck]
   |
17 |     - run: |
   |       ^~~~
.github/workflows/instantiate.yml:26:7: shellcheck reported issue in this script: SC2086:info:4:8: Double quote to prevent globbing and word splitting [shellcheck]
   |
26 |     - run: |
   |       ^~~~
.github/workflows/instantiate.yml:56:7: shellcheck reported issue in this script: SC2046:warning:3:4: Quote this to prevent word splitting [shellcheck]
   |
56 |     - run: |
   |       ^~~~
.github/workflows/instantiate.yml:56:7: shellcheck reported issue in this script: SC2086:info:4:19: Double quote to prevent globbing and word splitting [shellcheck]
   |
56 |     - run: |
   |       ^~~~
.github/workflows/instantiate.yml:56:7: shellcheck reported issue in this script: SC2086:info:5:43: Double quote to prevent globbing and word splitting [shellcheck]
   |
56 |     - run: |
   |       ^~~~

treefmt: error: formatting failure: formatter '/nix/store/gksvgsiwzwc69lm7rm54wgfll26c2dyp-actionlint-1.6.26/bin/actionlint' with options '[]' failed to apply: exit status 1

╭─jaen@glados ~/Projects/homelab/robotnix2 <HEAD*>
╰─$ git status | grep py
	modified:   flavors/lineageos/update_device_dirs.py
	modified:   flavors/lineageos/update_device_metadata.py
	modified:   flavors/waydroid/extract-patch-metadata.py
	modified:   modules/apps/chromium-trichrome-patcher.py
	modified:   modules/apv/autogenerate.py
	modified:   scripts/mk_repo_file.py
	modified:   scripts/robotnix_common.py
	modified:   scripts/test_mk_repo_file.py

As you can see, after the first run no python files are changed and after the second run (after the check is commented out) they get formatted.

But I can still try to make a simpler reproducer if that would be helpful.

brianmcgee · 2024-07-29T20:17:33Z

Thanks for clarifying the steps. Let me try this tomorrow and see how I get on before going to the trouble of a simpler reproducer.

jaen · 2024-07-29T20:39:50Z

Okay, got a simpler reproducer going: treefmt-fail.tar.gz
If you update treefmt-nix here it will stop happening (because ruff-check now has --fix flag by default), but I think it would still be beneficial to fix this — you can't be sure a formatter/linter never fails in general, I'd think.

brianmcgee · 2024-07-30T18:47:10Z

If I understand correctly, I think what you're seeing is an artefact of how treefmt batches paths for processing. I think this has changed since the last time we looked into this and wasn't taken into account.

For every path to be processed, we determine the set of formatters that match it. We then canonically sort those formatters and with that sorted list, create a batch key. This represents a unique sequence of execution for that file path.

We then batch paths to be processed based on that batch key, and when we reach enough, trigger a separate go routine to apply the sequence of formatters in the order they were defined (priority can afffect this ordering) to the list of paths.

In your example above, ruff-check and ruff-format match the same files and therefore are being executed in order, one after the other. Without any priority being specified, ruff-check is being executed first and fails, and we currently don't attempt to execute with ruff-format.

It should be straightforward enough to change things so that we keep on trucking through a pipeline and log out any errors we come across. I've made a start with #366, and will continue with it tomorrow. Haven't had a chance to test it yet as I'm seeing some strange behaviour locally with the dev setup which I need to figure out first.

This change effectively means all formatting is carried out as best we can across all batches, rather than exiting early when one batch fails with one formatter.

@zimbatm are there any obvious gotchas I'm missing? If I'm not mistaken, this would bring us back in line with how v1 was doing things?

jaen · 2024-07-30T21:39:34Z

I can see how sometimes a short-circuit behaviour would be beneficial and sometimes how completing pipeline till the end to report as much errors as feasible would be better. I think the latter is a better default, but maybe sometimes you would prefer to opt-in into the short-circuting behaviour? Not sure how feasible that would be to implement and how convenient to configure, with pipelines being implicit rather than explicit.

Just food for thought, really, because I don't feel too strongly about exact implementation other than "it would be better if we didn't have false positives in CI in the default configuration".

jaen · 2024-08-29T17:33:22Z

@brianmcgee any further plans in this regard?

brianmcgee · 2024-09-02T09:43:59Z

@jaen, none yet.

I've been on holiday and am just spinning back up. I intend to give treefmt some love these next couple of weeks.

I need to review this issue again, as I think it meandered a bit off-topic, and I think #365 and #368 addressed some of the problems reported.

jaen · 2024-09-03T12:11:36Z

FWIW just tested the reproducer with the treefmt input bumped to the current HEAD (2feb7cf6051f75e94dd7bf60a62e50240843a512) and it still reproduces, so it unfortunately seems like it's a different bug than those two ¯\_(ツ)_/¯

brianmcgee · 2024-09-03T12:53:48Z

I'll give the reproducer another bash and see if I can get to the bottom of what's happening.

brianmcgee · 2024-09-03T13:33:42Z

Okay, for whatever reason, I find it difficult to keep the context for this issue straight in my head.

After reviewing the history again, I see that I started #366, which should bring the behaviour back in line with v1. I'm going to spend some time now getting that PR finished (improving the logging a bit and adding a test).

brianmcgee · 2024-09-03T14:55:01Z

I spent some time improving #366.

Here's an updated version of the reproducer using it: treefmt-fail-fix.tar.gz

On the first application, you can see that ruff-check fails, but it proceeds with ruff-format. The file src/nix/sum.py is detected as having changed, however we do not update the cache, to ensure this file is re-processed on subsequent runs in case the problem was something like a config error which is easily fixed.

❯ nix fmt -- -vv 
DEBU format: config-file=/nix/store/0f4l3y7lkfcnqriahmdlh01xvcf7qby7-treefmt.toml tree-root=/home/brian/Downloads/treefmt-fail
DEBU cache: finished generating change set in 106.491µs
DEBU format | nixpkgs-fmt: match: /home/brian/Downloads/treefmt-fail/src/nix/sum.nix
DEBU format | ruff-check: match: /home/brian/Downloads/treefmt-fail/src/nix/sum.py
DEBU format | ruff-format: match: /home/brian/Downloads/treefmt-fail/src/nix/sum.py
DEBU format | ruff-check: executing: /nix/store/s9sn9llf7xk4hjac2d4phj5zha9gszyh-ruff-0.6.3/bin/ruff check src/nix/sum.py
DEBU format | nixpkgs-fmt: executing: /nix/store/vfmlh00mb40qarwj236vmkqylcnv5la5-nixfmt-unstable-2024-08-16/bin/nixfmt src/nix/sum.nix
ERRO format | ruff-check: failed to apply with options '[check]': exit status 1
ruff-check error:
src/nix/sum.py:10:8: F401 [*] `urlib.request` imported but unused
   |
 8 | import os
 9 | import pathlib
10 | import urlib.request
   |        ^^^^^^^^^^^^^ F401
11 | 
12 | from typing import Any, Callable, Dict, List, Optional, cast
   |
   = help: Remove unused import: `urlib.request`

src/nix/sum.py:166:54: E703 [*] Statement ends with an unnecessary semicolon
    |
164 |     device_dirs_fn = os.path.join(args.branch, 'device-dirs.json')
165 |     if os.path.exists(device_dirs_fn):
166 |         device_dirs = json.load(open(device_dirs_fn));
    |                                                      ^ E703
167 |     else:
168 |         device_dirs = {}
    |
    = help: Remove unnecessary semicolon

Found 2 errors.
[*] 2 fixable with the `--fix` option.

DEBU format | ruff-format: executing: /nix/store/s9sn9llf7xk4hjac2d4phj5zha9gszyh-ruff-0.6.3/bin/ruff format src/nix/sum.py
INFO format | ruff-format: 1 file(s) processed in 8.664883ms
DEBU format: file has changed path=src/nix/sum.py prev_size=6249 prev_mod_time="2024-09-03 15:39:26 +0100 IST" current_size=6206 current_mod_time="2024-09-03 15:42:46 +0100 IST"
INFO format | nixpkgs-fmt: 1 file(s) processed in 32.92821ms
DEBU format: file has changed path=src/nix/sum.nix prev_size=19626 prev_mod_time="2024-09-03 15:39:26 +0100 IST" current_size=21639 current_mod_time="2024-09-03 15:42:46 +0100 IST"
DEBU cache: finished processing 1 paths in 12.636895ms
traversed 4 files
emitted 2 files for processing
formatted 2 files (2 changed) in 46ms

Running it again yields:

❯ nix fmt -- -vv 
DEBU format: config-file=/nix/store/0f4l3y7lkfcnqriahmdlh01xvcf7qby7-treefmt.toml tree-root=/home/brian/Downloads/treefmt-fail
DEBU cache: finished generating change set in 137.501µs
DEBU format | ruff-check: match: /home/brian/Downloads/treefmt-fail/src/nix/sum.py
DEBU format | ruff-format: match: /home/brian/Downloads/treefmt-fail/src/nix/sum.py
DEBU format | ruff-check: executing: /nix/store/s9sn9llf7xk4hjac2d4phj5zha9gszyh-ruff-0.6.3/bin/ruff check src/nix/sum.py
ERRO format | ruff-check: failed to apply with options '[check]': exit status 1
ruff-check error:
src/nix/sum.py:10:8: F401 [*] `urlib.request` imported but unused
   |
 8 | import os
 9 | import pathlib
10 | import urlib.request
   |        ^^^^^^^^^^^^^ F401
11 | 
12 | from typing import Any, Callable, Dict, List, Optional, cast
   |
   = help: Remove unused import: `urlib.request`

Found 1 error.
[*] 1 fixable with the `--fix` option.

DEBU format | ruff-format: executing: /nix/store/s9sn9llf7xk4hjac2d4phj5zha9gszyh-ruff-0.6.3/bin/ruff format src/nix/sum.py
INFO format | ruff-format: 1 file(s) processed in 7.4775ms
DEBU cache: finished processing 0 paths in 610ns
traversed 4 files
emitted 1 files for processing
formatted 1 files (0 changed) in 18ms

ruff-check still fails, and ruff-format is still applied, but this time there are no changes detected as there was nothing for ruff-format to do.

If you then add this to flake.nix and run again:

settings.formatter.ruff-check.options = nixpkgs.lib.mkForce [
    "check"
    "--fix"
];

You see that the python file is re-processed, and check now passes:

❯ nix fmt -- -vv 
DEBU format: config-file=/nix/store/masnfl7cfhpv7jqk0qmx14zvzp65a4kl-treefmt.toml tree-root=/home/brian/Downloads/treefmt-fail
DEBU cache: finished generating change set in 148.382µs
DEBU format | nixpkgs-fmt: match: /home/brian/Downloads/treefmt-fail/flake.nix
DEBU format | ruff-format: match: /home/brian/Downloads/treefmt-fail/src/nix/sum.py
DEBU format | ruff-check: match: /home/brian/Downloads/treefmt-fail/src/nix/sum.py
DEBU format | ruff-check: executing: /nix/store/s9sn9llf7xk4hjac2d4phj5zha9gszyh-ruff-0.6.3/bin/ruff check --fix src/nix/sum.py
DEBU format | nixpkgs-fmt: executing: /nix/store/vfmlh00mb40qarwj236vmkqylcnv5la5-nixfmt-unstable-2024-08-16/bin/nixfmt flake.nix
INFO format | ruff-check: 1 file(s) processed in 10.150282ms
DEBU format | ruff-format: executing: /nix/store/s9sn9llf7xk4hjac2d4phj5zha9gszyh-ruff-0.6.3/bin/ruff format src/nix/sum.py
INFO format | nixpkgs-fmt: 1 file(s) processed in 12.435466ms
INFO format | ruff-format: 1 file(s) processed in 8.326802ms
DEBU format: file has changed path=src/nix/sum.py prev_size=6206 prev_mod_time="2024-09-03 15:50:37 +0100 IST" current_size=6185 current_mod_time="2024-09-03 15:50:49 +0100 IST"
DEBU cache: finished processing 2 paths in 12.401286ms
traversed 4 files
emitted 2 files for processing
formatted 2 files (1 changed) in 32ms

I still need to add a test before this PR is ready for merging. I'll try and get it wrapped up in the next couple days.

jaen · 2024-09-03T15:26:40Z

Ok, I've updated my local reproducer to the PR branch (independently, just in case) and I can also see that now formatting gets applied, even though the check fails. I'll try this later on my robotnix branch to see if that also fixes the issue there, but it looks promising, thanks!

jaen · 2024-09-04T10:54:59Z

@brianmcgee seems to work on the real repo as well. There's some weird corner case with some nix file being formatted differently the first and second time around, but maybe it's the formatter itself acting up or something. I'd consider the PR as fixing this issue and if anything else turns out to be off later, I'll just file a different issue.

brianmcgee · 2024-09-04T11:00:03Z

Ok cool, I'll find try and find some time to add a test or two and get this merged.

jaen added the bug Something isn't working label Jun 11, 2024

brianmcgee self-assigned this Jun 11, 2024

brianmcgee assigned zimbatm and unassigned brianmcgee Jun 11, 2024

zimbatm mentioned this issue Jun 15, 2024

Softer cancel #321

Merged

brianmcgee closed this as completed in #321 Jun 15, 2024

brianmcgee closed this as completed in c68261e Jun 15, 2024

jaen mentioned this issue Jun 15, 2024

Add treefmt and re-format codebase nix-community/robotnix#237

Open

brianmcgee reopened this Jun 18, 2024

This was referenced Aug 1, 2024

Inconsistent treefmt behavior with just: Intermittent file change reporting #365

Closed

CI flag #368

Merged

brianmcgee mentioned this issue Sep 3, 2024

apply all formatters even when there's errors #366

Merged

brianmcgee closed this as completed in #366 Sep 9, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Errors reported only from the first failing linter #316

Errors reported only from the first failing linter #316

jaen commented Jun 11, 2024 •

edited

Loading

jaen commented Jun 11, 2024

brianmcgee commented Jun 11, 2024

jaen commented Jun 11, 2024

brianmcgee commented Jun 11, 2024

jaen commented Jun 15, 2024

jaen commented Jun 17, 2024

brianmcgee commented Jun 18, 2024

jaen commented Jul 22, 2024

brianmcgee commented Jul 22, 2024

brianmcgee commented Jul 29, 2024

jaen commented Jul 29, 2024

jaen commented Jul 29, 2024 •

edited

Loading

brianmcgee commented Jul 29, 2024

jaen commented Jul 29, 2024 •

edited

Loading

brianmcgee commented Jul 30, 2024 •

edited

Loading

jaen commented Jul 30, 2024 •

edited

Loading

jaen commented Aug 29, 2024

brianmcgee commented Sep 2, 2024

jaen commented Sep 3, 2024 •

edited

Loading

brianmcgee commented Sep 3, 2024

brianmcgee commented Sep 3, 2024

brianmcgee commented Sep 3, 2024

jaen commented Sep 3, 2024

jaen commented Sep 4, 2024

brianmcgee commented Sep 4, 2024

Errors reported only from the first failing linter #316

Errors reported only from the first failing linter #316

Comments

jaen commented Jun 11, 2024 • edited Loading

jaen commented Jun 11, 2024

brianmcgee commented Jun 11, 2024

jaen commented Jun 11, 2024

brianmcgee commented Jun 11, 2024

jaen commented Jun 15, 2024

jaen commented Jun 17, 2024

brianmcgee commented Jun 18, 2024

jaen commented Jul 22, 2024

brianmcgee commented Jul 22, 2024

brianmcgee commented Jul 29, 2024

jaen commented Jul 29, 2024

jaen commented Jul 29, 2024 • edited Loading

brianmcgee commented Jul 29, 2024

jaen commented Jul 29, 2024 • edited Loading

brianmcgee commented Jul 30, 2024 • edited Loading

jaen commented Jul 30, 2024 • edited Loading

jaen commented Aug 29, 2024

brianmcgee commented Sep 2, 2024

jaen commented Sep 3, 2024 • edited Loading

brianmcgee commented Sep 3, 2024

brianmcgee commented Sep 3, 2024

brianmcgee commented Sep 3, 2024

jaen commented Sep 3, 2024

jaen commented Sep 4, 2024

brianmcgee commented Sep 4, 2024

jaen commented Jun 11, 2024 •

edited

Loading

jaen commented Jul 29, 2024 •

edited

Loading

jaen commented Jul 29, 2024 •

edited

Loading

brianmcgee commented Jul 30, 2024 •

edited

Loading

jaen commented Jul 30, 2024 •

edited

Loading

jaen commented Sep 3, 2024 •

edited

Loading