RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v10]

[Severin Gehwolf]

On Tue, 28 Nov 2023 21:46:58 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/plugins/AddRunImageResourcesPlugin.java line 67:
> 
>> 65:     // RunImageArchive for further processing.
>> 66:     private static final String RESPATH = RESPATH_PREFIX + "%s_resources";
>> 67:     private static final String JLINK_MOD_NAME = "jdk.jlink";
> 
> There are 2 other occurrences of "jdk.jlink".  I guess you plan to replace them with the constant variable?

I think I've replaced them in the latest update.

> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/AddRunImageResourcesPlugin.java line 116:
> 
>> 114:         if (platform == null) {
>> 115:             throw new IllegalStateException("java.base not part of the image?");
>> 116:         }
> 
> Can simply use `orElseThrow`.  It should not reach here and so it's ok to use InternalError or AssertionError (which is also used in this code).
> 
> Suggestion:
> 
>         String platform = in.moduleView().findModule("java.base")
>                 .map(ResourcePoolModule::targetPlatform)
>                 .orElseThrow(() -> new AssertionError("java.base not found"));

Done.

> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/AddRunImageResourcesPlugin.java line 121:
> 
>> 119: 
>> 120:     private void addModuleResourceEntries(ResourcePoolBuilder out) {
>> 121:         for (String module: keysInSortedOrder()) {
> 
> Suggestion:
> 
>               nonClassResEntries.keySet().stream().sorted().forEach(module -> {
> 
> 
> `keysInSortedOrder` can be removed.

removed.

> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/AddRunImageResourcesPlugin.java line 126:
> 
>> 124:             if (mResources == null) {
>> 125:                 throw new AssertionError("Module listed, but no resources?");
>> 126:             }
> 
> Since it is always non-null, this check can be dropped.  NPE will be thrown if such bug exists.

I've replaced this with `Object.requireNonNull()`.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1414357943
PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1414358486
PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1414359291
PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1414359118