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