https://pantsbuild.org/ logo
Powered by
h

hundreds-father-404

08/07/2020, 9:08 PM
I’m flakily getting this failure in a new unit test. Is that flakiness the same thing as https://github.com/pantsbuild/pants/issues/8862? What’s surprising to me is that sometimes the graph is valid, sometimes it’s not, whereas Benjy’s reported case would error both times.
Copy code
E       Exception: Rules with errors: 2
E
E         @rule(pants.engine.internals.graph:435:addresses_with_origins_from_filesystem_specs(FilesystemSpecs, GlobalOptions) -> AddressesWithOrigins, gets=[Get(Snapshot, PathGlobs), Get(Owners, OwnersRequest)]):
E           Was not reachable, either because no rules could produce the params or because it was shadowed by another @rule.
E
E         @rule(pants.engine.internals.graph:481:resolve_addresses_with_origins(Specs) -> AddressesWithOrigins, gets=[Get(AddressesWithOrigins, AddressSpecs), Get(AddressesWithOrigins, FilesystemSpecs)]):
E           Was not reachable, either because no rules could produce the params or because it was shadowed by another @rule.
This is hacked around by registering 
FilesystemSpecs
as 
RootRule
. The issue is that 
AddressSpecs
is 
RootRule
in that test, so the 
Specs -> AddressesWithOrigins
rule has issues
I suspect that us registering RootRule in so many places we should not be results in some issues.
Nvm, it still flakes. I think the “shadowing” part is the issue? The only solution I can think of is to de-register the problematic 
Specs -> AddressesWithOrigins
rule?
w

witty-crayon-22786

08/07/2020, 9:57 PM
sorry, when you say that it flakes, you mean that it fails intermittently?
h

hundreds-father-404

08/07/2020, 9:58 PM
Yeah. If I run one run, the rule graph may be fine. If I invalidate via a whitespace change, then 2/7 of the tests may fail due to the above error
w

witty-crayon-22786

08/07/2020, 9:58 PM
flaking is a bug, in and of itself.
it should either succeed stably or fail stably
what a crop. sorry.
h

hundreds-father-404

08/07/2020, 10:00 PM
Agreed
Oh, I think I can work around this by just passing 
Specs
into the graph rather than 
AddressSpecs
. Then I don’t need to register it as a new 
RootRule
w

witty-crayon-22786

08/10/2020, 5:21 PM
good news, bad news: good news: i found a small fix for https://github.com/pantsbuild/pants/issues/10504 bad news: it exposed that the underlying issue here is a really deep one… we’ve outgrown the algorithm i was using to deal with discovering rule graph dependencies in cycles
h

hundreds-father-404

08/10/2020, 5:21 PM
What’s the underlying issue? Does it explain this issue I was having?
w

witty-crayon-22786

08/10/2020, 5:25 PM
yes, it does.
❤️ 1
rule graph construction was mostly recursive with a bit of “bouncing back out of the graph and retrying in case of cycles”
and in the case of multiple cycles, things got tangled. i didn’t dig deeply enough to figure out how to untangle them, because the special casing was already complicated, and i’ve had a line on how to do this more rigorously for a while.
💯 1
Powered by