RFR: 8319343: Improve CDS module graph support for --add-modules option [v2]
Calvin Cheung
ccheung at openjdk.org
Mon Oct 21 16:32:44 UTC 2024
On Mon, 21 Oct 2024 13:17:10 GMT, Alan Bateman <alanb at openjdk.org> wrote:
>> 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.
I've removed the `if` check at the beginning of the method.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21553#discussion_r1809136528
More information about the core-libs-dev
mailing list