RFR: 8319343: Improve CDS module graph support for --add-modules option [v2]

[Calvin Cheung]

On Fri, 18 Oct 2024 11:26:30 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> 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"));
>     }

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?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21553#discussion_r1808114239