Hi Jiangli, thanks for looking at the change. Unfortunately I've already pushed it in order to fix our nightly builds.
I think the comment should only mention the variants which are explicitly excluded by that function. All other cases are now handled by the "ENABLE_CDS" flag which seems more natural to me. But "ENABLE_CDS" is defined in hotspot.m4 where I put in some comments. Regards, Volker On Mon, Oct 8, 2018 at 8:06 PM Jiangli Zhou <jiangli.z...@oracle.com> wrote: > > Hi Volker, > > Looks good. Thanks for fixing! It would be good to update the following > comment in jdk-options.m4 to add AIX, minimal and core. > > 609 > ################################################################################ > 610 # > 611 # Disable the default CDS archive generation > 612 # cross compilation - disabled > 613 # zero - off by default (not a tested configuration) > 614 # > > > Thanks! > > Jiangli > > > On 10/8/18 3:10 AM, Volker Simonis wrote: > > 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.do...@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-boun...@openjdk.java.net> > >> On Behalf Of Volker Simonis > >> Sent: Montag, 8. Oktober 2018 10:19 > >> To: build-dev <build-dev@openjdk.java.net>; > >> hotspot-runtime-...@openjdk.java.net runtime > >> <hotspot-runtime-...@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 >
