On Mon, 9 Dec 2024 12:31:54 GMT, liyazzi <d...@openjdk.org> wrote:
>> For two cases:
>>
>> 1. When the ImageReaderFactory was loaded by local jdk,that means the
>> ImageReaderFactory was loaded by boot class loader,then init the `Path
>> BOOT_MODULES_JIMAGE` by using `sun.nio.fs.DefaultFileSystemProvider` which
>> is obtained through reflection,due to it is in jdk internal.
>> 2. When loaded by a target jdk, such as jdk8 runtime, then use the Java 8
>> compatible APIs: `FileSystems.getDefault()` to init the
>> `BOOT_MODULES_JIMAGE` field.
>> Then we can avoid the circular dependencies in class loading caused by
>> loading the defaultSystemProvider.
>
> liyazzi has updated the pull request incrementally with one additional commit
> since the last revision:
>
> add '\s' to avoid trailing whiteSpace
The update to ImageReaderFactory is what I suggested in the other PR so that
part is okay.
The proposed test is very different to what I would have expected, meaning I
was surprised to see it setting BOOT_MODULES_JIMAGE and needing to run with
--add-exports. It can be massively simplified with `@build` tag to compile the
test provider, using ToolProvider to run jlink with `--add-options` to set the
system provider to override the default provider, e.g.
String mlib = Path.of(System.getProperty("test.classes"),
"modules").toString();
String imageDir = "myimage";
ToolProvider jlink = ToolProvider.findFirst("jlink").orElseThrow();
int res = jlink.run(System.out, System.err,
"--module-path", mlib,
"--add-modules", "customfs",
"--add-options",
"-Djava.nio.file.spi.DefaultFileSystemProvider=customfs.CustomFileSystemProvider",
"--output", imageDir);
if (res != 0) {
throw new RuntimeException("jlink failed");
}
String launcher = Path.of(imageDir, "bin", "java").toString();
String[] cmd = { launcher, "--list-modules" };
var pb = new ProcessBuilder(cmd);
OutputAnalyzer outputAnalyzer = ProcessTools
.executeProcess(pb)
.outputTo(System.err)
.errorTo(System.err);
outputAnalyzer.shouldHaveExitValue(0);
Even better would if we updated
test/jdk/java/nio/file/spi/SetDefaultProvider.java to add this as a test case.
Right now, this test exercises several different configurations and this issue
is really just another configuration.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/22628#issuecomment-2528106421
