RFR(XS): 8211837: Creation of the default CDS Archive should depend on ENABLE_CDS

[Doerr, Martin]

+1

Thanks for improving the code.

Best regards,
Martin


-----Original Message-----
From: Lindenmaier, Goetz 
Sent: Montag, 8. Oktober 2018 12:36
To: Volker Simonis <volker.simonis at gmail.com>; Doerr, Martin <martin.doerr at sap.com>
Cc: build-dev <build-dev at openjdk.java.net>; hotspot-runtime-dev at openjdk.java.net runtime <hotspot-runtime-dev at openjdk.java.net>
Subject: RE: RFR(XS): 8211837: Creation of the default CDS Archive should depend on ENABLE_CDS

Still looks good (now touches much more scenarios, but we should 
get this straight.)

Best regards,
  Goetz.

> -----Original Message-----
> From: hotspot-runtime-dev <hotspot-runtime-dev-
> bounces at openjdk.java.net> On Behalf Of Volker Simonis
> Sent: Montag, 8. Oktober 2018 12:11
> To: Doerr, Martin <martin.doerr at sap.com>
> Cc: build-dev <build-dev at openjdk.java.net>; hotspot-runtime-
> dev at openjdk.java.net runtime <hotspot-runtime-dev at openjdk.java.net>
> Subject: Re: RFR(XS): 8211837: Creation of the default CDS Archive should
> depend on ENABLE_CDS
> 
> Hi Martin, Goetz,
> 
> thanks for reviewing my patch! As Aleksey posted a similar patch for
> fixing the Zero build I've extended my patch to handle
> zero/minimal/core as well.
> 
> In fact the patch now disables CDS on AIX, minimal, core and zero
> unless the user explicitly requests it with the help of
> `--with-jvm-features=cds`. The configuration variant with
> `--with-jvm-features=-cds` should now also be handled correctly (I
> hope caught all possible combinations :)
> 
> If CDS is disabled, the creation of the default class list and default
> archive will now be disabled as well.
> 
> @Aleksey Shipilev : can you please check if this new version of my
> patch also works for you?
> 
> http://cr.openjdk.java.net/~simonis/webrevs/2018/8211837.v1/
> 
> Thank you and best regards,
> Volker
> On Mon, Oct 8, 2018 at 11:10 AM Doerr, Martin <martin.doerr at sap.com>
> wrote:
> >
> > Hi Volker,
> >
> > looks good. Thanks for fixing.
> > Of course, it would be great if this could be used to fix minimal/zero build,
> too.
> >
> > Best regards,
> > Martin
> >
> >
> > -----Original Message-----
> > From: hotspot-runtime-dev <hotspot-runtime-dev-
> bounces at openjdk.java.net> On Behalf Of Volker Simonis
> > Sent: Montag, 8. Oktober 2018 10:19
> > To: build-dev <build-dev at openjdk.java.net>; hotspot-runtime-
> dev at openjdk.java.net runtime <hotspot-runtime-dev at openjdk.java.net>
> > Subject: RFR(XS): 8211837: Creation of the default CDS Archive should
> depend on ENABLE_CDS
> >
> > Hi,
> >
> > can I please have a review for the following trivial build fix:
> >
> > http://cr.openjdk.java.net/~simonis/webrevs/2018/8211837/
> > https://bugs.openjdk.java.net/browse/JDK-8211837
> >
> > It makes no sense to try to create a default CDS archive on VMs which
> > don't support CDS at all. We already have '--enable_cds' which
> > defaults to 'true' on all platforms except AIX.
> >
> > The check for '--enable_cds_archive' should use the result of
> > '--enable_cds' (which is saved in "ENABLE_CDS") and only enable
> > default archive creation if CDS is enabled.
> >
> > Failure to do so currently breaks the AIX build (after JDK-)8202951 was
> pushed).
> >
> > Thank you and best regards,
> > Volker