RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v10]
Severin Gehwolf
sgehwolf at openjdk.org
Mon Dec 4 19:15:36 UTC 2023
On Tue, 28 Nov 2023 23:16:07 GMT, Mandy Chung <mchung at openjdk.org> wrote:
>> Severin Gehwolf has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Tighten ModifiedFilesExitTest
>>
>> Ensure the error message is reasonable and doesn't include
>> Exceptions presented to the user.
>
> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java line 452:
>
>> 450: String addOptionsGlob = "glob:" + AddOptionsPlugin.OPTS_FILE;
>> 451: String saveJlinkOptsGlob = "glob:/jdk.jlink/" + JlinkTask.OPTIONS_RESOURCE;
>> 452: String additionalPatterns = systemModulesPattern + "," +
>
> Suggest to have each plugin to define `Plugin::excludeResourcesPattern` to return a non-empty string if linking from the run-time image. Construct this pattern using the API.
OK. Each plugin returns the pattern it needs to exclude now.
> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java line 475:
>
>> 473: // SystemModulesMap class isn't guaranteed to be correct for the
>> 474: // current module set.
>> 475: if (systemModulesPlugin == null) {
>
> Suggest to fail if system modules plugin does not exist which should not happen. The `disableFastPath` system property was added for testing only.
OK. `--disable-plugin system-modules` no longer allowed for run-time image links.
> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/AddRunImageResourcesPlugin.java line 56:
>
>> 54: * resources. Needed for the the run-time image based jlink.
>> 55: */
>> 56: public final class AddRunImageResourcesPlugin extends AbstractPlugin {
>
> This resource file is generated if jdk.jlink is linked in the resulting image. Maybe this plugin can be named `GenerateJlinkResourcesListPlugin` or `JlinkResourcesListPlugin`???
I went with `JlinkResourcesListPlugin`.
> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/AddRunImageResourcesPlugin.java line 184:
>
>> 182: } catch (RunImageLinkException e) {
>> 183: // RunImageArchive::RunImageFile.content() may throw this when
>> 184: // getting the content(). Propagate this specific exeption.
>
> Suggestion:
>
> // getting the content(). Propagate this specific exception.
Should be fixed.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1414364516
PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1414365118
PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1414363725
PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1414363091
More information about the core-libs-dev
mailing list