New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use TransitiveTargetsRequest
as input for resolving TransitiveTargets
#10915
Use TransitiveTargetsRequest
as input for resolving TransitiveTargets
#10915
Conversation
…ets` # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
e3fddac
to
40c8fc0
Compare
These behave almost the same, but don't work with dep inference [ci skip-rust] [ci skip-build-wheels]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ready. PTAL, particularly at TransitiveTargetsLite
and DependenciesRequestLite
.
direct_dependencies_addresses_per_tgt = await MultiGet( | ||
Get(Addresses, DependenciesRequestLite(tgt.get(Dependencies))) for tgt in queued | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fails if we change it to Get(Targets)
.
wrapped_transitive_excludes = await MultiGet( | ||
Get( | ||
WrappedTarget, AddressInput, AddressInput.parse(addr, relative_to=tgt.address.spec_path) | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't use Get(Targets, UnparsedAddressInputs)
.
subtargets = await Get( | ||
Subtargets, Address, request.field.address.maybe_convert_to_base_target() | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Normally we only do this if inference isn't at play. Because we never use inference here, we always perform this step.
Problem
As explained in #10888, we are not properly handling "special-cased" dependencies-like fields, such as
python_tests
'sruntime_package_dependencies
field.We need those fields' dependencies to show up in project introspection, such as
./pants dependencies
, but not in other contexts like when resolving the transitive targets to determine what source files are used by Pytest.That is, we need to be able to parametrize
await Get(TransitiveTargets)
so call sites can determine the behavior they want.Solution
Add
TransitiveTargetsRequest
, which will allow us to add new fields to it to change its behavior. This mirrors how we haveDependenciesRequest
for direct deps.Note that this means you can no longer request
TransitiveTargets
as a parameter to your rule and have it be evaluated automatically by looking at theSpecs
. You must now requestAddresses
, and then useawait Get
. There's an argument for this being a good thing. It's a bit confusing (imo) how some types likeAddresses
can be in a rule signature and will be evaluated by using the Specs. This means that you can now only do that withAddresses
andTargets
, but notTransitiveTargets
.To work around #10917, we add a
DependenciesRequestLite
andTransitiveTargetsLite
for use with codegen. These behave almost the same, but don't work with dep inference.[ci skip-rust]
[ci skip-build-wheels]