On Tue, 18 Feb 2025 19:29:10 GMT, Jiangli Zhou <jian...@openjdk.org> wrote:

>> Please review this change that adds the `jdk.static` VMProps. It can be used 
>> to skip tests not for running on static JDK.
>> 
>> This also adds a new WhiteBox native method, 
>> `jdk.test.whitebox.WhiteBox.isStatic()`, which is used by VMProps to 
>> determine if it's static at runtime. 
>> 
>> `@requires !jdk.static` is added in 
>> `test/hotspot/jtreg/runtime/modules/ModulesSymLink.java` to skip running the 
>> test on static JDK. This test uses `bin/jlink`, which is not provided on 
>> static JDK. There are other tests that require tools in `bin/`. Those are 
>> not modified by the current PR to skip running on static JDK. Those can be 
>> done after the current change is fully discussed and reviewed/approved.
>
> Jiangli Zhou has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase.

Hello Jiangli, the change to introduce a `@requires` property for identifying a 
static JDK looks OK to me.

> @requires !jdk.static is added in 
> test/hotspot/jtreg/runtime/modules/ModulesSymLink.java to skip running the 
> test on static JDK. This test uses bin/jlink, which is not provided on static 
> JDK. There are other tests that require tools in bin/. Those are not modified 
> by the current PR to skip running on static JDK. Those can be done after the 
> current change is fully discussed and reviewed/approved.

This part however feels odd. Updating this (and other tests in future) to use 
the `@requires !jdk.static` to identify the presence or absence of a specific 
tool in the JDK installation doesn't seem right. Perhaps they should instead 
rely on a tool-specific property (like maybe `@requires jdk.tool.jlink`)?

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

PR Comment: https://git.openjdk.org/jdk/pull/23528#issuecomment-2667657019

Reply via email to