On Thu, 4 Jun 2026 17:31:04 GMT, Shiv Shah <[email protected]> wrote:
>> test/jdk/java/lang/instrument/AppendToClassPathSetUp.sh line 60:
>>
>>> 58: echo "CLASSPATH=${CLASSPATH}"
>>> 59:
>>> 60: JAVAC="${COMPILEJAVA}/bin/javac -g"
>>
>> Nit: This one and previous shell
>> ([AppendToBootstrapClassPathSetUp.sh](https://github.com/openjdk/jdk/pull/31288/changes#diff-73e9584bfe8b61b51c35650555295e76e7080fd4af7c677bb19d4bcb41649fee))
>> scripts are using `-g` option for `javac` compilation. I'm not sure it is
>> really needed though. But I do not see this option was adopted in java-based
>> build. It can be that I've missed where this option is used. Anyway, just
>> wanted to double check nothing is missed here.
>
>> Nit: This one and previous shell
>> ([AppendToBootstrapClassPathSetUp.sh](https://github.com/openjdk/jdk/pull/31288/changes#diff-73e9584bfe8b61b51c35650555295e76e7080fd4af7c677bb19d4bcb41649fee))
>> scripts are using `-g` option for `javac` compilation. I'm not sure it is
>> really needed though. But I do not see this option was adopted in java-based
>> build. It can be that I've missed where this option is used. Anyway, just
>> wanted to double check nothing is missed here.
>
> jtreg’s @build includes -g by default, so CompilerUtils.compile() doesn’t
> need it explicitly. VerifyLocalVariableTableOnRetransformTest (which depends
> on LVT) passes on all 4 platforms. the only test that explicitly needs -g is
> RetransformWithMethodParametersTest which already has @run compile -g
> -parameters.
Okay, thanks!
>> test/jdk/java/lang/instrument/BootClassPath/BootClassPathTest.sh line 77:
>>
>>> 75: esac
>>> 76:
>>> 77: "$JAVA" ${TESTVMOPTS} ${TESTJAVAOPTS} -classpath "${TESTCLASSES}" Setup
>>> "${TESTCLASSES}" Agent "${CYGWIN}"
>>
>> Nit: Just to notice that the `${CYGWIN}` parameter to the agent is not
>> supported by the `BootClassPathDriver.java`. In fact, I'm not sure if it is
>> important or not. At least, it would be nice to understand better if we
>> still need it for some Windows-like environments.
>
> I think we no longer needed.. tests pass on windows-x64 without it. the old
> shell scripts used it for path conversion but the java conversions handle
> paths natively.
Okay, thanks.
>> test/jdk/java/lang/instrument/modules/AppendToClassPathModuleTest.java line
>> 30:
>>
>>> 28: * @library src /test/lib
>>> 29: * @build test/*
>>> 30: * @run driver BuildModuleAgent
>>
>> Q: The line `@run driver BuildModuleAgent` was added, but `BuildModuleAgent`
>> was not added to any `@build` line. The test only has the line: `@build
>> test/*`. The `BuildModuleAgent.java` is not under `src/test/*` and is not
>> going to be compiled. Is it correct?
>
> @run driver auto-compiles from the test source directory. verified, test
> passes on all 4 platforms including AppendToClassPathModuleTest.
Okay, thanks.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/31288#discussion_r3380365609
PR Review Comment: https://git.openjdk.org/jdk/pull/31288#discussion_r3380388717
PR Review Comment: https://git.openjdk.org/jdk/pull/31288#discussion_r3380376000
