On Thu, 15 Feb 2024 12:04:27 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> 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.
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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17787#discussion_r1490954776
