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