pants icon indicating copy to clipboard operation
pants copied to clipboard

Ruff backend showing up as `pants.core` since 2.19

Open sureshjoshi opened this issue 1 year ago • 7 comments

Good: https://www.pantsbuild.org/2.19/reference/subsystems/ruff

Bad: https://www.pantsbuild.org/2.20/reference/subsystems/ruff Screenshot 2024-05-30 at 18 59 14

sureshjoshi avatar May 30 '24 22:05 sureshjoshi

It looks like this regressed in https://github.com/pantsbuild/pants/releases/tag/release_2.20.0a0, based on comparing the output of PANTS_VERSION=2.20.0dev7 pants --backend-packages=pants.backend.experimental.python.lint.ruff help ruff (good) with that of PANTS_VERSION=2.20.0a0 pants --backend-packages=pants.backend.experimental.python.lint.ruff help ruff (bad).

Comparison: https://github.com/pantsbuild/pants/compare/release_2.20.0.dev7...release_2.20.0a0

I suspect it may be https://github.com/pantsbuild/pants/pull/20545.

That said, I note that this is true of other backends too, e.g. pants help black, or the online versions shows output other than the expected pants.backend.python.lint.black too, all the way back to ancient history:

  • pants.core: from https://www.pantsbuild.org/2.10/reference/subsystems/black to https://www.pantsbuild.org/2.22/reference/subsystems/black
  • ``: from https://www.pantsbuild.org/2.1/reference/subsystems/black to https://www.pantsbuild.org/2.9/reference/subsystems/black

yapf is similar.

I wonder if this is related to backends that have "shared" code between two: ruff shares between the .check and .format backends (and pants.backend.build_files.fmt.ruff), while black and yapf share between the python.lint one and the build_files.fmt one.

huonw avatar May 31 '24 05:05 huonw

This comes from the fact that they indeed have rules registered by pants.core: https://github.com/pantsbuild/pants/blob/cbf43fc3acdf86b6a0017e06478c7461437b4ba0/src/python/pants/core/register.py#L76-L80

.. and the help extractor gets a list of all backends registering a certain rule, and picks one of them based on heuristics (being the shortest string)

This can lead to confusion, such as this, when rules from other backends are "borrowed" in order to satisfy rule dependencies.

kaos avatar May 31 '24 06:05 kaos

I just realized, a better heuristic would be to take the rules module path into consideration, picking the register.py closest to itself, rather than just the shortest string.

kaos avatar May 31 '24 06:05 kaos

there's also the option of listing all packages that provide it. We do that already for the help-all determinism.

lilatomic avatar Jun 02 '24 03:06 lilatomic

I did some digging around this issue and there appear to be a few different problems causing this.

TL;DR the help output seems to depend on which backends are enabled, which causes at least some of these issues.


I implemented @kaos suggestion here and added a function that chooses the closest provider to the subsystem's fully qualified class name. This works for tools like black within the Pants repo which correspond to enabled backends:

`black` subsystem options
-------------------------

The Black Python code formatter (https://black.readthedocs.io/).

This version of Pants uses `black` version 23.3.0 by default. Use a dedicated lockfile and the `install_from_resolve` option to control this.

Activated by pants.backend.python.lint.black         <--- yay!
Config section: [black]

However, the solution above doesn't work if a relevant backend isn't directly included in the list of backends in pants.toml; for example, the Pants repo doesn't use ruff internally so running pants help ruff still points at pants.core:

`ruff` subsystem options
------------------------

The Ruff Python formatter (https://github.com/astral-sh/ruff).

This version of Pants uses `ruff` version 0.4.4 by default. Use a dedicated lockfile and the `install_from_resolve` option to control this.

Activated by pants.core
Config section: [ruff]

And in some extra logs I added, we see that the correct backend isn't even an option to choose from:

Getting closest provider for subsystem <class 'pants.backend.python.lint.ruff.subsystem.Ruff'> from providers ('pants.core',)

Using the backend-packages option does seem to help output the right thing:

Input

./pants --backend-packages=pants.backend.experimental.python.lint.ruff help ruff

Output

`ruff` subsystem options
------------------------

The Ruff Python formatter (https://github.com/astral-sh/ruff).

This version of Pants uses `ruff` version 0.4.4 by default. Use a dedicated lockfile and the `install_from_resolve` option to control this.

Activated by pants.backend.experimental.python.lint.ruff

However, the solution works great on my own work repo that has both the ruff backends enabled:

`ruff` subsystem options
------------------------

The Ruff Python formatter (https://github.com/astral-sh/ruff).

This version of Pants uses `ruff` version 0.4.4 by default. Use a dedicated lockfile and the `install_from_resolve` option to control this.

Activated by pants.backend.experimental.python.lint.ruff.check     <--- works correctly!
Config section: [ruff]

And conversely, black doesn't work well on my repo because I don't have that backend enabled:

`black` subsystem options
-------------------------

The Black Python code formatter (https://black.readthedocs.io/).

This version of Pants uses `black` version 23.3.0 by default. Use a dedicated lockfile and the `install_from_resolve` option to control this.

Activated by pants.core
Config section: [black]

So I'm not sure what's the best way around this - loading all backends seems way too expensive, but maybe there is another way to accomplish the same thing without needing to do a ton of extra unnecessary work.

krishnan-chandra avatar Jun 05 '24 23:06 krishnan-chandra

I implemented @kaos suggestion here

Nice! (I'd call it inspired by, my suggestion was rather vague, let me clarify.)

To make the HelpInfoExtracter.get_provider() generically better, not just for subsystems, it could take the module path of the thing getting the provider for, and pick the provider closest to that.

See https://github.com/kaos/pants/commit/5d2b5fd86b1c4f8b1bb1e58bb0fc883b4b1321d0 (nice use of difflib I stole that.)

Now as you've observed, this doesn't "work" unless the "correct" backend is enabled. I think for CLI help, this is still correct. It tells you which of your enabled backends registered the thing. But, for online help we'd have to enable all backends before generating the help-all data in order to get accurate activated/provided by data.

That is, I think the commit above is the correct approach, and we also need to adjust the docs generating step accordingly (to enable all backends).

kaos avatar Jun 07 '24 08:06 kaos

But, for online help we'd have to enable all backends before generating the help-all data in order to get accurate activated/provided by data

@thejcannon was clever enough to get that working, so our online docs should already satisfy this ✅ https://github.com/pantsbuild/pantsbuild.org/blob/67038eb6956fe4c8fc956884897efebac6e8fd8d/.github/workflows/sync_docs.yml#L28-L34

huonw avatar Jun 08 '24 10:06 huonw

Marking this one closed, as it should have been closed by https://github.com/pantsbuild/pants/pull/21030.

krishnan-chandra avatar Jul 01 '24 18:07 krishnan-chandra