https://pantsbuild.org/ logo
a

ancient-vegetable-10556

02/03/2022, 9:13 PM
@witty-crayon-22786 Can I get a sanity check on this? https://github.com/pantsbuild/pants/pull/14352
w

witty-crayon-22786

02/03/2022, 9:35 PM
yea, seems fine… although I think that maybe moving the awareness of the prefix into
JvmProcess
, and then converting into either a
Process
(with an empty prefix) or an
InteractiveProcess
(with a non-empty prefix) might avoid the
__
assumption?
a

ancient-vegetable-10556

02/03/2022, 9:36 PM
hrm.
w

witty-crayon-22786

02/03/2022, 9:39 PM
it would mean processes would need to be written with an awareness that they might not be in the same directory as their arguments… but i think that that would likely be good anyway (to the point where I’m a bit surprised that REAPI doesn’t support it natively)?
a

ancient-vegetable-10556

02/03/2022, 9:46 PM
That seems like a big deal for now?
w

witty-crayon-22786

02/03/2022, 9:48 PM
maybe. it’s just that changing a filename or adding a new filename in
JdkSetup
might silently break run without good testing.
a

ancient-vegetable-10556

02/03/2022, 9:49 PM
If we’re including random shell scripts in places, it seems like it’d be hard to avoid that?
w

witty-crayon-22786

02/03/2022, 9:49 PM
having said that, if
JvmProcess
did have support for the template variable, you could still forget to include the prefix on a relative argument and break a test silently
yea.
a

ancient-vegetable-10556

02/03/2022, 9:50 PM
that’s really the trick. But testing the
run
infrastructure should be enough to make sure that everything is written properly
w

witty-crayon-22786

02/03/2022, 9:50 PM
k.
imo doing something to encode the
__
assumption would be good… or making it a constant or something
a

ancient-vegetable-10556

02/03/2022, 9:51 PM
ah right. I think I did that properly somewhere else, too