On Tue, 31 Oct 2023 09:09:37 GMT, Alan Bateman <al...@openjdk.org> wrote:

> Thanks, it looks correctly now.

Thanks!

> 
> One small question. At ModuleBootstrap L235 we set canArchive as it's okay to 
> archive under specific restrictions. For completeness, shouldn't this set 
> canArchive to CDS.isDumpingStaticArchive? I don't think it matters right now 
> but might be confusing to have canArchive be true when not dumping.

Do you mean L230?

 226             if (!haveModulePath && addModules.isEmpty() && 
limitModules.isEmpty()) {
 227                 systemModules = 
SystemModuleFinders.systemModules(mainModule);
 228                 if (systemModules != null && !isPatched) {
 229                     needResolution = (traceOutput != null);
 230                     canArchive = true;
 231                 }
 232             }

Do you prefer the `canArchive` setting be inside `if 
(CDS.isDumpingStaticArchive())` like the following?

    if (CDS.isDumpingStaticArchive())
        canArchive = true;

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16016#discussion_r1378146393

Reply via email to