On Thu, 17 Oct 2024 01:28:41 GMT, Calvin Cheung <cche...@openjdk.org> wrote:
>> Summary of changes:
>>
>> Before dumping info the archive, all the module names from `--add-modules`
>> will be sorted and then concatenated into one string with comma as the
>> separator between module names.
>>
>> During runtime, same function will be used to obtain the string in the same
>> format with sorted module names. The string will be compared with the one
>> from the archive to determine if the same module names were specified during
>> dump time.
>>
>> ModuleBootstrap.java
>> The `addModulesFromRuntimeImage` method is added to check if the
>> modules in `addModules` are from the runtime image. If any of the modules
>> isn't in the runtime image, archiving will be disabled.
>>
>> ArchivedModuleGraph.java
>> The constructor has an addition argument `Set addModules`
>> The `get` method also has an addition argument `Set addModules`. It
>> returns the `archivedModuleGraph` only if both the `mainModule` and the
>> `addModules` are the same as the input arguments.
>>
>> Passed tiers 1 - 4 testing.
>
> Calvin Cheung has updated the pull request incrementally with one additional
> commit since the last revision:
>
> @rose00 comment
src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java line 467:
> 465: if (CDS.isDumpingStaticArchive()
> 466: && !haveUpgradeModulePath
> 467: && (addModules.isEmpty() ||
> addModulesFromRuntimeImage(addModules))
I think it would be better to check the modules in the Configuration as that is
what will be archived. Can you try this, calling addJrt(cf, addModules) where
this method is below (it's similar to allJrtOrModularJar that we added
recently, not tested btw).
/**
* Returns true if all modules named in the given set are in the
Configuration and
* the run-time image.
*/
private static boolean allJrt(Configuration cf, Set<String> moduleNames) {
return !moduleNames.stream()
.map(mn -> cf.findModule(mn).orElseThrow())
.map(m -> m.reference().location().orElseThrow())
.anyMatch(uri -> !uri.getScheme().equalsIgnoreCase("jrt"));
}
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21553#discussion_r1806342360
