https://pantsbuild.org/ logo
Join Slack
Powered by
Hmm, I'm in a huge mess with circular dependencies...
# plugins
g
Hmm, I'm in a huge mess with circular dependencies in Rust workspaces, and I'm unsure of what tools Pants offers to help disambiguate this. I have multiple ways of fixing it by just adding more targets, but they would no longer map to the conceptual model, which feels bad. I can solve this for "output" targets like binaries, tests and libraries, as they are separate targets. The dependency chain there ends up going for example 
binary_crate -> workspace -> {all_member_packages}
. However, for "non-output" targets it becomes weird. For example, 
clippy
operates on a package but needs the whole workspace. So then we get 
package -> workspace -> {all_member_packages}
- and those member packages then depend back on the package we're analysing via the workspace. I can definitely solve this by just adding more indirection, but having a specific target just for linting feels conceptually wrong. Maybe @fast-nail-55400 has some thoughts from working with go?
b
Python has a similar thing too, where files can (lazily) import each other. I think pants will sometimes work with the “coarsened targets” which I believe collapses the circular dependencies (strong connected components) into a single group. (Other jargon: graph condensation) I don’t know if something like that helps.
g
Ah, good lead! Yes, that could maybe work if I read up on it a bit.
f
The JVM backends use 
CoarsenedTarget
to collect the set of files which need to be compiled together.
Go does not use them since all files for a 
go_package
are compiled as one unit.
For the JVM backends, they support splitting a package into one or more 
CoarsenedTarget
instances depending on what dependency inference detected.
g
I might be misunderstanding how CoarsenedTargets should work. I was requesting TransitiveTargets before, which failed due to the cycle. I tried replacing it with 
CoarsenedTargets
, but that one starts with TransitiveTargetsRequest and explodes.
Copy code
pants.engine.internals.graph.CycleException: The dependency graph contained a cycle:
-> examples/workspace/secondlib#package
   examples/workspace/wsbin#package
-> examples/workspace/secondlib#package
f
What does the 
Get
look like?
g
Copy code
coarsened_targets = await Get(CoarsenedTargets, CoarsenedTargetsRequest(request.addresses))
Where 
request.addresses
is 
frozenset({Address(examples/workspace/wsbin#package)})
.
Ah, I wonder if the issue here might be that the root itself is part of the cycle.
Nope, wasn't it. Changed it around a bit to make the workspace an explicit dependency, and now it looks like this:
Copy code
examples/workspace/wsbin#package
-> examples/workspace:workspace
   examples/workspace/secondlib#package
-> examples/workspace:workspace
If I just comment out cycle detection here https://github.com/pantsbuild/pants/blob/55dcd4ac944712505b8cb58eb7a64096e1695ce6/src/python/pants/engine/internals/graph.py#L744C5-L744C19 things work fine. I wonder if that should actually happen for a coarsened request... I'm not sure. It feels like I'm missing something here. I notice that a lot of the code here talks about file targets specifically, so I'm wondering if that's the mistake I'm making. Do you remember @witty-crayon-22786?
Ending it for today, will probably keep hacking tomorrow... This is what I have for now, which yeah, works without the check above. https://github.com/tgolsson/pants-cargo-porcelain/blob/ts/workspace/pants-plugins/cargo-porcelain/pants_cargo_porcelain/util_rules/sandbox.py#L23-L41...
OK; spent some time just iterating w/ graph visualization... landing here I think. I think part of the issue here is that I'm trying to model two different concepts as dependencies: belonging, and depending. crates belong to a package, and packages belong to a workspace. I've been thinking about whether 
cargo_crates
should really be 
cargo_sources
, which maybe makes more sense from a naming standpoint. But I think for various reasons the "source owner" will remain the 
cargo_package
, and any lower level constructs are just for goals.
Re belonging I think the more truthful model would be a package depends on its crates, etc. But that doesn't help resolve any actual problems in the design.
Maybe this is a more truthful model... And it feels more pantsy?
It does imply that any other resources etc would be depended on by the 
cargo_sources
, which is... maybe not great.
Though maybe correct; in the sense that e.g. a 
include_str!(...)
is a file-to-file dependency.
f
Why not just have the user declare a 
resources
target type for those included files and then add an explicit dependency from the 
cargo_crate
?
(and maybe eventually implement dependency inference for 
include_str!
usages)
The Go backend does something similar for embedding files as resources.
g
I think mechanically that works. But the crate/s don't own sources, and if they did have dependencies they wouldn't be in the transitive closure of the workspace or package. So that would be worrisome for package level goals. Since that is where clippy runs to avoid wasted work, that breaks I think.
4 Views
Open in Slack
PreviousNext
Powered by