https://pantsbuild.org/ logo
#development
Title
# development
w

witty-crayon-22786

09/20/2019, 10:51 PM
OR tweaking the rust code to say "did you mean X...?"
h

hundreds-breakfast-49010

09/20/2019, 10:55 PM
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

witty-crayon-22786

09/20/2019, 10:56 PM
no, it's not empty i don't think
i'm fiddling with the block linked above to just
println(..)
the roots
h

hundreds-breakfast-49010

09/20/2019, 10:56 PM
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

witty-crayon-22786

09/20/2019, 10:56 PM
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

hundreds-breakfast-49010

09/20/2019, 10:58 PM
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

witty-crayon-22786

09/20/2019, 10:59 PM
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

hundreds-breakfast-49010

09/20/2019, 11:01 PM
this is what I get in my test. it looks like there's four nodes that yield a
SourceRootStrippedSources
from something
w

witty-crayon-22786

09/20/2019, 11:02 PM
blue nodes are roots
h

hundreds-breakfast-49010

09/20/2019, 11:02 PM
huh, so something is making
SourceRootStrippedSources
a root rule when it shouldn't be?
w

witty-crayon-22786

09/20/2019, 11:03 PM
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

hundreds-breakfast-49010

09/20/2019, 11:06 PM
hm I dfined both RootRule(SourceRootConfig) and optinable_rule(SourceRootConfig), that's probably not right
w

witty-crayon-22786

09/20/2019, 11:06 PM
so, then: why isn't there a root that will accept a
HydratedTarget
? ... because there is no
RootRule
for HydratedTarget.
☝️ 1
h

hundreds-breakfast-49010

09/20/2019, 11:06 PM
but I got the same error with either of them alone
e

enough-analyst-54434

09/20/2019, 11:06 PM
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

hundreds-breakfast-49010

09/20/2019, 11:06 PM
I'm creating a HydratedTarget manually for this test, so I didn't make a RootRule for it
w

witty-crayon-22786

09/20/2019, 11:06 PM
SourceRootConfig is not the missing thing
h

hundreds-breakfast-49010

09/20/2019, 11:06 PM
that probably exists in the real rule graph though
w

witty-crayon-22786

09/20/2019, 11:07 PM
this is not the real rule graph
h

hundreds-breakfast-49010

09/20/2019, 11:07 PM
actually is having both optionable_rule(SourceRootConfig) and RootRule(SourceRootConfig), harmful?
w

witty-crayon-22786

09/20/2019, 11:07 PM
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

enough-analyst-54434

09/20/2019, 11:08 PM
But, conceptually what is wrong with thinking you should be able to short-circuit anywhere by injecting a concrete thing?
w

witty-crayon-22786

09/20/2019, 11:08 PM
the rulegraph is compiled ahead of time
we're not dynamically computing entrypoints
h

hundreds-breakfast-49010

09/20/2019, 11:08 PM
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

witty-crayon-22786

09/20/2019, 11:08 PM
yea.
e

enough-analyst-54434

09/20/2019, 11:08 PM
So then the real problem here is intrinsics
h

hundreds-breakfast-49010

09/20/2019, 11:09 PM
I think I do want to mock a
HydratedTarget
for this test
w

witty-crayon-22786

09/20/2019, 11:09 PM
@hundreds-breakfast-49010: so, looking at the actual code for the rule, it looks like you're wrapping a TargetAdaptor in a HydratedTarget
h

hundreds-breakfast-49010

09/20/2019, 11:09 PM
yeah, I don't know if that's correct
w

witty-crayon-22786

09/20/2019, 11:09 PM
probably not
(it works, obviously, but you're faking the other arguments to the HydratedTarget constructor)
h

hundreds-breakfast-49010

09/20/2019, 11:10 PM
well it works in the sense that the rule outputs something now, not in the sense that the test usefully exercises the rule
w

witty-crayon-22786

09/20/2019, 11:10 PM
@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

hundreds-breakfast-49010

09/20/2019, 11:13 PM
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

witty-crayon-22786

09/20/2019, 11:13 PM
@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

hundreds-breakfast-49010

09/20/2019, 11:15 PM
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

witty-crayon-22786

09/20/2019, 11:30 PM
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

enough-analyst-54434

09/20/2019, 11:32 PM
Its probably nice to avoid and instead expend effort on making the failures comprehensible by average joes like us. Its highly non-intuitive.
w

witty-crayon-22786

09/20/2019, 11:32 PM
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

enough-analyst-54434

09/20/2019, 11:35 PM
Thanks! That would be amazing.
w

witty-crayon-22786

09/20/2019, 11:36 PM
are you also about to be on a plane?
SLC?
e

enough-analyst-54434

09/20/2019, 11:36 PM
Yup
w

witty-crayon-22786

09/20/2019, 11:36 PM
k, safe travels!
e

enough-analyst-54434

09/20/2019, 11:36 PM
You too!
h

hundreds-breakfast-49010

09/20/2019, 11:36 PM
have a good flight john
and stu
w

witty-crayon-22786

09/21/2019, 12:26 AM
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

hundreds-breakfast-49010

09/21/2019, 12:33 AM
yeah having the error message is useful without trying to be too smart
w

witty-crayon-22786

09/21/2019, 12:43 AM