On Wed, 3 Jun 2026 11:57:19 GMT, Serguei Spitsyn <[email protected]> wrote:
>> Shiv Shah has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> 8209814: Fix manifest capabilities and ProblemList alignment per review
>
> test/jdk/java/lang/instrument/MakeJAR4.sh line 46:
>
>> 44:
>> 45:
>> 46: ${JAR} ${TESTTOOLVMOPTS} cvfm ${AGENT}.jar ${AGENT}.mf ${AGENT}*.class
>> ${OTHER}*.java
>
> There is some inconsistency in the `MakeJAR*.sh` replacements. Many tests now
> use `jdk.test.lib.util.JavaAgentBuilder` for `basicAgent.jar`,
> `redefineAgent.jar`, and `retransformAgent.jar`. The problem is that the
> `JavaAgentBuilder` hard-codes both `Can-Redefine-Classes: true` and
> `Can-Retransform-Classes: true` unless explicitly overridden. The old
> manifests were more specific: `basicAgent.mf` had neither flag,
> `redefineAgent.mf` only had `redefine`, and `retransformAgent.mf`
> only had `retransform`. The tests are supposed to exercise a specific
> manifest capabilities.
>
> Q1: Also, please, look at `-g` option which is used for `javac` compilation.
> Do we still compile with `-g`? I'm afraid that some of the tests may depend
> on this option. Just need to double check. Most likely, this is a false alarm.
>
> Q2: The Java replacements use `CompilerUtils.compile()`, which does not carry
> over `TESTJAVACOPTS` / `TESTTOOLVMOPTS`. I'm not sure if it is important or
> not. Better to double check with @lmesnik .
fixed. explicitly set the missing capability to false for all 23 affected files
to match the old manifests: basicAgent gets both false, redefineAgent gets
Can-Retransform-Classes:false, retransformAgent gets
Can-Redefine-Classes:false, RedefineClassesDisabledTest gets
Can-Retransform-Classes:false. all 80 tests pass.
Q1: 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.
Q2: I think CompilerUtils doesn’t carry these over. will check with @lmesnik
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/31288#discussion_r3357905522
