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?

yep

Okay, that could either makes this extremely complex or be fairly simple, depending on this: Do you think it’s acceptable to use the identity of the

Dependencies

field to signal which dep inference implementation to use? E.g.

PythonDependencies

and subclasses -> the Python rule that knows how to analyze Python code. We would enforce that there is only one dep inference implementation for that type to avoid ambiguity, like we do with codegen. As opposed to, for example, finding the corresponding

Sources

field for the target and seeing if its

PythonSources

.

Do you think it’s acceptable to use the identity of the 

Dependencies

 field to signal which dep inference implementation to use? E.g. 

PythonDependencies

 and subclasses -> the Python rule that knows how to analyze Python code.

yes, in general i do. i’d go further and say that each concrete class should require its own rule so that it’s an exact type match (but that’s the other conversation, heh.)

Wonderful. Thank you - I had a temporary moment of panic hehe

dep inference will need to recurse to request the

Sources

, to compute the output

Agreed. The plan for now is to pass the original

Dependencies

field to the inference rule. This field stores the

address

, so, in the rule, we can call

await Get[Target](Address, field.address)

well, i mean the dep inference rule

i mean the dep inference rule

As do I, but I think we’re on the same page here.

Agreed. This current design wouldn’t do that because it would encounter a 

protobuf_library

 and see “I don’t have a inference rule for ProtobubLibrary; skip.”

what i meant here is that while inferring deps, a rule for PythonDependencies would request PythonSources for the Sources field, which might trigger codegen otherwise

what i meant here is that while inferring deps, a rule for PythonDependencies would request PythonSources for the Sources field, which might trigger codegen otherwise

This would only happen if in that rule we said

HydrateSourcesRequest(codegen_enabled=True)

. That is off by default.

it should request PythonSources from a correctness perspective, although it might be able to get away with just requesting whatever kind of sources are there based on some assumption of validation. but then it might actually end up inferring python deps for some arbitrary combination of fields where someone declared something like

PythonDependencies + RustSources

It can request

for_sources_types=[PythonSources], codegen_enabled=False

yep.

some arbitrary combination of fields where someone declared something like PythonDependencies + RustSources

Yay Field-Driven API? 🙃 hehe

heh, yep.

it does make me wonder whether this should really be kicked off by dependencies subclasses rather than sources** subclasses

I don’t follow

it’s not strictly the Dependencies field that decides whether some dependencies can be inferred for some sources

Agreed, it really is about what the

Sources

field’s identity is. We could make this work without making call sites more complex, I think (one of my main goals). In

resolve_dependencies

, call

await Get[WrappedTarget](Address, deps_field.address)

, then inspect the identity of the

Sources

field, if any.

can iterate once it’s in i think.

True, but I don’t think the

await Get[WrappedTarget]

so that we base this in the

Sources

identity is much more complex. (I thought it would be way more involved when I started this thread)

k

i do idly wonder whether some of these things would be easier if they operated on batches of fields

the

AsyncField

PR confirmed my intuition that I don’t think so. Most those call sites resolved dependencies in very different stages than hydrating sources. Some, like

setup-py

, never even need sources

nioce. thanks! will look this afternoon

So, my sole concern about doing the

await Get[WrappedTarget]

in

resolve_dependencies

is an unnecessary perf hit if you don’t even have dep inference. While small, there is a cost to go from

Address -> Target

, such as calling the constructor for every

Field

registered on the target. Two ways to solve this, I think: 1. Add a global flag

--dependency-inference-enabled

2. Simply check the

UnionMembership

to see if there are any implementations registered in the first place. I think #2 is better, right? Simpler and easier to onboard folks.

mm… i think i’d want to see it showing up in traces before adding flags to enable or disable

I’ll go with the second approach, for now. Beyond perf, it’s clearer code. Thanks!

i’m not sure… it’s redundant, right? because you’re checking in two places