On Mon, 12 Feb 2024 12:51:50 GMT, Daniel Fuchs <dfu...@openjdk.org> wrote:

>> Would it be possible to change the `createJavaProcessBuilder` method in 
>> ProcessTools to include something to check if -cp is already an argument 
>> being passed?
>> 
>> maybe something like this?
>> 
>> 
>> if (!noCP && !args.contains("-cp")) {
>>             args.add("-cp");
>>             args.add(System.getProperty("java.class.path"));
>>         }
>
> That's not a bad idea. What I would recommend in that case would be to not do 
> anything here - but rather log another issue against 
> `ProcessTools.createTestJavaProcessBuilder`. 
> Then proceed with this PR without changing anything...

Hello Darragh, Daniel,

As for introducing a new method on `ProcessTools` class, I don't know if it's 
needed - I feel that the `ProcessTools` class is already getting too complex 
because of the various similarly named methods. I think it might be better to 
reuse some of those existing `/test/lib` utility methods to achieve the same, 
something like:


final List<String> command = new ArrayList<>();
command.add(JDKToolFinder.getJDKTool("java"));
command.addAll(jdk.test.lib.Utils.prependTestJavaOpts("-cp", classpath, 
className, appArgs));
final ProcessBuilder pb = new ProcessBuilder(command);
final OutputAnalyzer outputAnalyzer = ProcessTools.executeCommand(pb);


I haven't tried out this snippet to be sure this works as expected.

I am not suggesting we do this change in this current PR.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/17787#discussion_r1490909855

Reply via email to