https://pantsbuild.org/ logo
Join Slack
Powered by
OR tweaking the rust code to say "did you mean X.....
# development
w
OR tweaking the rust code to say "did you mean X...?"
h
should I expect 
self.scheduler.visualize_graph_to_file('/tmp/test.dot')
to do what I want? when I run the test it creates this file but it appears to be blank when I convert it to a pdf
or maybe that's the problem to begin with?
w
no, it's not empty i don't think
i'm fiddling with the block linked above to just 
println(..)
the roots
h
the dot file has just the text:
Copy code
digraph plans {
  node[colorscheme=set312];
  concentrate=true;
  rankdir=TB;
}
so it looks like maybe it's creating the structure of a dot file and then not writing anything?
or maybe I'm not invoking visualize_ the right way?
w
hm
oh, 
visualize_graph_to_file
is the runtime graph
there are other methods to visualize the rule graph
@hundreds-breakfast-49010: 
visualize_rule_graph_to_file
h
will try
what's the distinction between the runtime graph and the pre-runtime rule graph? (although the pre-runtime one is made by parsing pants' own source files just after startup, right?)
w
no
oh. "own source files"... um, sortof?
when you construct a 
Scheduler(.., rules=...)
, we statically take those rules and create a graph of them
the RuleGraph
that's before anything has run. but it defines what is possible to run
h
this is what I get in my test. it looks like there's four nodes that yield a 
SourceRootStrippedSources
from something
w
blue nodes are roots
h
huh, so something is making 
SourceRootStrippedSources
a root rule when it shouldn't be?
w
no
you're passing in 
Params((HydratedTarget+SourceRootConfig))
there are multiple rules that can construct a 
SourceRootStrippedSources
oh.
so, look the roots (blue) that produce 
SourceRootStrippedSources
one of them takes 
Address+SourceRootConfig
, and the other takes 
Address+OptionsBootstrapper
(a SourceRootConfig can be constructed from an OptionsBootstrapper)
h
hm I dfined both RootRule(SourceRootConfig) and optinable_rule(SourceRootConfig), that's probably not right
w
so, then: why isn't there a root that will accept a 
HydratedTarget
? ... because there is no 
RootRule
for HydratedTarget.
☝️ 1
h
but I got the same error with either of them alone
e
Does this make any sense when a RootRule(SourceRootConfig) is installed and a SourceRootConfig instance is handed in as a Param though?
That is handing the engine a short-cicuited-pre-created root thing
no rule needed
h
I'm creating a HydratedTarget manually for this test, so I didn't make a RootRule for it
w
SourceRootConfig is not the missing thing
h
that probably exists in the real rule graph though
w
this is not the real rule graph
h
actually is having both optionable_rule(SourceRootConfig) and RootRule(SourceRootConfig), harmful?
w
but even the real rulegraph is not expecting to have a HydratedTarget injected from outside
a RootRule says: "this will come in from the outside!"
and here there isn't one, and so you don't end up with a root for that.
e
But, conceptually what is wrong with thinking you should be able to short-circuit anywhere by injecting a concrete thing?
w
the rulegraph is compiled ahead of time
we're not dynamically computing entrypoints
h
okay i think omitting the 
RootRule(HydratedTarget)
was the problem
although this may not be the right thing to do in this test anyway
w
yea.
e
So then the real problem here is intrinsics
h
I think I do want to mock a 
HydratedTarget
for this test
w
@hundreds-breakfast-49010: so, looking at the actual code for the rule, it looks like you're wrapping a TargetAdaptor in a HydratedTarget
h
yeah, I don't know if that's correct
w
probably not
(it works, obviously, but you're faking the other arguments to the HydratedTarget constructor)
h
well it works in the sense that the rule outputs something now, not in the sense that the test usefully exercises the rule
w
@hundreds-breakfast-49010: this call:
Copy code
HydratedTarget(address="", adaptor=test_target, dependencies=())
you're wrapping it around a 
PythonTestsAdaptor
that's a concrete type
so: your other rule can take one of those
...but you're trying to make it generic.
right.
h
right, yeah, what I'm currently doing in the test may not actually work
I was just trying to get the rule to work at all earlier
w
@hundreds-breakfast-49010: and it is
i think that using HydratedTarget might just be a bit of an abuse here. strawman would be: 
PythonSourcesTarget
or something.
this is related to #4535, because we aren't using typed targets yet.
h
I was going off of your comment on https://github.com/pantsbuild/pants/pull/8284
as I understood it, having the rule consume a value of type 
HydratedTarget
allowed it to not have to deal with the fact that 
TargetAdaptor
would in practice be 
TargetAdaptor
or any subclass thereof
w
yep
so you need "some" concrete type
i donno. maybe introducing inheritance would be ok here. but... it's ... a bit of a can of worms.
e
Its probably nice to avoid and instead expend effort on making the failures comprehensible by average joes like us. Its highly non-intuitive.
w
the shape of the things you want to pass here is basically "all python targets"... that's the connection to 4535
yep, totally. in this case that codepath i linked at the very, very top should definitely do a "did you mean X?"
should be quick. i can do that on this plane
e
Thanks! That would be amazing.
w
are you also about to be on a plane?
SLC?
e
Yup
w
k, safe travels!
e
You too!
h
have a good flight john
and stu
w
so, with this change in place, the error looks like:
Copy code
Exception: No installed @rules can compute SourceRootStrippedSources for input Params((HydratedTarget+SourceRootConfig)), but there were rules that could compute it using:
  Params((Address+OptionsBootstrapper))
  Params((Address+SourceRootConfig))
you'd still need to identify the need for the 
RootRule(HydratedTarget)
to allow that as a parameter, but i'm not sure that we can "guess" that that is what someone should do in general
h
yeah having the error message is useful without trying to be too smart
w
Open in Slack
PreviousNext
Powered by