On Wed, 5 Nov 2025 14:15:08 GMT, David Beaumont <[email protected]> wrote:
>> Creates a new, narrowed API explicitly for use by jlink, which view the >> resource entries in a jimage file without the re-mapping of names and >> invention of synthetic entries inherent in ImageReader. >> >> Another good reason to express this new API as something other than >> ImageReader is that, to fix issues such as >> [JDK-8357249](https://bugs.openjdk.org/browse/JDK-8357249), we don't want to >> have the (System)ImageReader class used directly in jlink code. It's just >> the wrong abstraction and will make it harder to refactor jlink to use a >> non-singleton API with a controlled lifetime later. >> >> I've not added unit tests for the new API (yet), but the fact the >> PackagedModulesVsRuntimeImageLinkTest passes with preview content in the >> jimage file means that it's working as expected. > > David Beaumont has updated the pull request incrementally with one additional > commit since the last revision: > > Tidied up diff code in test. src/java.base/share/classes/jdk/internal/jimage/BasicImageReader.java line 365: > 363: if (name.startsWith("/modules/") || > name.startsWith("/packages/")) { > 364: throw new IllegalArgumentException("Invalid entry > name: " + name); > 365: } This is already covered by the == null case below. The distinguished exception isn't necessary. src/java.base/share/classes/jdk/internal/jimage/ResourceEntries.java line 35: > 33: * <p>This API is designed only for use by the jlink classes, which > manipulate > 34: * jimage files directly. For inspection of runtime resources, it is > vital that > 35: * {@code previewMode} is correctly observed, making this API unsuitable. This would be more clearly stated as use API xxx to access the classes and resources of a runtime. src/java.base/share/classes/jdk/internal/jimage/ResourceEntries.java line 56: > 54: * name (e.g. {@code "/<module-name>/..."}). > 55: */ > 56: Stream<String> entryNamesIn(String module); Are these names useful with any ImageReader api other than sizeOf or open? src/java.base/share/classes/jdk/internal/jimage/ResourceEntries.java line 56: > 54: * name (e.g. {@code "/<module-name>/..."}). > 55: */ > 56: Stream<String> entryNamesIn(String module); Those extra words at the end of the method name are not consistent with naming conventions in openjdk. Drop the "In". ------------- PR Review Comment: https://git.openjdk.org/valhalla/pull/1721#discussion_r2494836681 PR Review Comment: https://git.openjdk.org/valhalla/pull/1721#discussion_r2494742427 PR Review Comment: https://git.openjdk.org/valhalla/pull/1721#discussion_r2494774562 PR Review Comment: https://git.openjdk.org/valhalla/pull/1721#discussion_r2494788265
