https://pantsbuild.org/ logo
Join Slack
Powered by
re: <https://github.com/pantsbuild/pants/pull/8369...
# development
h
i think the implication of this comment
are using an incorrect abstraction for running user code (whihc should not be cached and should be able to do anything at all on the system)
and we don't currently have the correct abstraction built, although it should probably look similar to 
ExecuteProcessRequest
-> 
ExecuteProcessResult
so, we need a rule whose input is something like an 
ExecuteProcessRequest
(maybe 
LocalExecuteProcessRequest
?) that never caches the result (since a user program should be completely nondeterministic from pants' perspective)
w
it's less about running "user code" (tests are user code, for example)
but yes
er
so, the design doc describes one approach here.
BUT, there is an aspect it didn't consider, which is the "should be able to modify the workspace" aspect
and if not "modify", then "read from" at least
h
by "design doc" do you mean the google doc mentioned in that thread? if so I need to be granted access to it
w
and that... makes it a pretty different case from what we were thinking initially.
@hundreds-breakfast-49010: it's available to pants-devel@
that doc talks about processes running in the "foreground". but i don't think we'd considered that they would also potentially need access to the workspace
and that... is interesting. because i'm not exactly sure how to do it, heh.
h
still trying to figure out how to give myself access to that doc, but a user process should have access to everything on the system, right?
w
@hundreds-breakfast-49010: join the pants-devel@googlegroups.com mailing list
h
just as if you had run it from the console with 
python3 <whatever>
w
ping me when you've taken a look
h
ok so that doc suggests adding a new flag on 
ExecuteProcessRequest
(rahter than making a new type)
and that flag would force local execution and grab exclusive access to the console
which I think implies that any 
@rule
that takes as one of its inputs an 
ExecuteProcessRequest
with that flag, will act like a 
@console_rule
in terms of not running concurrently with any other 
@console_rules
w
yep
BUT, the new aspect here that we didn't consider is the filesystem access part
not all usecases will require that... for example, i think that in a "please allow me to debug `pytest`" case, it's actually important that we're in the sandbox
h
yeah, we deliberately don't want to sandbox a python executable the user runs with 
pants run
I don't know how hard that would be to implement, given how the 
ExecuteProcessRequest
rule is currently implemented
w
i think that something like 
./pants repl
could go either way.. in a sandbox could be totally fine.
h
yeah 
repl
is a different story (maybe)
w
@hundreds-breakfast-49010: ...hardish, i think.
h
so maybe we don't want to use that abstraction at all
w
because the sandbox might contain things like the pex itself
h
and instead actually shell out to run the pex, or call the native pex 
run
method
which is what the v1 
run
is doing I believe
w
so a strawman approach might be extract all inputs into a sandbox, but invoke the paths as absolute
(possible, but a bit awkward, because paths are currently relative** to the sandbox)
err, fixed the above.
could potentially require env vars that the process executor could set...? like 
$HERE/my-pex
h
w
yep. and that will run in the sandbox, without access to the workspace.
h
what if instead of yielding an 
ExecuteProcessRequest
it took the 
argv
string and digest that it puts into the 
ExectuteProcessRequest
object, and just ran it with python subprocess?
(and then returned some other type than 
ExecuteProcessRequest
, and was also marked as a 
@console_rule
?)
w
@console_rule
is only at the top of the graph
doing something similar to 
Workspace
and running synchronously in the foreground would be an option. but the downside of that is that it doesn't address the "debug a 
pytest
in the foreground" case i don't think.
h
okay, then that rule would return 
ExecuteProcessRequest
with a local flag, and then the 
@console_rule\ndef run()
rule that the PR also defines could be in charge of running it in the foreground, as a python subprocess
w
if it was "definitely always" going to run in the foreground, i don't think you'd do 
ExecuteProcessRequest
... something else that is 
run
specific, maybe
h
so maybe a new type called 
Runner
(that only 
@console_rule
would be able to access, eventually) - that takes in an 
ExecuteProcessRequest
+ local flag, and runs it
or equivalently a new type that isn't 
ExecuteProcessRequest
w
hm, possibly
one approach would basically be to use 
Workspace
to materialize something into the 
dist
directory, and then expose a simpler API for running it
@hundreds-breakfast-49010: i've gotta run, but this feels like it would be worth creating a 1 design doc for.
back in a bit
h
ah so maybe 
Workspace
itself could have a method on it that does this running
kk
w
yea, possibly. but we'd want to think about the potential connections to the "debug something in the foreground" case. maybe there isn't any.
but should run through the 
run/repl/binary/test
cases in the doc... it was a big oversight to forget about 
run
like that, sorrry
h
I'm not 100% sure what the debugging 
pytest
case that you're talking about actually would entail
w
@hundreds-breakfast-49010: adding a 
pdb.set_breakpoint()
or whatever, and then using the repl that that launches
in a java test debugging case, it would be "the thing opens up a socket and waits for me", so that socket would need to be local
@hundreds-breakfast-49010: made you an editor.
h
@witty-crayon-22786 if you run an arbitrary process using python 
subprocess
, and that process spawns a repl, that repl will be available from the python process that called 
subprocess.run
I just confirmed that this works on my system with a quick and dirty python script that invokes 
pdb.set_trace
, and also with 
ghci
, which spawns its own repl
anyway does the strategy I wrote up in that doc seem reasonable?
Open in Slack
PreviousNext
Powered by