[internal] Fix go_binary's main field inference to work with generated targets
#13163
Conversation
…rated targets [ci skip-rust] [ci skip-build-wheels]
Eric-Arellano
force-pushed
the
infer-main
branch
from
8e5f91c
to
215a9f9
Compare
| if not self.address.is_generated_target: | ||
| # TODO: Make this error more eager via target validation. | ||
| raise AssertionError( | ||
| f"Target was manually created, but expected to be generated: {self.address}" | ||
| ) |
There was a problem hiding this comment.
@tdyas I've been thinking a lot the past few days about whether to keep _go_{first,third}_party_package targets private.
The downside is they won't show up in ./pants help and the docs, and I think it would be helpful for people to understand what the atoms of their builds are. In particular, it will be important when adding an overrides mechanism to go_mod.
I think the answer is to make them public, but use eager validation that they are never explicitly created.
There was a problem hiding this comment.
I share your concern with that downside. Letting users understand the magic even a little is probably beneficial, even if the targets should be auto-generated in the standard case. Should the first-party target then be called go_package again, or keep the longer name so users are less likely to try and use it?
There was a problem hiding this comment.
I think it should still be go_first_party_package, per discussion today. But document that it should not be manually created + add eager validation.
| "Hint: consider leaving off this field so that Pants will find the `go_package` " | ||
| "target for you." | ||
| "Hint: consider leaving off this field so that Pants will find the " | ||
| "`_go_internal_package` target for you." |
There was a problem hiding this comment.
Should this error explain to the user that _go_internal_package is a hidden target type that is auto-generated for them? Some of the other error messages in this PR do mention that fact.
There was a problem hiding this comment.
Probably. Will audit all the error messages when renaming these targets.
Restores support for #13117 post-#13139.
[ci skip-rust]