pantsbuild / pants

The Pants Build System
https://www.pantsbuild.org
Apache License 2.0
3.32k stars 636 forks source link

`help backends` should indicate which backends are used by `update-build-files` #18502

Open cognifloyd opened 1 year ago

cognifloyd commented 1 year ago

Is your feature request related to a problem? Please describe. ./pants help backends shows that pants.backend.build_files.fix.deprecations and pants.backend.build_files.fmt.black are NOT enabled by default. That surprised me because the update-build-files goal definitely uses black.

Describe the solution you'd like In the help backends output:

Enabled backends are marked with `*`. To enable a backend add it to `[GLOBAL].backend_packages`. 

Let's add another character, maybe -, to show that a backend is conditionally enabled.

The update-build-files goal will use a formatter backend without regard to the [GLOBAL].backend_packages setting. By default, it uses pants.backend.build_files.fmt.black and pants.backend.build_files.fix.deprecations.

The formatter can be switched from black to yapf or buildifier based on the [update-build-files].formatter setting.

Using fix.deprecations backend is controlled by the [update-build-files].fix_safe_deprecations setting (either True or False).

So, these goal-specific backends should get a - indicator to show that it is enabled, but only for that one special-purpose goal based on different settings.

Describe alternatives you've considered Just remove the backends from the output? That would be confusing too, because the backends exist. Also, you can enable the pants.backend.build_files.fix.deprecations backend so that it ALSO runs during the fix goal, not just during update-build-files.

Additional context

current output

% ./pants help backends

Backends
--------

List with all known backends for Pants.

Enabled backends are marked with `*`. To enable a backend add it to `[GLOBAL].backend_packages`. 

...
[ ] pants.backend.build_files.fix.deprecations                                              [pants]
[ ] pants.backend.build_files.fmt.black                                                     [pants]
[ ] pants.backend.build_files.fmt.buildifier                                                [pants]
[ ] pants.backend.build_files.fmt.yapf                                                      [pants]
...
[*] pants.backend.python                                                                    [pants]
     Support for Python.

     See https://www.pantsbuild.org/docs/python-backend.
[ ] pants.backend.python.lint.autoflake                                                     [pants]
     Autoformatter for removing unused Python imports.

     See https://github.com/myint/autoflake for details.
[*] pants.backend.python.lint.bandit                                                        [pants]
     Security linter for Python.

     See https://www.pantsbuild.org/docs/python-linters-and-formatters and
     https://bandit.readthedocs.io/en/latest/.
[*] pants.backend.python.lint.black                                                         [pants]
     Autoformatter for Python.

     See https://www.pantsbuild.org/docs/python-linters-and-formatters and
     https://black.readthedocs.io/en/stable/.
...

proposed output

% ./pants help backends

Backends
--------

List with all known backends for Pants.

Enabled backends are marked with `*`. To enable a backend add it to `[GLOBAL].backend_packages`. 
The `update-build-files` goal uses backends marked with `-` based on `[update-build-files]` settings.

...
[-] pants.backend.build_files.fix.deprecations                                              [pants]
[-] pants.backend.build_files.fmt.black                                                     [pants]
[ ] pants.backend.build_files.fmt.buildifier                                                [pants]
[ ] pants.backend.build_files.fmt.yapf                                                      [pants]
...
[*] pants.backend.python                                                                    [pants]
     Support for Python.

     See https://www.pantsbuild.org/docs/python-backend.
[ ] pants.backend.python.lint.autoflake                                                     [pants]
     Autoformatter for removing unused Python imports.

     See https://github.com/myint/autoflake for details.
[*] pants.backend.python.lint.bandit                                                        [pants]
     Security linter for Python.

     See https://www.pantsbuild.org/docs/python-linters-and-formatters and
     https://bandit.readthedocs.io/en/latest/.
[*] pants.backend.python.lint.black                                                         [pants]
     Autoformatter for Python.

     See https://www.pantsbuild.org/docs/python-linters-and-formatters and
     https://black.readthedocs.io/en/stable/.
...
kaos commented 1 year ago

I'm not a fan of too much special casing update-build-files here. These kind of dependencies is all over the place, not just for this. Worth noting is that the backend is not enabled, although the rules it would provide is in use by another.. To me seeing there is a backend, that is not enabled, for formatting build files with black tells me there is something available that I'm not using.

This is seen if you inspect the rules themselves, digging out one of the update build file rules for instance, reveals it is enabled by pants.core (i.e. by default and can't be disabled)

$ pants help-advanced FixResult

`pants.core.goals.fix.FixResult` api type
-----------------------------------------

FixResult(input: 'Snapshot', output: 'Snapshot', stdout: 'str', stderr: 'str', tool_name: 'str')

activated by       : pants.backend.python.lint.black, pants.backend.python.lint.isort, pants.backend.shell.lint.shfmt, pants.core
dependencies       : builtins
                     pants.backend.python
                     pants.engine.fs
                     pants.engine.platform
                     pants.engine.process
dependents         : pants.core
returned by 5 rules: pants.backend.build_files.fix.deprecations.renamed_fields_rules.fix
                     pants.backend.build_files.fix.deprecations.renamed_targets_rules.fix
                     pants.backend.python.lint.black.rules.black_fmt
                     pants.backend.python.lint.isort.rules.isort_fmt
                     pants.backend.shell.lint.shfmt.rules.shfmt_fmt
consumed by 1 rule : pants.core.goals.fix.convert_fix_result_to_lint_result
used in 1 rule     : pants.core.goals.fix.fix_batch

$ pants pants.backend.build_files.fix.deprecations.renamed_fields_rules.fix --help-advanced

`pants.backend.build_files.fix.deprecations.renamed_fields_rules.fix` rule
--------------------------------------------------------------------------

Fix deprecated field names

Undocumented.

activated by : pants.core
returns      : FixResult
takes 1 input: Batch
awaits 3 gets: Get(DigestContents, Digest, ..)
               Get(FixedBUILDFile, RenameFieldsInFileRequest, ..)
               Get(Snapshot, CreateDigest, ..)
cognifloyd commented 1 year ago

Hmm. Well then the help text should not be specific to update-build-files. I would like an indicator if a backend is enabled even if I did not explicitly enable it. If pants.core always activates it, then that counts as enabled.

kaos commented 1 year ago

Hmm. Well then the help text should not be specific to update-build-files. I would like an indicator if a backend is enabled even if I did not explicitly enable it. If pants.core always activates it, then that counts as enabled.

The thing is, it is not the backend that is enabled, it is only that some of the rules gets pulled in; in other words, there are some rules that gets activated by more than one backend. Most (all?) of them have a principal backend they "belong" to logically but as they may be used from any number of backend I don't think it would make sense (nor be correct) to indicate a backend as implicitly enabled whenever any of it's rules are activated. If the indicator meant "some rules from this backend is in use by another backend" that would be accurate, but I wonder how useful this information really is (i.e. how bad surprise was this for you?)

kaos commented 1 year ago

What would make more sense to me is to be able to list everything that is registered from a certain backend, for introspection.