pantsbuild / pants

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

docs: goal associated with multiple backends only lists one #17772

Open huonw opened 1 year ago

huonw commented 1 year ago

Describe the bug The export-codegen goal is associated with/created by many backends, like protobuf, docker, and several others. However, the docs page for it (https://www.pantsbuild.org/docs/reference-export-codegen) only lists one: pants.backend.docker.

image

Pants version 2.14

OS N/A

Additional info N/A

huonw commented 1 year ago

This seems to be somewhat of an explicit choice: https://github.com/pantsbuild/pants/blob/4dc2d025e8a7b2018e04a1e6ffde9a3f0b61cc80/src/python/pants/help/help_info_extracter.py#L600-L605

https://github.com/pantsbuild/pants/pull/13993

kaos commented 1 year ago

Yea, I found it counter-productive to list all backends that registered types/rules, as there is quite a bit of cross pollination going on, due to dependencies between backends. Listing them all would be rather confusing in terms of presenting "the source" for where it came from.

The idea here being, that it is reasonable to assume that the shorter module path is likely a "top level" thing that would make sense to enable in the [GLOBAL].backend_packages configuration.

kaos commented 1 year ago

Having said that, perhaps for goal_rules, the list of providers rarely is so long so it could make sense to not limit this to just one entry..?

huonw commented 1 year ago

Thanks for the insight. I tried the quick thing of switching to including all providers for all options (https://github.com/huonw/pants/commit/8b82d4514df33cab6fa2da296492946786f2045d#diff-6e1f224c2d6075bc35b2fe6023c3aef055e5985c7f4042676b0f9617e296c3af).

I checked in the docs so that the rest of that diff shows what changed. It looks like there's a few broad categories of what's changed (just a selection, this isn't a complete categorisation of all files):

  1. useless extra info along the lines of what you say:
    • 'core' features: generated-docs/GLOBAL.md, generated-docs/generate-lockfiles.md, generated-docs/pex.md, generated-docs/pex-cli.md
    • multiple backends that are all related (e.g. where there's one 'parent' one that would definitely be enabled if any of the others are): generated-docs/golang.md, generated-docs/kotlin-infer.md, generated-docs/shellcheck.md
    • weird cases: generated-docs/experimental-bsp.md, generated-docs/check.md
  2. cases where the backends aren't (all) subsets of each other, and so listing multiple is potentially useful: generated-docs/lambdex.md, generated-docs/jvm.md, generated-docs/jvm_artifact.md, generated-docs/docker.md, generated-docs/protoc.md (e.g. for the JVM ones, someone might reasonably activate pants.backend.experimental.scala without activating pants.backend.experimental.java, but the current docs show Java only)
  3. cases where pants.core appears but isn't the best choice: generated-docs/black.md, generated-docs/yapf.md
  4. a bit of all of the above: generated-docs/local_environment.md, generated-docs/docker_environment.md

This definitely seems like an annoying thing to get 'right'! Maybe the current behaviour is the best balance 😄

kaos commented 1 year ago

Haha, nice research. Yea, there are certainly cases where showing just one is not perfect or the shortest one is not what we'd like. As you say, getting this right for every case will be tricky I think.

Eric-Arellano commented 1 year ago

Thanks @huonw! I agree with your classification. How about we allow GoalSubsystem authors to give a hint what behavior we want? We default to the current behavior of only one backend, whichever has the shortest name (so we use pants.backend.python rather than pants.backend.python.lint.isort). But you can set a class property to say that the docs and help should instead list every backend.