On Mon, 21 Oct 2024 05:15:50 GMT, Calvin Cheung <cche...@openjdk.org> wrote:
>> 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"));
>> }
>
> Hi Alan,
> I tried your suggestion but it can't handle the `ALL-SYSTEM` case.
> I made some slight adjustments to your patch as follows:
>
>
> /**
> * 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) {
> if (moduleNames.size() == 1 && moduleNames.contains(ALL_SYSTEM)) {
> return true;
> }
> return !moduleNames.stream()
> .filter(mn -> !mn.equals(ALL_SYSTEM))
> .map(mn -> cf.findModule(mn).orElseThrow())
> .map(m -> m.reference().location().orElseThrow())
> .anyMatch(uri -> !uri.getScheme().equalsIgnoreCase("jrt"));
> }
>
> What do you think?
Good point as ALL-SYSTEM and ALL-MODULE-PATH are allowed in the value. I assume
you don't need to special case the single value case as this is dump time so
not performance critical.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21553#discussion_r1808807468
