https://pantsbuild.org/ logo
Join Slack
Powered by
<@UKPS08ZGW>, <@U6ZRNH0TC>: re materializing thing...
# development
w
@hundreds-breakfast-49010, @aloof-angle-91616: re materializing things in https://github.com/pantsbuild/pants/pull/8288
can discuss here
yes, thoughts!
a
+1 to Stu’s thoughts 🙂
Also curious as to whether it’s necessary for something you’re doing, or you just prefer the API
☝️ 1
h
we need this to handle 
pants binaries
correctly right?
w
i think he meant: whether that shape for the API is necessary
h
oh, no
this is just what I came up with when talking to @enough-analyst-54434 when he was in sf in person
w
if the 
@console_rule
constraint works, leaning in there would be good
h
but yeah I think we didn't think about the implications of a sde-effect-only rule well enough
a
./pants binary
is an interesting one… The way v1 works is that there’s this top-level 
dist
directory where everything gets dumped… I don’t love it, but it does lend itself to a decently restricted API of “a ConsoleRule can materialize a directory digest specifically into dist and nowhere else”
w
what it might look like is: "@console_rule collects all of the BuiltStrawmanBinary instances, and then uses some synchronous API to publish them"
h
why only `@console_rule`s?
w
@average-vr-56795: that prevents it from being used for 
fmt
h
I was thinking of this as just a generic way for any rule to make changes to the filesystem
not limiting it to just 
dist
either, although that's the first place I planned to use it, making that be the way that the created PEX file got written to 
dist
in the v2 
binaries
rule
w
@hundreds-breakfast-49010: a few reasons. but "concurrency of mutating things" is a big one
and being able to reason about 
@rules
not doing that, in general
h
do we have a way to say that a certain 
@rule
can only be invoked from 
@console_rule
?
a
@average-vr-56795: that prevents it from being used for 
fmt
You know how I feel about 
fmt
😉
w
@hundreds-breakfast-49010: no, not right now. but 
Console
should eventually be enforced to only be accessible from a 
@console_rule
h
how do you feel about 
fmt
?
w
@average-vr-56795: alright. how about 
fix
, which requires a build?
h
fmt
isn't a priority for toolchain right this second, but if we could design the rule such that it works correctly for 
fmt
in the futrue when we eventually port that, that would be useful
a
It shouldn’t exist inside a build tool. imo a build tool should never modify source files, but should provide whatever hooks another tool may need to get information so that it can modify the source tree.
h
@witty-crayon-22786 so we need a more general schema of making some input types only available in certain types of rules, and we could use that for both this work and also 
Console
?
w
@hundreds-breakfast-49010: that was my thinking, yea.
h
ok
a
I’m cool with providing enough hooks to get classpaths out so that something else can do 
fix
- I still don’t think the build tool should be the one doing the fixing 🙂
(I’m not going to die on this hill, but it’s worth mentioning in case it shapes things 🙂)
h
I think building that is not a priority for me/toolchain right this second, but it's easy enough to just only use this new rule within a 
@console_rule
until someone has the bandwidth to write something that will enforce it
w
i think "it's a sycnchronous method call, but is only available in a @console_rule" draws a nice line between sideffect-free stuff and others
h
so if we did enforce the 
@console_rule
-only thing, maybe it's okay to have a rule that only has a side effect?
a
I wonder if we can do something cute like exposing a set of globals in the namespace of `@console_rule`s when they execute…
h
@aloof-angle-91616 was mentioning something about a mock filesystem type in the comments on https://github.com/pantsbuild/pants/pull/8289 (which again was unrelated work that i happened to do on top of the materializing work that I acutally care about)
I was also thinking that we could get rid of the several places in the code where we call the 
materialize_directories
method directly, since this seems unclean
w
i suggested a 
Workspace
type
h
but most of those are not console rules, and they're not priorities for me right this second
w
@hundreds-breakfast-49010: well, v1 calling synchronous methods is "ok-ish", because everything is synchronous
i wouldn't necessarily make that a goal here
h
@witty-crayon-22786 so what would the semantics of this 
Workspace
type be? and this is the type that (in the future) we want only `@console_rule`s to have access to?
I can make an issue for "build infrastructure to forbid requesting certain types as inputs for anything other than a `@console_rule`", but I don't plan to work on it in the near term
w
`Workspace`+`Console`
h
(unless this already exists)
w
i think 
Workspace
is roughly the "mock filestystem" idea, but i'd personally really, really like it to expose absolute nothing aside from "materialize_directory", for now
(or maybe plural/batch)
h
okay
w
because there are better ways to "read the filesystem", and so we should discourage 
@console_rules
from doing that
h
so, maybe the python object of type 
Workspace
would expose the 
materialize_directory
method?
w
yes.
the reason to not merge 
Console
and 
Workspace
is that we have an easy way to DI as many different things as we want
h
so a 
@console_rule
will request a 
Workspace
as an input, then the rule can call that method (sort of like how we're already just calling it on the 
context
in v1 rules now)
and eventually we'll make it impossible for a non-console rule to request 
Workspace
at all
but not yet
w
so there is no reason to have a monolithic injected singleton
yep!
h
for now we will just not use it anywhere other than 
@console_rule
ok
I'll modify my PR to work that way in the interest of getting that code merged (unless @aloof-angle-91616 has any additional input)
💯 1
hm, now that I'm getting to this, I'm realizing that I'm not sure what the best way to hook up a new python type 
Workspace
to the existing rust FFI functionality is
I could either make this new 
Workspace
type invoke the engine somehow, and use the intrinsic I already wrote
or I could use the FFI 
materialize_directories
rust function that I had planned to eventually get rid of - but I'm not sure how I should invoke that from 
Workspace
w
...mm. yes.
Oh, no.
Basically, when you construct the 'Workspace', give it a private _scheduler field
👍 1
This will be right near where we construct `Console`: (a function called "run_console_rules", iirc)
the 
Workspace
would be installed as a 
RootRule
👍 1
and that codepath (which is the only place that actually runs `@console_rule`s) would construct one, and pass it down.
a
slack didn't record my liking of any messages in this thread
this is great
h
slack is very often a bad piece of software
a
irc was so good
open source always wins
Open in Slack
PreviousNext
Powered by