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

witty-crayon-22786

04/19/2019, 5:55 PM
@happy-kitchen-89482, @gentle-wolf-58752: a sketch of what i was suggesting you could do now, with minor changes to the engine (basically: just implementing #7490): https://gist.github.com/stuhood/10bed72e5dadf9fc36e9b9c96a8a230d ... we'd probably want some magic so that folks didn't need to manually create workunits, and instead had "named rules" or something, but.
👀 1
mm, was a bit broken. fixed now.
...and again. but you get the idea
g

gentle-wolf-58752

04/19/2019, 5:56 PM
👍 got it, thanks!
w

witty-crayon-22786

04/19/2019, 5:57 PM
params are (unfortunately) not yet explained on https://github.com/pantsbuild/pants/blob/master/src/python/pants/engine/README.md ... but the idea is that rather than a single 
Subject
, an 
@rule
can consumes zero or more 
Params
from callers
...and Params propagate without being explicitly passed... the only reason you need the syntax in this case is that you are introducing two new Params into a particular subgraph, rather than one
...by "propagate without being passed", i mean: if there were three or four different rules between 
compile
and 
compile_some_stuff
, they would not need to know about the existence of the `Workunit`: it would be propagated down to the relevant consumer automatically
g

gentle-wolf-58752

04/19/2019, 11:01 PM
@witty-crayon-22786 there are a few bits I’m having trouble connecting. re: 7490, is the syntax you’re referring to in your example the 
Get(Foo, Params((Bar, b), (Baz, c)))
from that issue?
ie. l15 in your gist should be something like
Copy code
yield Get(Classpath, Params((Target, t), (Workunit, w)))
?
w

witty-crayon-22786

04/19/2019, 11:40 PM
Ah, yea: something like that.
I think I prefer the 
dict
syntax, just because Params are unique by 
type
anyway, so the dict sends the right signal
g

gentle-wolf-58752

04/19/2019, 11:52 PM
Ok, so that would be the first change to set up the next bit around (v2, not v1) ’workunit’s
w

witty-crayon-22786

04/19/2019, 11:53 PM
yea, possibly... alternatively, if we added 
@rule(..., description=..)
or something, and then automatically created workunits at those boundaries...?
g

gentle-wolf-58752

04/19/2019, 11:53 PM
Though for l6 there in the gist, would that take a ‘Workunit’ instead of a ‘Logger’ (and access the logger via the ‘Workunit’ passed in)?
w

witty-crayon-22786

04/19/2019, 11:54 PM
yea, maybe. on l3, i raise the question: "do we actually need a separate Logger type"?
i think the answer is "no"
g

gentle-wolf-58752

04/20/2019, 12:00 AM
Got it, so ‘Workunit’ would act more like ‘Console’ (in that it’s not a 
@rule
 Product that is returned by a 
yield Get(...)
but instead is set up in / passed down from `@console_rule`s)
w

witty-crayon-22786

04/20/2019, 12:01 AM
The initial/root Workunit would be created where the Console is now, yea
...and then on the way down the graph, it would be extended
👍 1
It's sortof like a type-safe thread local variable...?
"thread local"... "callstack local"...?
g

gentle-wolf-58752

04/20/2019, 12:08 AM
✔️ I think I’ve got enough to take a swing at it, thanks for talking that through!
👍 1
w

witty-crayon-22786

04/20/2019, 1:27 AM
cc @average-vr-56795, @dry-policeman-7927: ^
finally documented `Params`: https://github.com/pantsbuild/pants/pull/7600 ... only a few months too late =P
Powered by