RFR: 8333130: MakeJAR2.sh uses hard-coded JDK version [v2]

[Alex Menkov]

On Fri, 31 May 2024 12:01:14 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> Can I please get a review of this test-only change which addresses https://bugs.openjdk.org/browse/JDK-8333130?
>> 
>> There are a couple of tests `NativeMethodPrefixApp` and `RetransformApp` under `test/jdk/java/lang/instrument/` which launch  the app/test with a `-javaagent:<jar>` pointing to a test specific jar. The test, launched with that `javaagent`, then verifies the test specific details.
>> 
>> In their current form, in order to construct the agent `.jar` and make it available to the jtreg test that's launched, they `@run` a  jtreg `shell` test. This `shell` test uses the `MakeJAR2.sh` script to generate the jar file. The contents of the script is relatively straightforward - it compiles (using `javac`) some boot classpath classes, some agent specific classes and then generates a jar file with the agent classes and a `MANIFEST.MF` which points to the boot classpath classes along with test specific manifest attributes. In a recent PR the `MakeJAR2.sh` script was updated to pass `--enable-preview --release 23` since one of the existing agent class was updated to use the ClassFile API PreviewFeature (https://github.com/openjdk/jdk/pull/13009).
>> 
>> This generated agent jar then is passed as `-javaagent:` to the subsequent `@run main` test which does the actual testing.
>> 
>> So essentially the `shell` test which uses the `MakeJAR2.sh` is merely there to create a jar file that's needed by the actual test. Within the JDK we have several tests which compile other classes and generate jar files using in-process test libraries and the `java.util.spi.ToolProvider` implementations. These 2 tests too can be updated to use a similar technique, to completely get rid of the `MakeJAR2.sh`. Removing the `MakeJAR2.sh` will simplify the ability to pass around value for `--release` when using `--enable-preview`.
>> 
>> The commit in this PR updates these 2 tests to use `@run driver` which then compiles relevant classes (maintaining the semantics of `MakeJAR2.sh`) and generates the agent jar file and then finally launching the test process. As part of the update, I did not retain the `@author` tag from the 2 test definitions, since it's my understanding that we no longer use those. I will add them back if we should retain them.
>> 
>> The tests continue to pass locally with this change. I've also triggered tier testing in our CI for this change.
>
> Jaikiran Pai has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fix Boot-Class-Path value for Windows

The fix looks over-complicated.
I think it can be done by something like:
- `@build NativeMethodPrefixAgent bootreporter.StringIdCallback bootreporter.StringIdCallbackReporter` (maybe `@compile` to specify `--release`)
- use jdk.test.lib.helpers.ClassFileInstaller:

    ClassFileInstaller.writeJar("NativeMethodPrefixAgent.jar",
                                ClassFileInstaller.Manifest.fromString(manifest),
                                "NativeMethodPrefixAgent",
                                "asmlib.Instrumentor",
                                "bootreporter.StringIdCallback",
                                "bootreporter.StringIdCallbackReporter");

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

PR Comment: https://git.openjdk.org/jdk/pull/19495#issuecomment-2143117731