https://pantsbuild.org/ logo
Join Slack
Powered by
Almost completely removed `BuildFileAddresses` fro...
# development
h
Almost completely removed 
BuildFileAddresses
from the engine! https://github.com/pantsbuild/pants/pull/9091 The sole remaining issue is that 
test.py
requesting an 
AddressOriginMap
results in the 
test
goal rule being unreachable. I can’t figure out how to fix it if anyone has a moment to take a look..Pull down the PR and in 
test.py
uncomment out line 179
❤️ 1
👍 1
w
what does the rule graph look like below 
AddressOriginMap
?
h
Hm let me check. I’ll upload the PDF. Is this 
graph.000.dot
or 
rule_graph.dot.pdf
?
w
the latter
👍 1
the former is a trace of the whole run.
👍 1
ie, the contents of the 
Graph
the latter is a dump of the 
RuleGraph
h
rule_graph.dot.pdf
(@aloof-angle-91616 if you have a moment to merge the rule graph visualization PR, it’d be very helpful. I’m happy to re-review if that’d help)
w
you have a root for: 
Select(AddressOriginMap) for (FilesystemSpecs, OptionsBootstrapper)
h
What does that mean?
w
and another for 
Select(AddressOriginMap) for AddressSpecs
👍 1
it means to get a Y, you need an (X, Z)
👍 1
these are the API docs.
we should generate something nicer. but.
was this the failing run?
h
Yes. I will upload the same graph from master in a moment
rule_graph.dot.pdf
a
i’ll try to get the rule graph viz PR in
❤️ 1
w
@hundreds-father-404: and what was the actual error?
h
Copy code
Exception message: Rules with errors: 1
  @goal_rule(pants.rules.core.test:136:run_tests(Console, TestOptions, InteractiveRunner, Addresses) -> Test, gets=[Get[Address](Addresses), Get[AddressAndDebugRequest](Address), Get[AddressAndTestResult](Address)]):
    Was not reachable, either because no rules could produce the params or because it was shadowed by another @rule.
w
that's... unlikely.
h
Commenting out that 
coordinator_of_tests
requires 
AddressOriginMap
fixes it all
I doubt it’s being shadowed. I figured that there is no path to give the 
AddressOriginMap
. This is the 
await
that causes the coordinator to be called:
Copy code
results = await MultiGet(Get[AddressAndTestResult](Address, addr) for addr in addresses)
w
well, no way to get an AddressOriginMap would look like something else i think.
test not being reachable means that it can't be computed using the registered RootRules.
h
*No way to get an 
AddressOriginMap
given the above 
await Get
, which only passes 
Address
as a param (which then gets converted into 
HydratedTarget
)
w
if you don't see an error for the rule that computes the AddressOriginMap, then that shouldn't be your issue, i don't think.
but i'll try to look later. need to focus on improving these errors at some point.
👖 1
h
Okay, thank you! Very excited for this change to land
w
going to unwind instead! hopefully tomorrow
sorry
a
don't be sorry!
unwinding is a sacred practice protected by the pants constitution
❤️ 1
w
😄
❤️ 1
the issue was probably the rule that we had installed to go from 
Addresses -> Address
... i was worried that that one might be ambiguous.
one of the runs rendered:
Copy code
Ambiguous rules to compute Get[Address](Addresses) with parameter types (Address, AddressSpecs, Console, InteractiveRunner, OptionsBootstrapper):
  @rule(pants.init.engine_initializer:435:single_address(Addresses) -> Address) for Addresses
  Param(Address)
the inconsistent errors would be due to https://github.com/pantsbuild/pants/issues/8862 ... sorry about that.
👍 1
h
Hm, any idea why that rule would be an issue?
w
see the error
(...on a wide enough screen to see the indentation)
Copy code
Param(Address)
vs
Copy code
@rule(pants.init.engine_initializer:435:single_address(Addresses) -> Address) for Addresses
will push a patch shortly.
❤️ 1
h
I don’t understand what that error means tbh. Thanks for investigating it and for the patch!
w
@hundreds-father-404: if you have both an Address and Addresses in some context, which should you use to get an Address?
that's the ambiguity.
mmm... solving this the right way probably involves moving 
ResolveError
... i need to run, but @hundreds-father-404: i pushed to https://github.com/pantsbuild/pants/pull/9091
👍 1
h
So should we introduce a wrapper type like UniqueAddress for that rule to disambiguate that you solely want one single address?
w
see the commit i made... it doesn't need to be a rule.
h
Ah, good point!
w
but there is an import cycle due to where 
ResolveError
lives right now... i think we should probably move it to 
base.exceptions
👍 1
Copy code
File "/Users/stuhood/src/pants/src/python/pants/engine/addressable.py", line 12, in <module>
    from pants.engine.mapper import ResolveError
  File "/Users/stuhood/src/pants/src/python/pants/engine/mapper.py", line 10, in <module>
    from pants.engine.parser import Parser
  File "/Users/stuhood/src/pants/src/python/pants/engine/parser.py", line 8, in <module>
    from pants.engine.struct import Struct
  File "/Users/stuhood/src/pants/src/python/pants/engine/struct.py", line 8, in <module>
    from pants.engine.addressable import addressable, addressable_list

Exception message: cannot import name 'addressable' from 'pants.engine.addressable' (/Users/stuhood/src/pants/src/python/pants/engine/addressable.py)
h
Good idea. Thank you!
w
it doesn't need to be in "mapper.py"
Open in Slack
PreviousNext
Powered by