go: add tailor support for go_package, go_module, and go_external_module targets
#12406
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tdyas
commented
Jul 22, 2021
tdyas
commented
Jul 22, 2021
This was referenced Jul 22, 2021
tdyas
pushed a commit
that referenced
this pull request
Jul 22, 2021
Introduce the `MaybeEmptySiblingAddresses` address spec to allow searching for addresses in a directory without a BUILD file or for which there are no targets in any of the BUILD files. It is similar in spirit to the already-existing `MaybeEmptyDescendantAddresses`. This PR is necessary for the Go tailor support in #12406. The inference for `go_external_module` attempts to find `go_module` targets along the tailor `search_paths`, but cannot use the `SiblingAddresses` address spec because directories at the top of the tree that may not have a BUILD file will cause `SiblingAddresses` to raise an exception. `SiblingAddresses` will now fail if it does not find any matching targets. [ci skip-rust] [ci skip-build-wheels]
tdyas
force-pushed
the
golang_tailor_support
branch
from
f62adc7
to
12388c6
Compare
tdyas
pushed a commit
that referenced
this pull request
Jul 26, 2021
## Problem `./pants tailor` currently assumes that all targets on which it operates will have a `sources` field. As part of the Go plugin's tailor support in #12406, I would like `tailor` to infer `go_external_module` targets which do not have a `sources` field from the dependencies of `go_module` targets. Attempting to add a `GoExternalModule` target as a `PutativeTarget` caused invocation of `tailor` to raise an exception in `default_sources_for_target_type` because that function could not find a `sources` field. ## Solution Work-around the issue by teaching `tailor` to ignore the lack of a `sources` field if there are no owned sources or triggering sources for the `PutativeTarget`. The exception is now raised in `PutativeTarget.for_target_type` if there are any such sources. `default_sources_for_target_type` just returns an empty tuple if it could not find a `sources` field. This is just a work-around and is not intended as a longer term solution for `tailor` to support target types without `sources`. [ci skip-rust] [ci skip-build-wheels]
added 2 commits
July 26, 2021 10:00
[ci skip-rust] [ci skip-build-wheels]
[ci skip-rust] [ci skip-build-wheels]
tdyas
force-pushed
the
golang_tailor_support
branch
from
12388c6
to
d214197
Compare
|
I rebased the PR now that both blocking PRs have landed on |
|
ping on review |
stuhood
approved these changes
Jul 26, 2021
added 3 commits
July 26, 2021 13:54
Eric-Arellano
approved these changes
Jul 26, 2021
[ci skip-rust] [ci skip-build-wheels]
|
Taking a look now, sorry for the delay. |
benjyw
approved these changes
Jul 26, 2021
There was a problem hiding this comment.
Looks good! I am slightly confused about the sources field on go_package. I thought we were moving towards that target not having a sources field but always owning the entire directory, since that is more idiomatic in Go? Is that future work? It will affect all this.
Yes, that change is coming in a future PR. |
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds support for
./pants tailorto the Go plugin:go_packagetargets from the existence of*.gofiles.go_moduletargets from the presence ofgo.modfiles.go_external_moduletargets based on resolving existinggo_moduletargets for their module dependencies. These are written into a separateBUILD.godepsfile so it is easy to delete just the generatedgo_external_moduletargets in order to re-run tailor to re-generate.Note: The inference for
go_external_moduletargets is necessary because Pants does not currently support dynamic target injection from engine rules and we want to implement dependency inference betweengo_packageandgo_external_moduletargets. Since plugins cannot dynamically creatego_external_moduletargets, the short term solution is to require users to run./pants tailorto create the appropriatego_external_moduletargets. A future PR will implement dependency inference between these targets. This PR does not try to solve what we would want for this use case in the long term.