but yea. food for thought!

Thanks for talking it over! I think I have enough to move forward

sure thing

await Get[Addresses](DependenciesRequest(tgt[Dependencies]))

or

await Get[Addresses](ResolveDependenciesRequest(tgt[Dependencies]))

I like the latter for being more clear, but we’ll also type this a fair amount so maybe too long

does it need the wrapper type…?

(…subclassing, maybe)

Yep, due to subclassing of the field. We can’t have a rule that goes from

Dependencies -> Addresses

, because it would break with the subclass

ProtobufDependencies

. Same reason we need the wrapping

HydrateSourcesRequest

Subclassing of Fields is encouraged with the Target API, so we need to support it.

um

or… is that the way to declare that you need to install a

@rule

to produce Addresses? … ie should that be the union here…?

(i know that that’s not the only reason to subclass dependencies, but maybe it should be? there might be better ways to provide docstrings… unknown)

@hundreds-father-404 FWIW https://github.com/pantsbuild/pants/pull/9841 is one of the reasons I kind of wanted all fields to be async. If we were in a less experimental place, if someone wanted to make dependencies be computed, it would’ve been a horrible breaking change to make dependencies an AsyncField, breaking a bunch of code for folks… Let’s imagine that in the future we decide that a binary’s entry_point should be determined by a rule inspecting its source files in some way - that’s going to be the same problem

Sure, but it’s also a balancing act. We get an enormous explosion of boilerplate in field definitions, as you now have to: 1) define a request type (so that subclasses still work) 2) define a result type, unless there is already a natural result like

Addresses

3) write a rule And it makes call sites much more complex.

from pants.backend.python.target_types import EntryPoint, HydrateEntryPointRequest, HydratedEntryPoint

await Get[HydratedEntryPoint](HydrateEntryPoint(tgt.get(EntryPoint))
print(hydrated_entry_point.value)

vs.

from pants.backend.python.target_types import EntryPoint

print(tgt.get(EntryPoint).value)

I still suggest that means we need to work out how to improve our syntax 🙂 But my real point is: We should work out how to do that kind of transition in the future without breaking the world

I still suggest that means we need to work out how to improve our syntax

Generally agreed with this sentiment. But we’re also running up against natural constraints of the engine that we are unlikely to want to change. For example, needing a new request type and a result type will likely always be the case, afaict. The engine is exceptionally magical, in a good way. But sometimes plain and simple Python is more appropriate. —

We should work out how to do that kind of transition in the future without breaking the world

Agreed

There will be a rule to infer Python dependencies vs. one to infer Java sources etc, right? And it needs to be pluggable that someone can give their own implementation for how to infer for a Python target, right?

For example, needing a new request type and a result type will likely always be the case, afaict.

only if we continue to encourage subclassing of fields for as many usecases as we currently do. i mentioned the possibility of breaking out the pluggable logic there into another type

i agree that consistency is a thing that might be good to bias toward here, even if it causes a bit more boilerplate at the start. because then the incentives are aligned to make the “most pluggable” thing less boilerplatey

sidenote: https://github.com/pantsbuild/pants/pull/9826 has only four test failures, which all superficially seem to have the same cause…?! might be reviewable today. 🤞

…oh, or inferring based on return type? that might be less magical

^ this one might actually be straightforward enough not to be a strawman.

Weird. The dep cycle issue reported in #pantsd is only when the Protobuf dependency injection rules are triggered, and only when using transitive deps. When using transitive deps, the last line of the below doesn’t resolve properly. It gives back a reference to only one target, and the wrong target:

@rule
async def transitive_target(wrapped_root: WrappedTarget) -> TransitiveTarget:
    root = wrapped_root.target
    if not root.has_field(Dependencies):
        return TransitiveTarget(root, ())
    dependency_addresses = await Get[Addresses](DependenciesRequest(root[Dependencies]))

Unregistering the protobuf rule fixes it all.

Hello. This week, I plan to deprecate plugin authors not having a Target API binding for any custom targets. Any feedback on this email to pants-devel? https://docs.google.com/document/d/1w7OMGAwXG0sCklz9_omiJim6Fl5ZXSHhZ4R4aRfuN7c/edit?usp=sharing

Also, is a deprecation of 1.30 okay? I reasoned the audience is much smaller, as this only impacts plugin authors. Whereas, say,

pants.ini

impacts everyone. We can’t remove a bunch of things like V1 isort and

filedeps

until this deprecation lands.

@hundreds-father-404: re: contrib_plugin: can it be an object …?

feels like it could.

fyi I added docs on the new macros feature: https://pants.readme.io/docs/macros

The only issue so far with

object

for

contrib_plugin

is that we need access to the target’s relative path (spec path). Does this mean I need to use

context_aware_object_factory

?