On Tue, 31 Oct 2023 09:09:37 GMT, Alan Bateman <[email protected]> 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
