build(lint): use local ruff executable

[sisp]

This PR contains a few related commits regarding Ruff and pre-commit. Before, Ruff was a Nix dependency and configured as a pre-commit hook via git-hooks.nix. This setup has been a bit problematic:

1.Ruff was either not available to non-Nix users or they were using a different version than the one locked by Nix (e.g. via the VS Code extension).

1. Dependabot cannot update Nix dependencies, so we tend to use an outdated Ruff version.
2. The Nix-locked Ruff version is not obvious, as Nix bundles its packages under a distribution release.

To address these problems and improve our (contributors') developer experience, I've made Ruff a regular Python development dependency managed via Poetry and configured a custom hook in flake.nix which uses the local ruff executable installed via Poetry. In a separate commit, I've added a similar custom hook for running Ruff's formatter. The custom hook definitions are based on Ruff's official pre-commit hook definitions and Ruff's own use thereof. In another separate commit, I've run Ruff's linter and formatter on our code base.

I think this setup is better calibrated to ease contributions also by non-Nix users while retaining the advantages of Nix.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.47%. Comparing base (4ec9757) to head (4adf640).

❗ Current head 4adf640 differs from pull request most recent head 90c9cd0

Please upload reports for the commit 90c9cd0 to get more accurate results.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1641      +/-   ##
==========================================
+ Coverage   97.32%   97.47%   +0.15%     
==========================================
  Files          49       49              
  Lines        5004     4998       -6     
==========================================
+ Hits         4870     4872       +2     
+ Misses        134      126       -8     

Flag	Coverage Δ
unittests	97.47% <100.00%> (+0.15%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[sisp]

@yajo I'm a bit stuck here, probably because I don't have enough Nix experience. I can't seem to get the custom pre-commit hooks to find the ruff executable installed by Poetry into its virtual env. I wonder whether pre-commit hooks are executed in isolation where the environment variables exposed by devenv aren't available. Using a native Nix Ruff package via poetry2nix doesn't seem like a good alternative because poetry2nix needs a manual update with a hash for each Ruff version. See also https://github.com/nix-community/poetry2nix/blob/master/docs/edgecases.md#errors-that-are-related-to-rust-and-cargo and the subsection "Use preferWheels and call it a day".

Any ideas how to make this work? And WDYT in general about this idea?

[yajo]

I don't think this will really fix the problem. With this patch, users still need nix to develop as it uses it to configure pre-commit, and thus there's not much benefit to package ruff with poetry. It would be much easier to install https://github.com/marketplace/actions/update-nix-flake-lock and have updated dev env automatically.

FWIW I configured here the stable release of nixpkgs, which is released every 6 months more or less. That's why ruff is so outdated. We could switch to the unstable release where the latest ruff is already packaged. However dealing with unstabilities twice per year seemed easier to me, and having the very latest formatter doesn't make me lose my sleep. It would be easy to get all from stable and only ruff from unstable, if you think it's worth it.

Pre-commit hooks we use include prettier, taplo and alejandra, which poetry cannot handle as they are js and rust. Thus, to remove nix from the picture, we'd have to rely on pre-commit for installation. And that part of pre-commit is doomed. I collected 21 public issues so far here: OCA/oca-addons-repo-template#175 (comment); compare that with the close to zero maintenance current setup gives us here. I'd be happy to experiment with https://treefmt.com/ which removes that part and makes the rest much faster.

So, back to the basics, what's the problem here? Simply put, nix scares people or they use Windows. But we want frictionless motivated contributions right? We can just mark the pre-commit ci check as optional on PRs and add https://github.com/stefanzweifel/git-auto-commit-action as a post-merge action. We'd have more reformatting commits in our main history, but if someone comes and fixes a typo in our docs, they won't need to set up any dev env (not even with poetry) for their valid pr to be merged. WDYT?

[sisp]

With this change, users wouldn't need Nix to contribute properly formatted Python code, as Ruff would be a Python dev dependency installed via Poetry. Yes, they wouldn't have pre-commit and wouldn't be able to run pre-commit hooks without Nix, but most contributions change only Python code (or docs) and would benefit from having (the correct version of) Ruff available by simply executing poetry install --with dev. Without pre-commit, one can run Ruff manually or use an IDE like VS Code with the right extension.

Also, the current git-hooks.nix hook for Ruff doesn't include its formatter, so CI checks don't detect Ruff formatting errors. Sure, at some point the hook will be added there, but asking contributors to reformat their code although CI checks are passing isn't great.

I feel like Nix should be opt-in as much as possible for contributors, and I understand that pre-commit and some hooks can't be available without Nix – that's simply a limitation of a non-Nix dev setup here –, but pulling Ruff into Python space seems like a low-hanging high-impact change (plus we'd have up-to-date versions maintained by Dependabot).