On Thu, 15 Feb 2024 12:44:10 GMT, Daniel Fuchs <dfu...@openjdk.org> wrote:
>> 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.
>
> Hi Jaikiran,
>
> What Darragh suggested was to modify the current method in ProcessTools to
> *not* add -cp <java.class.path> if the argument it is given already contains
> -cp. Since the first -cp added by the ProcessTools should be ignored anyway
> by the compiler (last one wins) - then I think it's a good idea, but to
> follow up outside of the PR.
Sorry Daniel and Darragh, I misunderstood the discussion and thought it was a
proposal to add a new method in that class. What you suggest about changing the
implementation of that existing method, in a separate PR, sounds fine to me.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17787#discussion_r1490991807
