Re: RFR JDK-8209786: gcc 7.3 compiler errors on zLinux
Thanks Magnus, Yes, these libraries are moribound it seems, hence their preference for compile options.. Cheers Andrew Andrew Leonard Java Runtimes Development IBM Hursley IBM United Kingdom Ltd Phone internal: 245913, external: 01962 815913 internet email: andrew_m_leon...@uk.ibm.com From: Magnus Ihse Bursie To: Andrew Leonard , Brian Burkhalter Cc: core-libs-...@openjdk.java.net, build-dev Date: 30/08/2018 13:49 Subject:Re: RFR JDK-8209786: gcc 7.3 compiler errors on zLinux Andrew, If you want to make changes to the build system (files in make/*), please always include build-dev in the reviews. From a build point, this fix looks okay. My general preference is to fix shady code instead of disabling warnings, but in this case it's in two libraries that are either external or moribound, so if the maintainer's of the respective libraries want to avoid code changes, I accept that. /Magnus On 2018-08-30 14:18, Andrew Leonard wrote: > Hi Brian, > Thanks for taking a look at this, I have just done a rebuild with a new > patch with appropriate gcc disable warnings for these libraries: > http://cr.openjdk.java.net/~aleonard/8209786/webrev.01/ > This works fine, so if you think this is a more favourable approach for > these libraries? i'd like to get this merged please. > Thanks > Andrew > > Andrew Leonard > Java Runtimes Development > IBM Hursley > IBM United Kingdom Ltd > Phone internal: 245913, external: 01962 815913 > internet email: andrew_m_leon...@uk.ibm.com > > > > > From: Brian Burkhalter > To: Andrew Leonard > Cc: core-libs-...@openjdk.java.net > Date: 28/08/2018 15:52 > Subject:Re: RFR JDK-8209786: gcc 7.3 compiler errors on zLinux > > > > Hi Andrew, > > It was suggested that it would be preferable to dial down the compilation > settings for the fdlibm code rather than make a source code change. Was > this investigated? > > Thanks, > > Brian > > On Aug 28, 2018, at 7:18 AM, Andrew Leonard > wrote: > > We have discovered issues with gcc 7.3 on zLinux, combined with OpenJDK's > default compiler options has highlighted a couple of native code issues, > with undefined behaviours: > - validating loop test array bounds > - left shifts of negative values > I have created bug https://bugs.openjdk.java.net/browse/JDK-8209786 > and attached the webrev fix here: > http://cr.openjdk.java.net/~aleonard/8209786/webrev.00/ > > This has already been discussed and refined on the "s390x-port-dev" > maillist > and as it was pointed out, it should have been posted here... > > I'd like to request a sponsor for this fix please? > > > > > Unless stated otherwise above: > IBM United Kingdom Limited - Registered in England and Wales with number > 741598. > Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
RE: RFR JDK-8209786: gcc 7.3 compiler errors on zLinux
Thanks Goetz, I've created an hg export below.. I've not used jdk-submit, i've tested it locally on xLinux and zLinux. Cheers Andrew # HG changeset patch # User aleonard # Date 1535641094 -3600 # Thu Aug 30 15:58:14 2018 +0100 # Node ID 592a8ad8d4c16287c316d018a0a402148720718b # Parent 1ddd1ec044311512c55643bed641859e78b9d25e 8209786: disable gcc warnings for libmlib & libfdlibm to enable gcc 7.3 on zLinux diff -r 1ddd1ec04431 -r 592a8ad8d4c1 make/lib/Awt2dLibraries.gmk --- a/make/lib/Awt2dLibraries.gmk Thu Aug 30 09:08:23 2018 -0400 +++ b/make/lib/Awt2dLibraries.gmk Thu Aug 30 15:58:14 2018 +0100 @@ -55,6 +55,7 @@ OPTIMIZATION := HIGHEST, \ CFLAGS := $(CFLAGS_JDKLIB) \ $(BUILD_LIBMLIB_CFLAGS), \ +DISABLED_WARNINGS_gcc := shift-negative-value, \ LDFLAGS := $(LDFLAGS_JDKLIB) \ $(call SET_SHARED_LIBRARY_ORIGIN), \ LIBS := $(JDKLIB_LIBS), \ diff -r 1ddd1ec04431 -r 592a8ad8d4c1 make/lib/CoreLibraries.gmk --- a/make/lib/CoreLibraries.gmkThu Aug 30 09:08:23 2018 -0400 +++ b/make/lib/CoreLibraries.gmkThu Aug 30 15:58:14 2018 +0100 @@ -68,7 +68,7 @@ CFLAGS_linux_ppc64le := -ffp-contract=off, \ CFLAGS_linux_s390x := -ffp-contract=off, \ CFLAGS_linux_aarch64 := -ffp-contract=off, \ - DISABLED_WARNINGS_gcc := sign-compare misleading-indentation, \ + DISABLED_WARNINGS_gcc := sign-compare misleading-indentation array-bounds, \ DISABLED_WARNINGS_microsoft := 4146 4244 4018, \ ARFLAGS := $(ARFLAGS), \ OBJECT_DIR := $(SUPPORT_OUTPUTDIR)/native/$(MODULE)/libfdlibm, \ Andrew Leonard Java Runtimes Development IBM Hursley IBM United Kingdom Ltd Phone internal: 245913, external: 01962 815913 internet email: andrew_m_leon...@uk.ibm.com From: "Lindenmaier, Goetz" To: Andrew Leonard , Brian Burkhalter Cc: "core-libs-...@openjdk.java.net" , "build-dev (build-dev@openjdk.java.net)" Date: 30/08/2018 15:36 Subject:RE: RFR JDK-8209786: gcc 7.3 compiler errors on zLinux Hi Leonard, the change looks good to me. I'll test it tonight, to make sure it runs with our compilers. Did you run it through jdk-submit? If you supply a patch with all the changeset information (like from hg export) that jchecks fine, I'll sponsor this for you. Best regards, Goetz. > -----Original Message- > From: core-libs-dev On Behalf > Of Andrew Leonard > Sent: Donnerstag, 30. August 2018 14:19 > To: Brian Burkhalter > Cc: core-libs-...@openjdk.java.net > Subject: Re: RFR JDK-8209786: gcc 7.3 compiler errors on zLinux > > Hi Brian, > Thanks for taking a look at this, I have just done a rebuild with a new > patch with appropriate gcc disable warnings for these libraries: > http://cr.openjdk.java.net/~aleonard/8209786/webrev.01/ > This works fine, so if you think this is a more favourable approach for > these libraries? i'd like to get this merged please. > Thanks > Andrew > > Andrew Leonard > Java Runtimes Development > IBM Hursley > IBM United Kingdom Ltd > Phone internal: 245913, external: 01962 815913 > internet email: andrew_m_leon...@uk.ibm.com > > > > > From: Brian Burkhalter > To: Andrew Leonard > Cc: core-libs-...@openjdk.java.net > Date: 28/08/2018 15:52 > Subject:Re: RFR JDK-8209786: gcc 7.3 compiler errors on zLinux > > > > Hi Andrew, > > It was suggested that it would be preferable to dial down the compilation > settings for the fdlibm code rather than make a source code change. Was > this investigated? > > Thanks, > > Brian > > On Aug 28, 2018, at 7:18 AM, Andrew Leonard > > wrote: > > We have discovered issues with gcc 7.3 on zLinux, combined with OpenJDK's > default compiler options has highlighted a couple of native code issues, > with undefined behaviours: > - validating loop test array bounds > - left shifts of negative values > I have created bug https://bugs.openjdk.java.net/browse/JDK-8209786 > and attached the webrev fix here: > http://cr.openjdk.java.net/~aleonard/8209786/webrev.00/ > > This has already been discussed and refined on the "s390x-port-dev" > maillist > and as it was pointed out, it should have been posted here... > > I'd like to request a sponsor for this fix please? > > > > > Unless stated otherwise above: > IBM United Kingdom Limited - Registered in England and Wales with number > 741598. > Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 > 3AU Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
Re: [OpenJDK 2D-Dev] RFR JDK-8209786: gcc 7.3 compiler errors on zLinux
Hi, So there seems to be varying opinion here, taking the 2D view point since it is going to be maintained, the opinion seems to be more with the fix ( http://cr.openjdk.java.net/~aleonard/8209786/webrev.00/). This would be my personal preference also, but previous comments seemed to prefer compiler options. Looking at each error: - /libfdlibm/k_rem_pio2.c : This is a simple "for" loop bound check, which with good programming practice should really have been there to prevent ArrayOutOfBounds.. or native overwrites.. Simple fix. - libmlib_image/mlib_ImageLookUp_Bit.c : This is -ve bit shifting which is spec undefined, which in theory could mean it breaks in the future if we upgrade/change compiler... However, this is complex code, we need to be very sure the new fix is "correct", myself and my colleague here have examined it closely and are happy, but i'd appreciate your in-depth analysis please. Magnus, Philip, Brian, Goetz, can we have a vote? => "Fix" or "DisableWarnings" ? Thanks Andrew Andrew Leonard Java Runtimes Development IBM Hursley IBM United Kingdom Ltd Phone internal: 245913, external: 01962 815913 internet email: andrew_m_leon...@uk.ibm.com From: Magnus Ihse Bursie To: Philip Race , Brian Burkhalter Cc: 2d-dev <2d-...@openjdk.java.net>, build-dev , Andrew Leonard , core-libs-dev Date: 31/08/2018 09:27 Subject:Re: [OpenJDK 2D-Dev] RFR JDK-8209786: gcc 7.3 compiler errors on zLinux On 2018-08-31 01:28, Philip Race wrote: > Some day, I'd like to replace a lot of medialib functionality with > something > like the proposed Vector API. But that is far enough away that > medialib needs > to be maintained, and unlike a previous discussion about a similar > issue in > the JPEG library, we are on the hook for maintaining medialib. > So if there is an actual logic error in medialib, I'd prefer to fix it. > If the issue is a false positive, I'd be OK to disable the warning, > but wrapped > in a platform-specific manner. So is it a real error here ? Gcc is emitting a warning due to shifting of negative values, which is deemed undefined behavior by the spec. Is this a real error? Well. Undefined behavior is no good. It can work for now and then suddenly start breaking. The originally suggested code change ( http://cr.openjdk.java.net/~aleonard/8209786/webrev.00 ) seems reasonable to me. It is at the very least much clearer what the intention is. And it's conformant with the spec. So I'd advocate using that fix, if you want to continue supporting the library. /Magnus > > -phil. > > On 8/30/18, 3:37 PM, Brian Burkhalter wrote: >> Hi Andrew, >> >> As noted in the issue comments, the fdlibm C code is obsolescent and >> eventually to be superseded by equivalent Java implementations. As >> for the mediaLib code being moribund I cannot speak but am copying >> 2d-dev which owns java.desktop. >> >> Thanks, >> >> Brian >> >> On Aug 30, 2018, at 6:03 AM, Andrew Leonard >> mailto:andrew_m_leon...@uk.ibm.com>> >> wrote: >> >>> Thanks Magnus, >>> Yes, these libraries are moribound it seems, hence their preference >>> for compile options.. >>> Cheers >>> Andrew >>> >>> Andrew Leonard >>> Java Runtimes Development >>> IBM Hursley >>> IBM United Kingdom Ltd >>> Phone internal: 245913, external: 01962 815913 >>> internet email: andrew_m_leon...@uk.ibm.com >>> <mailto:andrew_m_leon...@uk.ibm.com> >>> >>> >>> >>> >>> From: Magnus Ihse Bursie >> <mailto:magnus.ihse.bur...@oracle.com>> >>> To: Andrew Leonard >> <mailto:andrew_m_leon...@uk.ibm.com>>, Brian Burkhalter >>> mailto:brian.burkhal...@oracle.com>> >>> Cc: core-libs-...@openjdk.java.net >>> <mailto:core-libs-...@openjdk.java.net>, build-dev >>> mailto:build-dev@openjdk.java.net>> >>> Date: 30/08/2018 13:49 >>> Subject: Re: RFR JDK-8209786: gcc 7.3 compiler errors on zLinux >>> >>> >>> >>> >>> >>> Andrew, >>> >>> If you want to make changes to the build system (files in make/*), >>> please always include build-dev in the reviews. >>> >>> From a build point, this fix looks okay. My general preference is to >>> fix shady code instead of disabling warnings, but in this case it's in >>> two libraries that are either external or moribound, so if the >>> maintainer's of the respective librarie
Re: [OpenJDK 2D-Dev] RFR JDK-8209786: gcc 7.3 compiler errors on zLinux
Hi Brian/Goetz, Yes, that seems sensible. I have created a new webrev with fdlibm compiler option disabled, and mediaLib code fixed: http://cr.openjdk.java.net/~aleonard/8209786/webrev.02/ I've built and tested on xLinux and zLinux, with gcc 4.8.4 and 7.3. Are we good to go? Thanks Andrew hg export: # HG changeset patch # User aleonard # Date 1536055438 -3600 # Tue Sep 04 11:03:58 2018 +0100 # Node ID c2523f285c503e8f82f1212b38de1cb54093255e # Parent 3ee91722550680c18b977f0e00b1013323b5c9ef 8209786: JDK12 fails to build on s390x with gcc 7.3 diff -r 3ee917225506 -r c2523f285c50 make/lib/CoreLibraries.gmk --- a/make/lib/CoreLibraries.gmkTue Sep 04 14:47:55 2018 +0800 +++ b/make/lib/CoreLibraries.gmkTue Sep 04 11:03:58 2018 +0100 @@ -68,7 +68,7 @@ CFLAGS_linux_ppc64le := -ffp-contract=off, \ CFLAGS_linux_s390x := -ffp-contract=off, \ CFLAGS_linux_aarch64 := -ffp-contract=off, \ - DISABLED_WARNINGS_gcc := sign-compare misleading-indentation, \ + DISABLED_WARNINGS_gcc := sign-compare misleading-indentation array-bounds, \ DISABLED_WARNINGS_microsoft := 4146 4244 4018, \ ARFLAGS := $(ARFLAGS), \ OBJECT_DIR := $(SUPPORT_OUTPUTDIR)/native/$(MODULE)/libfdlibm, \ diff -r 3ee917225506 -r c2523f285c50 src/java.desktop/share/native/libmlib_image/mlib_ImageLookUp_Bit.c --- a/src/java.desktop/share/native/libmlib_image/mlib_ImageLookUp_Bit.c Tue Sep 04 14:47:55 2018 +0800 +++ b/src/java.desktop/share/native/libmlib_image/mlib_ImageLookUp_Bit.c Tue Sep 04 11:03:58 2018 +0100 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2018, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -259,18 +259,18 @@ } #ifdef _LITTLE_ENDIAN - emask = (mlib_u32)((mlib_s32)(-1)) >> ((4 - (size - i)) * 8); + emask = (~(mlib_u32)0) >> ((4 - (size - i)) * 8); #else - emask = (mlib_s32)(-1) << ((4 - (size - i)) * 8); + emask = (~(mlib_u32)0) << ((4 - (size - i)) * 8); #endif /* _LITTLE_ENDIAN */ ((mlib_u32*)da)[0] = (val1 & emask) | (((mlib_u32*)da)[0] &~ emask); #else /* _NO_LONGLONG */ #ifdef _LITTLE_ENDIAN - mlib_u64 emask = (mlib_u64)((mlib_s64)(-1)) >> ((8 - (size - i)) * 8); + mlib_u64 emask = (~(mlib_u64)0) >> ((8 - (size - i)) * 8); #else - mlib_u64 emask = (mlib_s64)(-1) << ((8 - (size - i)) * 8); + mlib_u64 emask = (~(mlib_u64)0) << ((8 - (size - i)) * 8); #endif /* _LITTLE_ENDIAN */ ((mlib_u64*)da)[0] = (((mlib_u64*)dd_array)[sa[0]] & emask) | (((mlib_u64*)da)[0] &~ emask); @@ -395,9 +395,9 @@ } #ifdef _LITTLE_ENDIAN - emask = (mlib_u32)((mlib_s32)(-1)) >> ((4 - (size - i)) * 8); + emask = (~(mlib_u32)0) >> ((4 - (size - i)) * 8); #else - emask = (mlib_s32)(-1) << ((4 - (size - i)) * 8); + emask = (~(mlib_u32)0) << ((4 - (size - i)) * 8); #endif /* _LITTLE_ENDIAN */ ((mlib_u32*)da)[0] = (dd1 & emask) | (((mlib_u32*)da)[0] &~ emask); @@ -413,9 +413,9 @@ } #ifdef _LITTLE_ENDIAN - emask = (mlib_u64)((mlib_s64)(-1)) >> ((8 - (size - i)) * 8); + emask = (~(mlib_u64)0) >> ((8 - (size - i)) * 8); #else - emask = (mlib_s64)(-1) << ((8 - (size - i)) * 8); + emask = (~(mlib_u64)0) << ((8 - (size - i)) * 8); #endif /* _LITTLE_ENDIAN */ ((mlib_u64*)da)[0] = (dd & emask) | (((mlib_u64*)da)[0] &~ emask); @@ -565,9 +565,9 @@ } #ifdef _LITTLE_ENDIAN - emask = (mlib_u32)((mlib_s32)(-1)) >> ((4 - (size - i)) * 8); + emask = (~(mlib_u32)0) >> ((4 - (size - i)) * 8); #else - emask = (mlib_s32)(-1) << ((4 - (size - i)) * 8); + emask = (~(mlib_u32)0) << ((4 - (size - i)) * 8); #endif /* _LITTLE_ENDIAN */ da[0] = (dd & emask) | (da[0] &~ emask); } Andrew Leonard Java Runtimes Development IBM Hursley IBM United Kingdom Ltd Phone internal: 245913, external: 01962 815913 internet email: andrew_m_leon...@uk.ibm.com From: Brian Burkhalter To: Magnus Ihse Bursie Cc: Andrew Leonard , "Lindenmaier, Goetz" , 2d-dev <2d-...@openjdk.java.net>, build-dev , core-libs-dev , Philip Race Date: 31/08/2018 15:44 Subject:Re: [OpenJDK 2D-Dev] RFR JDK-8209786: gcc 7.3 compiler errors on zLinux On Aug 31, 2018, at 2:28 AM, Magnus Ihse Bursie < magnus.ihse.bur...@oracle.com> wrote: Magnus, Philip, Brian, Goetz, can we have a vote? => "Fix" or "DisableWarnings" ? Note that this decision can be different for the two libraries. I'd argue that the maintainer of each library decides. And if so, it seems to be "comp
Re: [OpenJDK 2D-Dev] RFR JDK-8209786: gcc 7.3 compiler errors on zLinux
Thanks Magnus, Hi Goetz, we have agreement from both library owners, so I think we're good now? Thanks Andrew Andrew Leonard Java Runtimes Development IBM Hursley IBM United Kingdom Ltd Phone internal: 245913, external: 01962 815913 internet email: andrew_m_leon...@uk.ibm.com From: Magnus Ihse Bursie To: Andrew Leonard Cc: Brian Burkhalter , 2d-dev <2d-...@openjdk.java.net>, build-dev , core-libs-dev , "Lindenmaier, Goetz" , Philip Race Date: 04/09/2018 16:41 Subject:Re: [OpenJDK 2D-Dev] RFR JDK-8209786: gcc 7.3 compiler errors on zLinux Looks good to me. /Magnus 4 sep. 2018 kl. 12:11 skrev Andrew Leonard : Hi Brian/Goetz, Yes, that seems sensible. I have created a new webrev with fdlibm compiler option disabled, and mediaLib code fixed: http://cr.openjdk.java.net/~aleonard/8209786/webrev.02/ I've built and tested on xLinux and zLinux, with gcc 4.8.4 and 7.3. Are we good to go? Thanks Andrew hg export: # HG changeset patch # User aleonard # Date 1536055438 -3600 # Tue Sep 04 11:03:58 2018 +0100 # Node ID c2523f285c503e8f82f1212b38de1cb54093255e # Parent 3ee91722550680c18b977f0e00b1013323b5c9ef 8209786: JDK12 fails to build on s390x with gcc 7.3 diff -r 3ee917225506 -r c2523f285c50 make/lib/CoreLibraries.gmk --- a/make/lib/CoreLibraries.gmkTue Sep 04 14:47:55 2018 +0800 +++ b/make/lib/CoreLibraries.gmkTue Sep 04 11:03:58 2018 +0100 @@ -68,7 +68,7 @@ CFLAGS_linux_ppc64le := -ffp-contract=off, \ CFLAGS_linux_s390x := -ffp-contract=off, \ CFLAGS_linux_aarch64 := -ffp-contract=off, \ - DISABLED_WARNINGS_gcc := sign-compare misleading-indentation, \ + DISABLED_WARNINGS_gcc := sign-compare misleading-indentation array-bounds, \ DISABLED_WARNINGS_microsoft := 4146 4244 4018, \ ARFLAGS := $(ARFLAGS), \ OBJECT_DIR := $(SUPPORT_OUTPUTDIR)/native/$(MODULE)/libfdlibm, \ diff -r 3ee917225506 -r c2523f285c50 src/java.desktop/share/native/libmlib_image/mlib_ImageLookUp_Bit.c --- a/src/java.desktop/share/native/libmlib_image/mlib_ImageLookUp_Bit.c Tue Sep 04 14:47:55 2018 +0800 +++ b/src/java.desktop/share/native/libmlib_image/mlib_ImageLookUp_Bit.c Tue Sep 04 11:03:58 2018 +0100 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2018, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -259,18 +259,18 @@ } #ifdef _LITTLE_ENDIAN - emask = (mlib_u32)((mlib_s32)(-1)) >> ((4 - (size - i)) * 8); + emask = (~(mlib_u32)0) >> ((4 - (size - i)) * 8); #else - emask = (mlib_s32)(-1) << ((4 - (size - i)) * 8); + emask = (~(mlib_u32)0) << ((4 - (size - i)) * 8); #endif /* _LITTLE_ENDIAN */ ((mlib_u32*)da)[0] = (val1 & emask) | (((mlib_u32*)da)[0] &~ emask); #else /* _NO_LONGLONG */ #ifdef _LITTLE_ENDIAN - mlib_u64 emask = (mlib_u64)((mlib_s64)(-1)) >> ((8 - (size - i)) * 8); + mlib_u64 emask = (~(mlib_u64)0) >> ((8 - (size - i)) * 8); #else - mlib_u64 emask = (mlib_s64)(-1) << ((8 - (size - i)) * 8); + mlib_u64 emask = (~(mlib_u64)0) << ((8 - (size - i)) * 8); #endif /* _LITTLE_ENDIAN */ ((mlib_u64*)da)[0] = (((mlib_u64*)dd_array)[sa[0]] & emask) | (((mlib_u64*)da)[0] &~ emask); @@ -395,9 +395,9 @@ } #ifdef _LITTLE_ENDIAN - emask = (mlib_u32)((mlib_s32)(-1)) >> ((4 - (size - i)) * 8); + emask = (~(mlib_u32)0) >> ((4 - (size - i)) * 8); #else - emask = (mlib_s32)(-1) << ((4 - (size - i)) * 8); + emask = (~(mlib_u32)0) << ((4 - (size - i)) * 8); #endif /* _LITTLE_ENDIAN */ ((mlib_u32*)da)[0] = (dd1 & emask) | (((mlib_u32*)da)[0] &~ emask); @@ -413,9 +413,9 @@ } #ifdef _LITTLE_ENDIAN - emask = (mlib_u64)((mlib_s64)(-1)) >> ((8 - (size - i)) * 8); + emask = (~(mlib_u64)0) >> ((8 - (size - i)) * 8); #else - emask = (mlib_s64)(-1) << ((8 - (size - i)) * 8); + emask = (~(mlib_u64)0) << ((8 - (size - i)) * 8); #endif /* _LITTLE_ENDIAN */ ((mlib_u64*)da)[0] = (dd & emask) | (((mlib_u64*)da)[0] &~ emask); @@ -565,9 +565,9 @@ } #ifdef _LITTLE_ENDIAN - emask = (mlib_u32)((mlib_s32)(-1)) >> ((4 - (size - i)) * 8); + emask = (~(mlib_u32)0) >> ((4 - (size - i)) * 8); #else - emask = (mlib_s32)(-1) << ((4 - (size - i)) * 8); + emask = (~(mlib_u32)0) << ((4 - (size - i)) * 8); #endif /* _LITTLE_ENDIAN */ da[0] = (dd & emask) | (da[0] &~ emask); } Andrew Leonard Java Runtimes Development IBM Hursley IBM United Kingdom Ltd
Proposal: JDK-8217896: Make better use of LCPUs when building on AIX
Hi, Please can I get a sponsor for this build performance enhancement on AIX, by better determining the number of LCPUs on smt AIX machines: https://cr.openjdk.java.net/~aleonard/8217896/webrev.00/ My testing shows up to 30% improved build times when leveraging the available LCPUs. Many thanks Andrew Andrew Leonard Java Runtimes Development IBM Hursley IBM United Kingdom Ltd Phone internal: 245913, external: 01962 815913 internet email: andrew_m_leon...@uk.ibm.com Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
Re: Proposal: JDK-8217896: Make better use of LCPUs when building on AIX
Thanks David, I just found the reference you mention, it is a little vague, but says maybe as you said. I've reworked and tested the following which seems better: NUM_CORES=`lparstat -m | grep -o "lcpu=[[0-9]]*" | cut -d "=" -f 2` I'll create a new webrev with this. Thanks Andrew Andrew Leonard Java Runtimes Development IBM Hursley IBM United Kingdom Ltd Phone internal: 245913, external: 01962 815913 internet email: andrew_m_leon...@uk.ibm.com From: David Holmes To: Erik Joelsson , Andrew Leonard , build-dev@openjdk.java.net Date: 30/01/2019 02:25 Subject:Re: Proposal: JDK-8217896: Make better use of LCPUs when building on AIX On 30/01/2019 3:05 am, Erik Joelsson wrote: > Hello, > > I'm OK with this, but since I can't verify it at all, I would be more > comfortable if one of the other AIX users would review this as well. Ditto. I did some googling on this and pmcycles is flagged as maybe being deprecated in the future and may require privileges to execute. lparstat -i seems to be more commonly used. David ----- > /Erik > > On 2019-01-29 05:08, Andrew Leonard wrote: >> Hi, >> Please can I get a sponsor for this build performance enhancement on AIX, >> by better determining the number of LCPUs on smt AIX machines: >> https://urldefense.proofpoint.com/v2/url?u=https-3A__cr.openjdk.java.net_-7Ealeonard_8217896_webrev.00_&d=DwIDaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=NaV8Iy8Ld-vjpXZFDdTbgGlRTghGHnwM75wUPd5_NUQ&m=FpNeQfFJBut0AAVlbC0w7o0urDCArfDb4PY1TtMVW4I&s=8vmPiiNU5P6MnXKdE-eyyBm48kgCssjH5VyWHAt5zcM&e= >> My testing shows up to 30% improved build times when leveraging the >> available LCPUs. >> Many thanks >> Andrew >> >> Andrew Leonard >> Java Runtimes Development >> IBM Hursley >> IBM United Kingdom Ltd >> Phone internal: 245913, external: 01962 815913 >> internet email: andrew_m_leon...@uk.ibm.com >> >> >> Unless stated otherwise above: >> IBM United Kingdom Limited - Registered in England and Wales with number >> 741598. >> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 >> 3AU Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
Re: Proposal: JDK-8217896: Make better use of LCPUs when building on AIX
Hiya, I've created and tested a new webrev using lparstat and following your guidelines, webrev.01 is here: http://cr.openjdk.java.net/~aleonard/8217896/webrev.01/ If someone is happy to sponsor and review please? Many thanks Andrew Andrew Leonard Java Runtimes Development IBM Hursley IBM United Kingdom Ltd Phone internal: 245913, external: 01962 815913 internet email: andrew_m_leon...@uk.ibm.com From: Magnus Ihse Bursie To: Andrew Leonard , David Holmes Cc: build-dev@openjdk.java.net Date: 30/01/2019 17:16 Subject:Re: Proposal: JDK-8217896: Make better use of LCPUs when building on AIX On 2019-01-30 17:37, Andrew Leonard wrote: > Thanks David, > I just found the reference you mention, it is a little vague, but says > maybe as you said. I've reworked and tested the following which seems > better: >NUM_CORES=`lparstat -m | grep -o "lcpu=[[0-9]]*" | cut -d "=" -f 2` > I'll create a new webrev with this. Note that you should use $GREP and $CUT; in theory we should also check for lparstat, but we've been sloppy with that for the other cpu checking methods so maybe we can be that for AIX here as well. /Magnus > Thanks > Andrew > > Andrew Leonard > Java Runtimes Development > IBM Hursley > IBM United Kingdom Ltd > Phone internal: 245913, external: 01962 815913 > internet email: andrew_m_leon...@uk.ibm.com > > > > > From: David Holmes > To: Erik Joelsson , Andrew Leonard > , build-dev@openjdk.java.net > Date: 30/01/2019 02:25 > Subject:Re: Proposal: JDK-8217896: Make better use of LCPUs when > building on AIX > > > > On 30/01/2019 3:05 am, Erik Joelsson wrote: >> Hello, >> >> I'm OK with this, but since I can't verify it at all, I would be more >> comfortable if one of the other AIX users would review this as well. > Ditto. I did some googling on this and pmcycles is flagged as maybe > being deprecated in the future and may require privileges to execute. > lparstat -i seems to be more commonly used. > > David > - > >> /Erik >> >> On 2019-01-29 05:08, Andrew Leonard wrote: >>> Hi, >>> Please can I get a sponsor for this build performance enhancement on > AIX, >>> by better determining the number of LCPUs on smt AIX machines: >>> > https://urldefense.proofpoint.com/v2/url?u=https-3A__cr.openjdk.java.net_-7Ealeonard_8217896_webrev.00_&d=DwIDaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=NaV8Iy8Ld-vjpXZFDdTbgGlRTghGHnwM75wUPd5_NUQ&m=FpNeQfFJBut0AAVlbC0w7o0urDCArfDb4PY1TtMVW4I&s=8vmPiiNU5P6MnXKdE-eyyBm48kgCssjH5VyWHAt5zcM&e= > >>> My testing shows up to 30% improved build times when leveraging the >>> available LCPUs. >>> Many thanks >>> Andrew >>> >>> Andrew Leonard >>> Java Runtimes Development >>> IBM Hursley >>> IBM United Kingdom Ltd >>> Phone internal: 245913, external: 01962 815913 >>> internet email: andrew_m_leon...@uk.ibm.com >>> >>> >>> Unless stated otherwise above: >>> IBM United Kingdom Limited - Registered in England and Wales with > number >>> 741598. >>> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 >>> 3AU > > > > > Unless stated otherwise above: > IBM United Kingdom Limited - Registered in England and Wales with number > 741598. > Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
Re: Proposal: JDK-8217896: Make better use of LCPUs when building on AIX
Thanks Magnus, no hurry, next week will be fine. Cheers Andrew Andrew Leonard Java Runtimes Development IBM Hursley IBM United Kingdom Ltd Phone internal: 245913, external: 01962 815913 internet email: andrew_m_leon...@uk.ibm.com From: Magnus Ihse Bursie To: Andrew Leonard Cc: David Holmes , build-dev@openjdk.java.net Date: 31/01/2019 20:10 Subject:Re: Proposal: JDK-8217896: Make better use of LCPUs when building on AIX 31 jan. 2019 kl. 16:01 skrev Andrew Leonard : Hiya, I've created and tested a new webrev using lparstat and following your guidelines, webrev.01 is here: http://cr.openjdk.java.net/~aleonard/8217896/webrev.01/ If someone is happy to sponsor and review please? This looks good to me. I can sponsor, but won't have time to push it until tomorrow or after the weekend, so if anyone else wants to step in feel free to do so. :) /Magnus Many thanks Andrew Andrew Leonard Java Runtimes Development IBM Hursley IBM United Kingdom Ltd Phone internal: 245913, external: 01962 815913 internet email: andrew_m_leon...@uk.ibm.com From:Magnus Ihse Bursie To: Andrew Leonard , David Holmes < david.hol...@oracle.com> Cc:build-dev@openjdk.java.net Date:30/01/2019 17:16 Subject:Re: Proposal: JDK-8217896: Make better use of LCPUs when building on AIX On 2019-01-30 17:37, Andrew Leonard wrote: > Thanks David, > I just found the reference you mention, it is a little vague, but says > maybe as you said. I've reworked and tested the following which seems > better: >NUM_CORES=`lparstat -m | grep -o "lcpu=[[0-9]]*" | cut -d "=" -f 2` > I'll create a new webrev with this. Note that you should use $GREP and $CUT; in theory we should also check for lparstat, but we've been sloppy with that for the other cpu checking methods so maybe we can be that for AIX here as well. /Magnus > Thanks > Andrew > > Andrew Leonard > Java Runtimes Development > IBM Hursley > IBM United Kingdom Ltd > Phone internal: 245913, external: 01962 815913 > internet email: andrew_m_leon...@uk.ibm.com > > > > > From: David Holmes > To: Erik Joelsson , Andrew Leonard > , build-dev@openjdk.java.net > Date: 30/01/2019 02:25 > Subject:Re: Proposal: JDK-8217896: Make better use of LCPUs when > building on AIX > > > > On 30/01/2019 3:05 am, Erik Joelsson wrote: >> Hello, >> >> I'm OK with this, but since I can't verify it at all, I would be more >> comfortable if one of the other AIX users would review this as well. > Ditto. I did some googling on this and pmcycles is flagged as maybe > being deprecated in the future and may require privileges to execute. > lparstat -i seems to be more commonly used. > > David > - > >> /Erik >> >> On 2019-01-29 05:08, Andrew Leonard wrote: >>> Hi, >>> Please can I get a sponsor for this build performance enhancement on > AIX, >>> by better determining the number of LCPUs on smt AIX machines: >>> > https://cr.openjdk.java.net/~aleonard/8217896/webrev.00/ > >>> My testing shows up to 30% improved build times when leveraging the >>> available LCPUs. >>> Many thanks >>> Andrew >>> >>> Andrew Leonard >>> Java Runtimes Development >>> IBM Hursley >>> IBM United Kingdom Ltd >>> Phone internal: 245913, external: 01962 815913 >>> internet email: andrew_m_leon...@uk.ibm.com >>> >>> >>> Unless stated otherwise above: >>> IBM United Kingdom Limited - Registered in England and Wales with > number >>> 741598. >>> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 >>> 3AU > > > > > Unless stated otherwise above: > IBM United Kingdom Limited - Registered in England and Wales with number > 741598. > Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
Re: Proposal: JDK-8217896: Make better use of LCPUs when building on AIX
Thanks for merging Magnus, Cheers Andrew Leonard Java Runtimes Development IBM Hursley IBM United Kingdom Ltd Phone internal: 245913, external: 01962 815913 internet email: andrew_m_leon...@uk.ibm.com From: Magnus Ihse Bursie To: Andrew Leonard Cc: David Holmes , build-dev@openjdk.java.net Date: 31/01/2019 20:10 Subject:Re: Proposal: JDK-8217896: Make better use of LCPUs when building on AIX 31 jan. 2019 kl. 16:01 skrev Andrew Leonard : Hiya, I've created and tested a new webrev using lparstat and following your guidelines, webrev.01 is here: http://cr.openjdk.java.net/~aleonard/8217896/webrev.01/ If someone is happy to sponsor and review please? This looks good to me. I can sponsor, but won't have time to push it until tomorrow or after the weekend, so if anyone else wants to step in feel free to do so. :) /Magnus Many thanks Andrew Andrew Leonard Java Runtimes Development IBM Hursley IBM United Kingdom Ltd Phone internal: 245913, external: 01962 815913 internet email: andrew_m_leon...@uk.ibm.com From:Magnus Ihse Bursie To: Andrew Leonard , David Holmes < david.hol...@oracle.com> Cc:build-dev@openjdk.java.net Date:30/01/2019 17:16 Subject:Re: Proposal: JDK-8217896: Make better use of LCPUs when building on AIX On 2019-01-30 17:37, Andrew Leonard wrote: > Thanks David, > I just found the reference you mention, it is a little vague, but says > maybe as you said. I've reworked and tested the following which seems > better: >NUM_CORES=`lparstat -m | grep -o "lcpu=[[0-9]]*" | cut -d "=" -f 2` > I'll create a new webrev with this. Note that you should use $GREP and $CUT; in theory we should also check for lparstat, but we've been sloppy with that for the other cpu checking methods so maybe we can be that for AIX here as well. /Magnus > Thanks > Andrew > > Andrew Leonard > Java Runtimes Development > IBM Hursley > IBM United Kingdom Ltd > Phone internal: 245913, external: 01962 815913 > internet email: andrew_m_leon...@uk.ibm.com > > > > > From: David Holmes > To: Erik Joelsson , Andrew Leonard > , build-dev@openjdk.java.net > Date: 30/01/2019 02:25 > Subject:Re: Proposal: JDK-8217896: Make better use of LCPUs when > building on AIX > > > > On 30/01/2019 3:05 am, Erik Joelsson wrote: >> Hello, >> >> I'm OK with this, but since I can't verify it at all, I would be more >> comfortable if one of the other AIX users would review this as well. > Ditto. I did some googling on this and pmcycles is flagged as maybe > being deprecated in the future and may require privileges to execute. > lparstat -i seems to be more commonly used. > > David > - > >> /Erik >> >> On 2019-01-29 05:08, Andrew Leonard wrote: >>> Hi, >>> Please can I get a sponsor for this build performance enhancement on > AIX, >>> by better determining the number of LCPUs on smt AIX machines: >>> > https://cr.openjdk.java.net/~aleonard/8217896/webrev.00/ > >>> My testing shows up to 30% improved build times when leveraging the >>> available LCPUs. >>> Many thanks >>> Andrew >>> >>> Andrew Leonard >>> Java Runtimes Development >>> IBM Hursley >>> IBM United Kingdom Ltd >>> Phone internal: 245913, external: 01962 815913 >>> internet email: andrew_m_leon...@uk.ibm.com >>> >>> >>> Unless stated otherwise above: >>> IBM United Kingdom Limited - Registered in England and Wales with > number >>> 741598. >>> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 >>> 3AU > > > > > Unless stated otherwise above: > IBM United Kingdom Limited - Registered in England and Wales with number > 741598. > Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
Proposal: Make custom extension point additions
Hi, I would like to find a sponsor for this change please? To support certain extension capabilities I would like to propose the following patch to the following make files, summary of changes: M make/common/SetupJavaCompilers.gmk ==> Add IncludeCustomExtension hook and allow custom override of DISABLE_WARNINGS and JAVAC_WARNINGS M make/gensrc/GensrcVarHandles.gmk ==> Add IncludeCustomExtension "post" hook M make/lib/Lib-java.management.gmk ==> Add IncludeCustomExtension "post" hook M make/lib/Lib-jdk.management.gmk ==> Add IncludeCustomExtension "post" hook Many thanks, Andrew hg diff -g output: diff --git a/make/common/SetupJavaCompilers.gmk b/make/common/SetupJavaCompilers.gmk --- a/make/common/SetupJavaCompilers.gmk +++ b/make/common/SetupJavaCompilers.gmk @@ -26,13 +26,16 @@ ifndef _SETUP_GMK _SETUP_GMK := 1 +# Include custom extension hook +$(eval $(call IncludeCustomExtension, common/SetupJavaCompilers.gmk)) + include JavaCompilation.gmk -DISABLE_WARNINGS := -Xlint:all,-deprecation,-removal,-unchecked,-rawtypes,-cast,-serial,-dep-ann,-static,-fallthrough,-try,-varargs,-empty,-finally +DISABLE_WARNINGS ?= -Xlint:all,-deprecation,-removal,-unchecked,-rawtypes,-cast,-serial,-dep-ann,-static,-fallthrough,-try,-varargs,-empty,-finally # If warnings needs to be non-fatal for testing purposes use a command like: # make JAVAC_WARNINGS="-Xlint:all -Xmaxwarns 1" -JAVAC_WARNINGS := -Xlint:all -Werror +JAVAC_WARNINGS ?= -Xlint:all -Werror # The BOOT_JAVAC setup uses the boot jdk compiler to compile the tools # and the interim javac, to be run by the boot jdk. diff --git a/make/gensrc/GensrcVarHandles.gmk b/make/gensrc/GensrcVarHandles.gmk --- a/make/gensrc/GensrcVarHandles.gmk +++ b/make/gensrc/GensrcVarHandles.gmk @@ -167,4 +167,7 @@ $(foreach t, $(VARHANDLES_BYTE_ARRAY_TYPES), \ $(eval $(call GenerateVarHandleByteArray,VAR_HANDLE_BYTE_ARRAY_$t,$t))) +# Include custom extension post hook +$(eval $(call IncludeCustomExtension, gensrc/GensrcVarHandles-post.gmk)) + GENSRC_JAVA_BASE += $(GENSRC_VARHANDLES) diff --git a/make/lib/Lib-java.management.gmk b/make/lib/Lib-java.management.gmk --- a/make/lib/Lib-java.management.gmk +++ b/make/lib/Lib-java.management.gmk @@ -67,6 +67,9 @@ $(BUILD_LIBMANAGEMENT): $(call FindLib, java.base, java) +# Include custom extension post hook +$(eval $(call IncludeCustomExtension, lib/Lib-java.management-post.gmk)) + TARGETS += $(BUILD_LIBMANAGEMENT) diff --git a/make/lib/Lib-jdk.management.gmk b/make/lib/Lib-jdk.management.gmk --- a/make/lib/Lib-jdk.management.gmk +++ b/make/lib/Lib-jdk.management.gmk @@ -77,6 +77,9 @@ $(BUILD_LIBMANAGEMENT_EXT): $(call FindLib, java.base, java) +# Include custom extension post hook +$(eval $(call IncludeCustomExtension, lib/Lib-jdk.management-post.gmk)) + TARGETS += $(BUILD_LIBMANAGEMENT_EXT) ############ Andrew Leonard Java Runtimes Development IBM Hursley IBM United Kingdom Ltd Phone internal: 245913, external: 01962 815913 internet email: andrew_m_leon...@uk.ibm.com Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
Re: Proposal: Make custom extension point additions
Thanks Erik, I have moved the "post" hooks after the ###'s as you mentioned and similarly to the end for GensrcVarHandles.gmk. Here is the new hg diff -g patch, I have built it with the latest jdk head and run the jtreg tests successfully. Cheers Andrew diff --git a/make/common/SetupJavaCompilers.gmk b/make/common/SetupJavaCompilers.gmk --- a/make/common/SetupJavaCompilers.gmk +++ b/make/common/SetupJavaCompilers.gmk @@ -26,13 +26,16 @@ ifndef _SETUP_GMK _SETUP_GMK := 1 +# Include custom extension hook +$(eval $(call IncludeCustomExtension, common/SetupJavaCompilers.gmk)) + include JavaCompilation.gmk -DISABLE_WARNINGS := -Xlint:all,-deprecation,-removal,-unchecked,-rawtypes,-cast,-serial,-dep-ann,-static,-fallthrough,-try,-varargs,-empty,-finally +DISABLE_WARNINGS ?= -Xlint:all,-deprecation,-removal,-unchecked,-rawtypes,-cast,-serial,-dep-ann,-static,-fallthrough,-try,-varargs,-empty,-finally # If warnings needs to be non-fatal for testing purposes use a command like: # make JAVAC_WARNINGS="-Xlint:all -Xmaxwarns 1" -JAVAC_WARNINGS := -Xlint:all -Werror +JAVAC_WARNINGS ?= -Xlint:all -Werror # The BOOT_JAVAC setup uses the boot jdk compiler to compile the tools # and the interim javac, to be run by the boot jdk. diff --git a/make/gensrc/GensrcVarHandles.gmk b/make/gensrc/GensrcVarHandles.gmk --- a/make/gensrc/GensrcVarHandles.gmk +++ b/make/gensrc/GensrcVarHandles.gmk @@ -168,3 +168,7 @@ $(eval $(call GenerateVarHandleByteArray,VAR_HANDLE_BYTE_ARRAY_$t,$t))) GENSRC_JAVA_BASE += $(GENSRC_VARHANDLES) + +# Include custom extension post hook +$(eval $(call IncludeCustomExtension, gensrc/GensrcVarHandles-post.gmk)) + diff --git a/make/lib/Lib-java.management.gmk b/make/lib/Lib-java.management.gmk --- a/make/lib/Lib-java.management.gmk +++ b/make/lib/Lib-java.management.gmk @@ -70,3 +70,7 @@ TARGETS += $(BUILD_LIBMANAGEMENT) + +# Include custom extension post hook +$(eval $(call IncludeCustomExtension, lib/Lib-java.management-post.gmk)) + diff --git a/make/lib/Lib-jdk.management.gmk b/make/lib/Lib-jdk.management.gmk --- a/make/lib/Lib-jdk.management.gmk +++ b/make/lib/Lib-jdk.management.gmk @@ -80,3 +80,7 @@ TARGETS += $(BUILD_LIBMANAGEMENT_EXT) + +# Include custom extension post hook +$(eval $(call IncludeCustomExtension, lib/Lib-jdk.management-post.gmk)) + Andrew Leonard Java Runtimes Development IBM Hursley IBM United Kingdom Ltd Phone internal: 245913, external: 01962 815913 internet email: andrew_m_leon...@uk.ibm.com From: Erik Joelsson To: Andrew Leonard , build-dev@openjdk.java.net Date: 31/01/2018 17:40 Subject:Re: Proposal: Make custom extension point additions Hello Andrew, This suggestion looks reasonable to me. Just a minor note on the placement of the post hooks. Especially in the Lib-* files, each individual library, or link entity, is put within ### separator lines. In the two files you modified, since there is only one library declared, this isn't as apparent. I would prefer if the post hook came after all the declared library sections instead of inside one of them. The Gensrc file is more of a snowflake so the placement is reasonable there. /Erik On 2018-01-31 03:21, Andrew Leonard wrote: > Hi, > I would like to find a sponsor for this change please? > To support certain extension capabilities I would like to propose the > following patch to the following make files, summary of changes: > M make/common/SetupJavaCompilers.gmk > ==> Add IncludeCustomExtension hook and allow custom override of > DISABLE_WARNINGS and JAVAC_WARNINGS > M make/gensrc/GensrcVarHandles.gmk > ==> Add IncludeCustomExtension "post" hook > M make/lib/Lib-java.management.gmk > ==> Add IncludeCustomExtension "post" hook > M make/lib/Lib-jdk.management.gmk > ==> Add IncludeCustomExtension "post" hook > > Many thanks, > Andrew > > hg diff -g output: > diff --git a/make/common/SetupJavaCompilers.gmk > b/make/common/SetupJavaCompilers.gmk > --- a/make/common/SetupJavaCompilers.gmk > +++ b/make/common/SetupJavaCompilers.gmk > @@ -26,13 +26,16 @@ > ifndef _SETUP_GMK > _SETUP_GMK := 1 > > +# Include custom extension hook > +$(eval $(call IncludeCustomExtension, common/SetupJavaCompilers.gmk)) > + > include JavaCompilation.gmk > > -DISABLE_WARNINGS := > -Xlint:all,-deprecation,-removal,-unchecked,-rawtypes,-cast,-serial,-dep-ann,-static,-fallthrough,-try,-varargs,-empty,-finally > +DISABLE_WARNINGS ?= > -Xlint:all,-deprecation,-removal,-unchecked,-rawtypes,-cast,-serial,-dep-ann,-static,-fallthrough,-try,-varargs,-empty,-finally > > # If warn
RFR & sponsor: 8248701: On Windows generated modules-deps.gmk can contain backslash-r (CR) characters
Hi, Please can I request a review and sponsor to push the following simple fix to Modules.gmk: http://cr.openjdk.java.net/~aleonard/8248701/webrev.00/ for bug: https://bugs.openjdk.java.net/browse/JDK-8248701 which is to cover situations on Windows where a /r(CR) can cause a build break due to badly generated make dependency. Thank you, Andrew Andrew Leonard Java Runtimes Development IBM Hursley IBM United Kingdom Ltd internet email: andrew_m_leon...@uk.ibm.com Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
RFR: 8252233: Enable debug-image target to support producing a pure debug image package
Hi, Please may I request a sponsor and review for this build enhancement to provide a pure debug "image", for those developers that want to accompany a straight jdk image with a debug-image when needed: https://bugs.openjdk.java.net/browse/JDK-8252233 webrev: http://cr.openjdk.java.net/~aleonard/8252233/webrev.00/ We have been providing this as part of the openj9 builds at AdoptOpenJDK for a while now, and would like to contribute it upstream to openjdk. Thanks Andrew Andrew Leonard Java Runtimes Development IBM Hursley IBM United Kingdom Ltd internet email: andrew_m_leon...@uk.ibm.com Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
RE: RFR: 8252233: Enable debug-image target to support producing a pure debug image package
Hi Severin, So yes I guess maybe "debugsymbols-image" is more precise, we call it "debugimage" at Adopt, see artifacts: https://ci.adoptopenjdk.net/job/build-scripts/job/jobs/job/jdk15u/job/jdk15u-linux-x64-openj9/ that's less of a mouthful and I think we're used to that name, but if the community would prefer that? Cheers Andrew Andrew Leonard Java Runtimes Development IBM Hursley IBM United Kingdom Ltd internet email: andrew_m_leon...@uk.ibm.com From: Severin Gehwolf To: Andrew Leonard , build-dev@openjdk.java.net Date: 27/08/2020 17:20 Subject:[EXTERNAL] Re: RFR: 8252233: Enable debug-image target to support producing a pure debug image package Hi Andrew, On Thu, 2020-08-27 at 16:55 +0100, Andrew Leonard wrote: > Hi, > Please may I request a sponsor and review for this build enhancement to > provide a pure debug "image", for those developers that want to accompany > a straight jdk image with a debug-image when needed: > https://bugs.openjdk.java.net/browse/JDK-8252233 > webrev: http://cr.openjdk.java.net/~aleonard/8252233/webrev.00/ Not really a review... If I understand this correctly it's an image of only *debugsymbols* for native files, right? debug-image as a name seems a bit confusing to me. There is already a concept of 'release', 'fastdebug' and 'slowdebug' builds for HotSpot. Perhaps calling it debugsymbols-image would make more sense? Thanks, Severin Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
RE: RFR: 8252233: Enable debug-image target to support producing a pure debug image package
Hi Erik, Thank you for taking a look at this, what you say makes perfect sense, and provides exactly what we're after. I've taken a look at your patch, and have tested it on my Ubuntu VM and it works a treat. I've also run it at Adopt on Windows & Mac, and produced debug symbol images fine: https://ci.adoptopenjdk.net/view/work%20in%20progress/job/andrew-jdk-windows-x64-hotspot/13/ https://ci.adoptopenjdk.net/view/work%20in%20progress/job/andrew-jdk-mac-x64-hotspot/2/ So looks good to me. Thanks Andrew Andrew Leonard Java Runtimes Development IBM Hursley IBM United Kingdom Ltd internet email: andrew_m_leon...@uk.ibm.com From: Erik Joelsson To: Magnus Ihse Bursie , Andrew Leonard , build-dev@openjdk.java.net Date: 28/08/2020 13:50 Subject:[EXTERNAL] Re: RFR: 8252233: Enable debug-image target to support producing a pure debug image package On 2020-08-28 00:51, Magnus Ihse Bursie wrote: > On 2020-08-27 23:26, Erik Joelsson wrote: >> Hello Andrew, >> >> We certainly appreciate contributions, but this patch as it currently >> looks does not fit well with our current build system model. I'm >> however not against adding a target for your usecase. >> >> On a high level, just as Severin pointed out, the term "debug" is >> used for the type configuration we are building (release vs debug). >> For debug information we have instead chosen the term "symbols", so >> the target should rather be called symbols-image. Now I realize we >> already have that target, for something slightly different. I >> introduced that a long time ago for gcov symbols, with the intention >> of eventually move the main debug symbols into it too, but that never >> happened. I think we can just make that change now and let the >> symbols-image be what you are asking for (in addition to the gcov >> files if those are enabled). This will be one step in the direction >> of a bigger overhaul of the images that I want to do anyway. >> >> On a lower level, in Main.gmk, all targets should be declared as >> calls to the macro SetupTarget and in the correct section of the >> file. In this case it would be around line 418 where the current >> symbols-image is declared. >> >> The new file DebugImage.gmk contains a lot of duplication of logic >> already implemented in Images.gmk, which can quite easily be extended >> to cover your usecase. >> >> So all that said, this is how I would suggest to solve it: >> >> http://cr.openjdk.java.net/~erikj/8252233/webrev.01/index.html > Erik, > > I fully agree with you in your analysis on the proper way to solve > this. However, I have a hard time figuring out how the patch you > supplied will achieve that. :-) Maybe you can help me understand how > this would actually make the debug symbols be added to the symbol image? Sure! In the already present "Debug symbols" section of Images.gmk, we have a bunch of convoluted logic for finding all debugsymbols in the respective modules_libs and modules_cmds dirs and generate rules for copying them to the relevant images. This logic is already parameterized to do the same job for the JRE and JDK images, so at the bottom, there were already two calls to SetupCopyDebuginfo, one for JDK and one for JRE. For these calls to work, there needs to be 2 variables defined specific to the image we are copying to. The list of modules relevant for this image and the target directory, ALL_$1_MODULES and $_IMAGE_DIR respectively. The patch adds the first variable and the second was already defined as we already have symbols image. In Bundles.gmk, we generate the main JDK bundle by filtering out the debugsymbols from the jdk image, then we generate the symbols image by filtering just the debugsymbols from the jdk image, as well as adding everything from the symbols image. The last part is basically only done in case we built with gcov. Now with this patch, the symbols image always contains the full set of debug symbols, so we would end up with duplicate rules as the same files would be copied from two different places. To fix this, I simply removed the JDK image as a source of files for the symbols bundle. The logic in Bundles.gmk is still unreasonably complex, and I hope to fix this at some point, but I think it's out of scope for this change. What I would like to do is change Images.gmk to generate a pure symbols image (basically done with this patch) and a pure JDK image containing exactly what should go into the bundle, then also (optionally) a 3rd image that looks like our current JDK image with the former two combined which is convenient for development use in local builds. /Erik > > /Magnus >> >> With this change, building the
RFR: 8260289: Unable to customize module lists after change JDK-8258411
A problem was found downstream with Eclipse OpenJ9 builds whereby since JDK-8258411 they were unable to extend the module lists. This PR adds a IncludeCustomExtension to the conf/module-loader-map.conf, to enable the module list extension again. Signed-off-by: Andrew Leonard - Commit messages: - 8260289: Unable to customize module lists after change JDK-8258411 Changes: https://git.openjdk.java.net/jdk/pull/2219/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2219&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8260289 Stats: 5 lines in 1 file changed: 4 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2219.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2219/head:pull/2219 PR: https://git.openjdk.java.net/jdk/pull/2219
Re: RFR: 8260289: Unable to customize module lists after change JDK-8258411
On Mon, 25 Jan 2021 13:08:53 GMT, Magnus Ihse Bursie wrote: >> A problem was found downstream with Eclipse OpenJ9 builds whereby since >> JDK-8258411 they were unable to extend the module lists. >> This PR adds a IncludeCustomExtension to the conf/module-loader-map.conf, to >> enable the module list extension again. >> >> Signed-off-by: Andrew Leonard > > make/conf/module-loader-map.conf line 100: > >> 98: # Hook to include the corresponding custom file, if present. >> 99: $(eval $(call IncludeCustomExtension, conf/module-loader-map.conf)) >> 100: > > Using IncludeCustomExtension is a good idea, but you should to this where > module-loader-map.conf is included in common/Modules.gmk, not here. The > `*.conf` files are supposed to be formatted as simple configuration files, > and not include special make commands. Ah ok thanks Magnus, didn't realize that, I will update - PR: https://git.openjdk.java.net/jdk/pull/2219
Re: RFR: 8260289: Unable to customize module lists after change JDK-8258411
On Mon, 25 Jan 2021 13:19:31 GMT, Alan Bateman wrote: >> make/conf/module-loader-map.conf line 100: >> >>> 98: # Hook to include the corresponding custom file, if present. >>> 99: $(eval $(call IncludeCustomExtension, conf/module-loader-map.conf)) >>> 100: >> >> Using IncludeCustomExtension is a good idea, but you should to this where >> module-loader-map.conf is included in common/Modules.gmk, not here. The >> `*.conf` files are supposed to be formatted as simple configuration files, >> and not include special make commands. > > @magicus Is there an ordering issue in common/Modules.gmk? It also invokes > IncludeCustomExtension but this is done before it includes the conf file. @AlanBateman Yes, that's what JDK-8258411 inadvertently changed and I am fixing. I am going to now move the common/Modules.gmk IncludeCustomExtension after the module-loader-map.conf include. - PR: https://git.openjdk.java.net/jdk/pull/2219
Re: RFR: 8260289: Unable to customize module lists after change JDK-8258411
On Mon, 25 Jan 2021 13:27:29 GMT, Magnus Ihse Bursie wrote: >> @magicus Is there an ordering issue in common/Modules.gmk? It also invokes >> IncludeCustomExtension but this is done before it includes the conf file. > >> @AlanBateman Yes, that's what JDK-8258411 inadvertently changed and I am >> fixing. I am going to now move the common/Modules.gmk IncludeCustomExtension >> after the module-loader-map.conf include. > > Andrew, why can't you just use the existing IncludeCustomExtension then to > load your own module-loader-map.conf? Or maybe that's just what you are going > to do? The previous behavior provided the ability to "extend" the upstream MODULE lists, ie.just add OpenJ9 modules to the base module lists, see: https://github.com/ibmruntimes/openj9-openjdk-jdk/blob/01e13c398721628540babac94ac8663be716c0a8/closed/custom/common/Modules.gmk#L21 The present https://github.com/openjdk/jdk/blob/master/make/conf/module-loader-map.conf will simply over-write whatever is set in the BOOT_MODULES list. What I am now going to do is move the IncludeCustomExtension after the module-loader-map.conf inclusion. The other alternative would be to change module-loader-map.conf so it appended to any existing BOOT_MODULES variable, but I think you suggested that file is a straight conf file? @magicus thoughts? - PR: https://git.openjdk.java.net/jdk/pull/2219
Re: RFR: 8260289: Unable to customize module lists after change JDK-8258411
On Mon, 25 Jan 2021 13:41:06 GMT, Andrew Leonard wrote: >>> @AlanBateman Yes, that's what JDK-8258411 inadvertently changed and I am >>> fixing. I am going to now move the common/Modules.gmk >>> IncludeCustomExtension after the module-loader-map.conf include. >> >> Andrew, why can't you just use the existing IncludeCustomExtension then to >> load your own module-loader-map.conf? Or maybe that's just what you are >> going to do? > > The previous behavior provided the ability to "extend" the upstream MODULE > lists, ie.just add OpenJ9 modules to the base module lists, see: > https://github.com/ibmruntimes/openj9-openjdk-jdk/blob/01e13c398721628540babac94ac8663be716c0a8/closed/custom/common/Modules.gmk#L21 > > The present > https://github.com/openjdk/jdk/blob/master/make/conf/module-loader-map.conf > will simply over-write whatever is set in the BOOT_MODULES list. > > What I am now going to do is move the IncludeCustomExtension after the > module-loader-map.conf inclusion. > The other alternative would be to change module-loader-map.conf so it > appended to any existing BOOT_MODULES variable, but I think you suggested > that file is a straight conf file? > > @magicus thoughts? Note, it was the change from "appending" to BOOT_MODULES to just "assigning" that broke the existing behaviour with the JDK-8258411 change. - PR: https://git.openjdk.java.net/jdk/pull/2219
Re: RFR: 8260289: Unable to customize module lists after change JDK-8258411 [v2]
> A problem was found downstream with Eclipse OpenJ9 builds whereby since > JDK-8258411 they were unable to extend the module lists. > This PR adds a IncludeCustomExtension to the conf/module-loader-map.conf, to > enable the module list extension again. > > Signed-off-by: Andrew Leonard Andrew Leonard has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: 8260289: Unable to customize module lists after change JDK-8258411 Signed-off-by: Andrew Leonard - Changes: - all: https://git.openjdk.java.net/jdk/pull/2219/files - new: https://git.openjdk.java.net/jdk/pull/2219/files/0250fdcc..ffd53bff Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2219&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2219&range=00-01 Stats: 14 lines in 2 files changed: 5 ins; 7 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/2219.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2219/head:pull/2219 PR: https://git.openjdk.java.net/jdk/pull/2219
Re: RFR: 8260289: Unable to customize module lists after change JDK-8258411 [v2]
On Mon, 25 Jan 2021 13:08:58 GMT, Magnus Ihse Bursie wrote: >> Andrew Leonard has refreshed the contents of this pull request, and previous >> commits have been removed. The incremental views will show differences >> compared to the previous content of the PR. The pull request contains one >> new commit since the last revision: >> >> 8260289: Unable to customize module lists after change JDK-8258411 >> >> Signed-off-by: Andrew Leonard > > Changes requested by ihse (Reviewer). @magicus do you know what the magic pull request command is to re-generate the webrev having updated the commit? - PR: https://git.openjdk.java.net/jdk/pull/2219
Re: RFR: 8260289: Unable to customize module lists after change JDK-8258411 [v2]
On Mon, 25 Jan 2021 13:57:26 GMT, Magnus Ihse Bursie wrote: >> Note, it was the change from "appending" to BOOT_MODULES to just "assigning" >> that broke the existing behaviour with the JDK-8258411 change. > > @andrew-m-leonard (Seems I can't get github to tag you???) > > That sounds good. I think you could move the IncludeCustomExtension to after > all *.conf files, to future-proof it and to make it a bit more consistent -- > "first include conf files, then adjust them in custom extensions". @magicus yes, that's exactly the new commit i've just pushed. @(lookups) have been a bit wobbly recently with github.com I have noticed, I can find some people but not others! - PR: https://git.openjdk.java.net/jdk/pull/2219
Re: RFR: 8260289: Unable to customize module lists after change JDK-8258411 [v2]
On Mon, 25 Jan 2021 13:57:26 GMT, Magnus Ihse Bursie wrote: >> Note, it was the change from "appending" to BOOT_MODULES to just "assigning" >> that broke the existing behaviour with the JDK-8258411 change. > > @andrew-m-leonard (Seems I can't get github to tag you???) > > That sounds good. I think you could move the IncludeCustomExtension to after > all *.conf files, to future-proof it and to make it a bit more consistent -- > "first include conf files, then adjust them in custom extensions". @magicus @AlanBateman please see new commit (webrev https://webrevs.openjdk.java.net/?repo=jdk&pr=2219&range=01) - PR: https://git.openjdk.java.net/jdk/pull/2219
Re: RFR: 8260289: Unable to customize module lists after change JDK-8258411 [v2]
On Mon, 25 Jan 2021 13:08:58 GMT, Magnus Ihse Bursie wrote: >> Andrew Leonard has refreshed the contents of this pull request, and previous >> commits have been removed. The incremental views will show differences >> compared to the previous content of the PR. The pull request contains one >> new commit since the last revision: >> >> 8260289: Unable to customize module lists after change JDK-8258411 >> >> Signed-off-by: Andrew Leonard > > Changes requested by ihse (Reviewer). > @magicus do you know what the magic pull request command is to re-generate > the webrev having updated the commit? Ah looks like it automatically picked it up :-) - PR: https://git.openjdk.java.net/jdk/pull/2219
Integrated: 8260289: Unable to customize module lists after change JDK-8258411
On Mon, 25 Jan 2021 12:57:29 GMT, Andrew Leonard wrote: > A problem was found downstream with Eclipse OpenJ9 builds whereby since > JDK-8258411 they were unable to extend the module lists. > This PR adds a IncludeCustomExtension to the conf/module-loader-map.conf, to > enable the module list extension again. > > Signed-off-by: Andrew Leonard This pull request has now been integrated. Changeset: 12ccd211 Author:Andrew Leonard URL: https://git.openjdk.java.net/jdk/commit/12ccd211 Stats: 9 lines in 1 file changed: 5 ins; 3 del; 1 mod 8260289: Unable to customize module lists after change JDK-8258411 Reviewed-by: ihse, alanb - PR: https://git.openjdk.java.net/jdk/pull/2219
RFR: 8265666: Enable AIX build platform to make external debug symbols
Signed-off-by: Andrew Leonard - Commit messages: - 8265666: Enable AIX build platform to make external debug symbols Changes: https://git.openjdk.java.net/jdk/pull/3763/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3763&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8265666 Stats: 15 lines in 2 files changed: 4 ins; 9 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/3763.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3763/head:pull/3763 PR: https://git.openjdk.java.net/jdk/pull/3763
Re: RFR: 8265666: Enable AIX build platform to make external debug symbols
This change simply enables the specification of configure args to build external AIX debug symbols: --with-native-debug-symbols=external/zipped Now that we build using XLC/xlclang++ v16+ which supports standard external .debuginfo generation, it is simply a matter of enabling the option for AIX, and the rules for the debug info files and copy. This change has also been tested at AdoptOpenJDK build Jenkins. Thanks Andrew
Re: RFR: 8265666: Enable AIX build platform to make external debug symbols
On Wed, 28 Apr 2021 15:14:24 GMT, Erik Joelsson wrote: >> Signed-off-by: Andrew Leonard > > make/common/NativeCompilation.gmk line 979: > >> 977: else ifeq ($(call isTargetOs, aix), true) >> 978: $1_DEBUGINFO_FILES := >> $$($1_SYMBOLS_DIR)/$$($1_NOSUFFIX).debuginfo >> 979: $1_CREATE_DEBUGINFO_CMDS := $(CP) $$($1_TARGET) >> $$($1_DEBUGINFO_FILES) > > This looks like you are just copying the linked binary and calling it > .debuginfo. Is the idea that the original binary then gets stripped and this > is just saving the unstripped binary? I don't know how things are done in > AIX, but this seems like a rather strange way of achieving external symbol > information to me. You're essentially correct Erik, on AIX there is no such tool as OBJCOPY on Linux platforms, so you cannot physically remove the symbolic information from the native compiled object. So this CREATE_DEBUG_INFO_CMDS copies the object containing symbols, prior to the STRIP_CMD being executed as part of the make of $$($1_TARGET) recipe: $$($1_CREATE_DEBUGINFO_CMDS) $$($1_STRIP_CMD) The external debuginfo is then passed by the user of AIX dbx using -B <.debuginfo> You make a good point though that this looks confusing, so I will update the PR with a comment to this rule. thanks - PR: https://git.openjdk.java.net/jdk/pull/3763
Re: RFR: 8265666: Enable AIX build platform to make external debug symbols [v2]
> Signed-off-by: Andrew Leonard Andrew Leonard has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: 8265666: Enable AIX build platform to make external debug symbols Signed-off-by: Andrew Leonard - Changes: - all: https://git.openjdk.java.net/jdk/pull/3763/files - new: https://git.openjdk.java.net/jdk/pull/3763/files/731bd9c3..d457570c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3763&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3763&range=00-01 Stats: 3 lines in 1 file changed: 3 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/3763.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3763/head:pull/3763 PR: https://git.openjdk.java.net/jdk/pull/3763
Re: RFR: 8265666: Enable AIX build platform to make external debug symbols [v2]
On Wed, 28 Apr 2021 17:00:13 GMT, Andrew Leonard wrote: >> make/common/NativeCompilation.gmk line 982: >> >>> 980: >>> 981: else ifeq ($(call isTargetOs, macosx), true) >>> 982: $1_DEBUGINFO_FILES := \ >> >> This looks like you are just copying the linked binary and calling it >> .debuginfo. Is the idea that the original binary then gets stripped and this >> is just saving the unstripped binary? I don't know how things are done in >> AIX, but this seems like a rather strange way of achieving external symbol >> information to me. > > You're essentially correct Erik, on AIX there is no such tool as OBJCOPY on > Linux platforms, so you cannot physically remove the symbolic information > from the native compiled object. So this CREATE_DEBUG_INFO_CMDS copies the > object containing symbols, prior to the STRIP_CMD being executed as part of > the make of $$($1_TARGET) recipe: > > $$($1_CREATE_DEBUGINFO_CMDS) > $$($1_STRIP_CMD) > > The external debuginfo is then passed by the user of AIX dbx using -B > <.debuginfo> > > You make a good point though that this looks confusing, so I will update the > PR with a comment to this rule. > thanks I've added a comment in the make file in the refresh - PR: https://git.openjdk.java.net/jdk/pull/3763
Integrated: 8265666: Enable AIX build platform to make external debug symbols
On Wed, 28 Apr 2021 14:41:26 GMT, Andrew Leonard wrote: > Signed-off-by: Andrew Leonard This pull request has now been integrated. Changeset: 84b52db9 Author: Andrew Leonard URL: https://git.openjdk.java.net/jdk/commit/84b52db931943db5aa2df7edca7103776f2f2092 Stats: 18 lines in 2 files changed: 7 ins; 9 del; 2 mod 8265666: Enable AIX build platform to make external debug symbols Reviewed-by: erikj, mdoerr - PR: https://git.openjdk.java.net/jdk/pull/3763
Re: RFR: 8265666: Enable AIX build platform to make external debug symbols [v2]
On Thu, 29 Apr 2021 12:26:01 GMT, Martin Doerr wrote: >> Andrew Leonard has refreshed the contents of this pull request, and previous >> commits have been removed. The incremental views will show differences >> compared to the previous content of the PR. The pull request contains one >> new commit since the last revision: >> >> 8265666: Enable AIX build platform to make external debug symbols >> >> Signed-off-by: Andrew Leonard > > Looks good to me. Thanks! Thank you @TheRealMDoerr @erikj79 - PR: https://git.openjdk.java.net/jdk/pull/3763
RFR: 8276400: openjdk image Jars, Zips and Jmods built from the same source are not reproducible
This PR enables reproducible Jars, Jmods and openjdk image zip files (eg.src.zip). It provides support for SOURCE_DATE_EPOCH for Jar, Jmod and underlying ZipOutputStream's. It fixes the following keys issues relating to reproducibility: - Jar and ZipOutputStream are not SOURCE_DATE_EPOCH aware - Jar and ZipOutputStream now detect SOURCE_DATE_EPOCH environment setting - Jar and Jmod file content ordering was non-determinsitic - Fixes to Jar and Jmod main's to ensure sorted classes content ordering - openjdk image zip file generation used "zip" which is non-determinsitic - New openjdk build tool "GenerateZip" which produces the final determinsitic zip files as part of the build and also detects SOURCE_DATE_EPOCH Signed-off-by: Andrew Leonard - Commit messages: - 8276400: openjdk image Jars, Zips and Jmods built from the same source are not reproducible Changes: https://git.openjdk.java.net/jdk/pull/6268/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6268&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8276400 Stats: 818 lines in 11 files changed: 797 ins; 3 del; 18 mod Patch: https://git.openjdk.java.net/jdk/pull/6268.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6268/head:pull/6268 PR: https://git.openjdk.java.net/jdk/pull/6268
Re: RFR: 8276400: openjdk image Jars, Zips and Jmods built from the same source are not reproducible
On Thu, 4 Nov 2021 20:56:45 GMT, Andrew Leonard wrote: > This PR enables reproducible Jars, Jmods and openjdk image zip files > (eg.src.zip). > It provides support for SOURCE_DATE_EPOCH for Jar, Jmod and underlying > ZipOutputStream's. > It fixes the following keys issues relating to reproducibility: > - Jar and ZipOutputStream are not SOURCE_DATE_EPOCH aware > - Jar and ZipOutputStream now detect SOURCE_DATE_EPOCH environment setting > - Jar and Jmod file content ordering was non-determinsitic > - Fixes to Jar and Jmod main's to ensure sorted classes content ordering > - openjdk image zip file generation used "zip" which is non-determinsitic > - New openjdk build tool "GenerateZip" which produces the final > determinsitic zip files as part of the build and also detects > SOURCE_DATE_EPOCH > > Signed-off-by: Andrew Leonard @magicus fyi, please review if you have a moment, thanks - PR: https://git.openjdk.java.net/jdk/pull/6268
Re: RFR: 8276400: openjdk image Jars, Zips and Jmods built from the same source are not reproducible
On Thu, 4 Nov 2021 21:51:46 GMT, Joe Darcy wrote: >> This PR enables reproducible Jars, Jmods and openjdk image zip files >> (eg.src.zip). >> It provides support for SOURCE_DATE_EPOCH for Jar, Jmod and underlying >> ZipOutputStream's. >> It fixes the following keys issues relating to reproducibility: >> - Jar and ZipOutputStream are not SOURCE_DATE_EPOCH aware >> - Jar and ZipOutputStream now detect SOURCE_DATE_EPOCH environment setting >> - Jar and Jmod file content ordering was non-determinsitic >> - Fixes to Jar and Jmod main's to ensure sorted classes content ordering >> - openjdk image zip file generation used "zip" which is non-determinsitic >> - New openjdk build tool "GenerateZip" which produces the final >> determinsitic zip files as part of the build and also detects >> SOURCE_DATE_EPOCH >> >> Signed-off-by: Andrew Leonard > > Please file a CSR for the behavioral change of recognizing SOURCE_DATE_EPOCH, > etc. Thanks. @jddarcy thanks, I did wonder that, i'll start a CSR, thanks - PR: https://git.openjdk.java.net/jdk/pull/6268
Re: RFR: 8276400: openjdk image Jars, Zips and Jmods built from the same source are not reproducible
On Thu, 4 Nov 2021 21:51:46 GMT, Joe Darcy wrote: >> This PR enables reproducible Jars, Jmods and openjdk image zip files >> (eg.src.zip). >> It provides support for SOURCE_DATE_EPOCH for Jar, Jmod and underlying >> ZipOutputStream's. >> It fixes the following keys issues relating to reproducibility: >> - Jar and ZipOutputStream are not SOURCE_DATE_EPOCH aware >> - Jar and ZipOutputStream now detect SOURCE_DATE_EPOCH environment setting >> - Jar and Jmod file content ordering was non-determinsitic >> - Fixes to Jar and Jmod main's to ensure sorted classes content ordering >> - openjdk image zip file generation used "zip" which is non-determinsitic >> - New openjdk build tool "GenerateZip" which produces the final >> determinsitic zip files as part of the build and also detects >> SOURCE_DATE_EPOCH >> >> Signed-off-by: Andrew Leonard > > Please file a CSR for the behavioral change of recognizing SOURCE_DATE_EPOCH, > etc. Thanks. @jddarcy CSR created: https://bugs.openjdk.java.net/browse/JDK-8276667 - PR: https://git.openjdk.java.net/jdk/pull/6268
Re: RFR: 8276400: openjdk image Jars, Zips and Jmods built from the same source are not reproducible
On Thu, 4 Nov 2021 20:56:45 GMT, Andrew Leonard wrote: > This PR enables reproducible Jars, Jmods and openjdk image zip files > (eg.src.zip). > It provides support for SOURCE_DATE_EPOCH for Jar, Jmod and underlying > ZipOutputStream's. > It fixes the following keys issues relating to reproducibility: > - Jar and ZipOutputStream are not SOURCE_DATE_EPOCH aware > - Jar and ZipOutputStream now detect SOURCE_DATE_EPOCH environment setting > - Jar and Jmod file content ordering was non-determinsitic > - Fixes to Jar and Jmod main's to ensure sorted classes content ordering > - openjdk image zip file generation used "zip" which is non-determinsitic > - New openjdk build tool "GenerateZip" which produces the final > determinsitic zip files as part of the build and also detects > SOURCE_DATE_EPOCH > > Signed-off-by: Andrew Leonard > I think this is going to require discussion, a PR may be too premature. Is > your goal to get the JDK build and run-time images creates with jlink to use > the SOURCE_DATE_EPOCH? Are you looking for project builds that use the > jar/jmod/etc. tools to use it? Or are you looking to have every library and > application that uses the java.util.zip APIs to read it? If it includes the > latter, and the patch in the PR suggests you are, then it will require > analysis to work through the API spec changes. > > One suggestion is to look at JDK-8231640 and PR 5372 [1]. The original > proposal was to use SOURCE_DATE_EPOCH. After a lengthy discussion we > converged on the system property java.properties.date rather than the env > variable. I suspect that much of that discussion applies here. > > -Alan > > [1] https://github.com/[/pull/5372](https://github.com/openjdk/jdk/pull/5372) @AlanBateman Hi Alan, thank you for the comments. So yes, totally agree this has a way to go, I thought creating a PR would kick things off...! So just to clarify the initial objectives which you raise above. My current aim is actually purely an OpenJDK JDK image perspective, building on the work @magicus has been doing with SOURCE_DATE_EPOCH in the build, hence my natural approach to this PR. So from that perspective, it's purely a "build" change. However, the openjdk build obviously uses openjdk Jar, Jmod tooling to build itself, hence the direction I took in this PR. Magnus's PR #5372 is useful thank you, I had not discovered that change, as you say the direction there is similar to mine, and I understand the potential pitfalls of adding doPrivileged's etc. So that resolved to a new system property java.properties.date, which makes sense. So with that in mind, and given the objective of JDK image reproducibility, this would imply presumably an approach of a new cmd line option to Jar and Jmod tools (eg.--source-date ), since these tools are cmds? One of the key changes in this PR is to ZipOutputStream where it sets the ZipEntry.xdostime if not set, to the current time. Now I am thinking given the "objective", if we fix all upstream tooling to ensure they set ZipEntry times, using "--source-date" (or whatever mechanism), then we can then avoid changing ZipOutputStream I believe. JDK tools like jar, jmod generate/update extra files like Manifests, and in fact I think the current jmod implementation writes all its content using the current time. So something along these lines: jar -cf myjar.jar --source-date "" * jmod create --source-date "" ... my.jmod The jar, jmod,.. tooling then ensure all ZipEntry times are set to "source-date". I chose "source-date" name based on being synonymous with SOURCE_DATE_EPOCH, implying a similar "use case". So taking this approach for JDK tooling should achieve the reproducible JDK image objective, without a java API behavior change I am hoping. Thoughts? Regards Andrew - PR: https://git.openjdk.java.net/jdk/pull/6268
Re: RFR: 8276400: openjdk image Jars, Zips and Jmods built from the same source are not reproducible
On Fri, 5 Nov 2021 11:16:45 GMT, Lance Andersen wrote: >> This PR enables reproducible Jars, Jmods and openjdk image zip files >> (eg.src.zip). >> It provides support for SOURCE_DATE_EPOCH for Jar, Jmod and underlying >> ZipOutputStream's. >> It fixes the following keys issues relating to reproducibility: >> - Jar and ZipOutputStream are not SOURCE_DATE_EPOCH aware >> - Jar and ZipOutputStream now detect SOURCE_DATE_EPOCH environment setting >> - Jar and Jmod file content ordering was non-determinsitic >> - Fixes to Jar and Jmod main's to ensure sorted classes content ordering >> - openjdk image zip file generation used "zip" which is non-determinsitic >> - New openjdk build tool "GenerateZip" which produces the final >> determinsitic zip files as part of the build and also detects >> SOURCE_DATE_EPOCH >> >> Signed-off-by: Andrew Leonard > > src/jdk.jartool/share/classes/sun/tools/jar/Main.java line 795: > >> 793: // Ensure files list is sorted for reproducible jar content >> 794: Arrays.sort(files); >> 795: > > Have you had a chance to measure the performance with a large number of Zip > entries with this change? No I haven't, but my thoughts on this were assuming you had a zip with many 1000s of ZipEntries the file I/O would be far more significant. Also, you will note this is not sorting the whole set, just within each directory, so the sort won't be complex, unless you had 1000s of files in a single directory. The "non-determinism" comes from the File.list() implementation which uses OS file listing, whose order is non-deterministic. - PR: https://git.openjdk.java.net/jdk/pull/6268
Re: RFR: 8276400: openjdk image Jars, Zips and Jmods built from the same source are not reproducible
On Fri, 5 Nov 2021 11:31:34 GMT, Andrew Leonard wrote: >> This PR enables reproducible Jars, Jmods and openjdk image zip files >> (eg.src.zip). >> It provides support for SOURCE_DATE_EPOCH for Jar, Jmod and underlying >> ZipOutputStream's. >> It fixes the following keys issues relating to reproducibility: >> - Jar and ZipOutputStream are not SOURCE_DATE_EPOCH aware >> - Jar and ZipOutputStream now detect SOURCE_DATE_EPOCH environment setting >> - Jar and Jmod file content ordering was non-determinsitic >> - Fixes to Jar and Jmod main's to ensure sorted classes content ordering >> - openjdk image zip file generation used "zip" which is non-determinsitic >> - New openjdk build tool "GenerateZip" which produces the final >> determinsitic zip files as part of the build and also detects >> SOURCE_DATE_EPOCH >> >> Signed-off-by: Andrew Leonard > >> I think this is going to require discussion, a PR may be too premature. Is >> your goal to get the JDK build and run-time images creates with jlink to use >> the SOURCE_DATE_EPOCH? Are you looking for project builds that use the >> jar/jmod/etc. tools to use it? Or are you looking to have every library and >> application that uses the java.util.zip APIs to read it? If it includes the >> latter, and the patch in the PR suggests you are, then it will require >> analysis to work through the API spec changes. >> >> One suggestion is to look at JDK-8231640 and PR 5372 [1]. The original >> proposal was to use SOURCE_DATE_EPOCH. After a lengthy discussion we >> converged on the system property java.properties.date rather than the env >> variable. I suspect that much of that discussion applies here. >> >> -Alan >> >> [1] https://github.com/[/pull/5372](https://github.com/openjdk/jdk/pull/5372) > > @AlanBateman Hi Alan, thank you for the comments. So yes, totally agree this > has a way to go, I thought creating a PR would kick things off...! So just to > clarify the initial objectives which you raise above. My current aim is > actually purely an OpenJDK JDK image perspective, building on the work > @magicus has been doing with SOURCE_DATE_EPOCH in the build, hence my natural > approach to this PR. So from that perspective, it's purely a "build" change. > However, the openjdk build obviously uses openjdk Jar, Jmod tooling to build > itself, hence the direction I took in this PR. > > Magnus's PR #5372 is useful thank you, I had not discovered that change, as > you say the direction there is similar to mine, and I understand the > potential pitfalls of adding doPrivileged's etc. So that resolved to a new > system property java.properties.date, which makes sense. > > So with that in mind, and given the objective of JDK image reproducibility, > this would imply presumably an approach of a new cmd line option to Jar and > Jmod tools (eg.--source-date ), since these tools are cmds? > > One of the key changes in this PR is to ZipOutputStream where it sets the > ZipEntry.xdostime if not set, to the current time. Now I am thinking given > the "objective", if we fix all upstream tooling to ensure they set ZipEntry > times, using "--source-date" (or whatever mechanism), then we can then avoid > changing ZipOutputStream I believe. JDK tools like jar, jmod generate/update > extra files like Manifests, and in fact I think the current jmod > implementation writes all its content using the current time. > > So something along these lines: > jar -cf myjar.jar --source-date "" * > jmod create --source-date "" ... my.jmod > The jar, jmod,.. tooling then ensure all ZipEntry times are set to > "source-date". > > I chose "source-date" name based on being synonymous with SOURCE_DATE_EPOCH, > implying a similar "use case". > > So taking this approach for JDK tooling should achieve the reproducible JDK > image objective, without a java API behavior change I am hoping. > Thoughts? > > Regards > Andrew > @andrew-m-leonard Just to be clear: changing arguments to the command line > utilities is still an API change and will need a CSR. > > My gut feeling is that using a system property that affect all users of > ZipStream is preferable. That way a user could run like `java > -Djava.properties.date=$(SOURCE_DATE_EPOCH) > -Djava.zipfile.date=$(SOURCE_DATE_EPOCH) ...` (or whatever) and get > reproducible behavior in all their code, for places that could otherwise be > hard to fix. Thanks @magicus Yes, I understand a CSR is required, I was trying to state there would n
Re: RFR: 8276400: openjdk image Jars, Zips and Jmods built from the same source are not reproducible
On Thu, 4 Nov 2021 20:56:45 GMT, Andrew Leonard wrote: > This PR enables reproducible Jars, Jmods and openjdk image zip files > (eg.src.zip). > It provides support for SOURCE_DATE_EPOCH for Jar, Jmod and underlying > ZipOutputStream's. > It fixes the following keys issues relating to reproducibility: > - Jar and ZipOutputStream are not SOURCE_DATE_EPOCH aware > - Jar and ZipOutputStream now detect SOURCE_DATE_EPOCH environment setting > - Jar and Jmod file content ordering was non-determinsitic > - Fixes to Jar and Jmod main's to ensure sorted classes content ordering > - openjdk image zip file generation used "zip" which is non-determinsitic > - New openjdk build tool "GenerateZip" which produces the final > determinsitic zip files as part of the build and also detects > SOURCE_DATE_EPOCH > > Signed-off-by: Andrew Leonard > _Mailing list message from [Alan Bateman](mailto:alan.bate...@oracle.com) on > [build-dev](mailto:build-...@mail.openjdk.java.net):_ > > On 05/11/2021 11:39, Magnus Ihse Bursie wrote: > > > : > > I agree with Alan's comment above. First of all, to be absolutely clear, I > > think this is a very worthy goal, and much overdue. I'll do my best to help > > get this implemented. > > One suggestion is to separate out the issue of ordering of entries (in > zip/JAR and JMOD) and whether it would be the default. There may be > trade-offs to discuss, also whether it's limited to just new zip/JAR files or > updates to existing zip/JARs files. > > -Alan Yes, makes sense, @LanceAndersen already picked up on that for file sorting. We can discuss in a separate PR. So picking up on what @magicus suggested as well, what I shall do is split this into 3: 1) GenerateZip openjdk build tool 2) Changes to Jar and Jmod main's to ensure file ordering 3) Jar, Jmod and/or ZipOutputStream changes to support a "source-date" specification - PR: https://git.openjdk.java.net/jdk/pull/6268
Re: RFR: 8276400: openjdk image Jars, Zips and Jmods built from the same source are not reproducible
On Fri, 5 Nov 2021 11:19:16 GMT, Lance Andersen wrote: >> This PR enables reproducible Jars, Jmods and openjdk image zip files >> (eg.src.zip). >> It provides support for SOURCE_DATE_EPOCH for Jar, Jmod and underlying >> ZipOutputStream's. >> It fixes the following keys issues relating to reproducibility: >> - Jar and ZipOutputStream are not SOURCE_DATE_EPOCH aware >> - Jar and ZipOutputStream now detect SOURCE_DATE_EPOCH environment setting >> - Jar and Jmod file content ordering was non-determinsitic >> - Fixes to Jar and Jmod main's to ensure sorted classes content ordering >> - openjdk image zip file generation used "zip" which is non-determinsitic >> - New openjdk build tool "GenerateZip" which produces the final >> determinsitic zip files as part of the build and also detects >> SOURCE_DATE_EPOCH >> >> Signed-off-by: Andrew Leonard > > test/jdk/java/util/zip/TestZipSourceDateEpoch.sh line 1: > >> 1: #!/bin/sh > > Unless there is a specific reason to use a shell script, it would be better > to convert this to java. We have been trying to reduce the usage of shell > scripts I had copied an existing example, I obviously chose a bad example! I can re-write with ProcessBuilder... cheers - PR: https://git.openjdk.java.net/jdk/pull/6268
Re: RFR: 8276400: openjdk image Jars, Zips and Jmods built from the same source are not reproducible
On Thu, 4 Nov 2021 20:56:45 GMT, Andrew Leonard wrote: > This PR enables reproducible Jars, Jmods and openjdk image zip files > (eg.src.zip). > It provides support for SOURCE_DATE_EPOCH for Jar, Jmod and underlying > ZipOutputStream's. > It fixes the following keys issues relating to reproducibility: > - Jar and ZipOutputStream are not SOURCE_DATE_EPOCH aware > - Jar and ZipOutputStream now detect SOURCE_DATE_EPOCH environment setting > - Jar and Jmod file content ordering was non-determinsitic > - Fixes to Jar and Jmod main's to ensure sorted classes content ordering > - openjdk image zip file generation used "zip" which is non-determinsitic > - New openjdk build tool "GenerateZip" which produces the final > determinsitic zip files as part of the build and also detects > SOURCE_DATE_EPOCH > > Signed-off-by: Andrew Leonard Thanks Magnus, You've just reminded me of why I gave up trying to write my own zip, I achieved it, but realized it was slow damn slow, and how efficient and optimized the off the shelf zip is...! Hence, why I let zip still do it's inclusion/exclusion job it does so well, and then just re-generate at the end. Takes about 2-3 seconds longer, so not an issue really. cheers - PR: https://git.openjdk.java.net/jdk/pull/6268
RFR: 8276654: element-list order is non deterministic
Fixes: https://bugs.openjdk.java.net/browse/JDK-8276654 A intermittent problem with the make dependencies means the jdk.javadoc element-list-.txt generation can remove the src defined element|package-list-<7,8,9,10>.txt files. Recreatable by using --with-jobs=1 causing jdk.javadoc "gendata" to always occur after "java" module build dependency. This fix puts a dependency of jdk.javadoc-java on jdk.javadoc-gendata to avoid this. Signed-off-by: Andrew Leonard - Commit messages: - 8276654: element-list order is non deterministic Changes: https://git.openjdk.java.net/jdk/pull/6278/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6278&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8276654 Stats: 5 lines in 1 file changed: 5 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/6278.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6278/head:pull/6278 PR: https://git.openjdk.java.net/jdk/pull/6278
Re: RFR: 8276654: element-list order is non deterministic
On Fri, 5 Nov 2021 16:15:02 GMT, Erik Joelsson wrote:
>> Fixes: https://bugs.openjdk.java.net/browse/JDK-8276654
>>
>> A intermittent problem with the make dependencies means the jdk.javadoc
>> element-list-.txt generation can remove the src defined
>> element|package-list-<7,8,9,10>.txt files.
>> Recreatable by using --with-jobs=1 causing jdk.javadoc "gendata" to always
>> occur after "java" module build dependency.
>> This fix puts a dependency of jdk.javadoc-java on jdk.javadoc-gendata to
>> avoid this.
>>
>> Signed-off-by: Andrew Leonard
>
> Good catch, but wouldn't it be better if we could fix the recipe in
> jdk.javadoc/Gendata.gmk to only delete the files it intends to create. If I
> understand what files are created where correctly, you could generate a glob
> for the rm statement using something like this:
>
> $(RM) $(ELEMENT_LISTS_DIR)/element-list-{$(call CommaList,$(call
> sequence,11,$(VERSION_FEATURE)))}.txt
> @erikj79 Ah, we have a `sequence` function! I forgot about it. My initial
> reaction was also to prefer fixing the recipe to not delete non-related
> files, but I could not find a nice and not too overly complicated way of
> doing it.
>
> I agree that this is a better solution.
@erikj79 Yes, I thought that to start with, but exactly as @magicus said, I
couldn't figure and simple/non-complex way in gnu make language to fix it there
either!
> Good catch, but wouldn't it be better if we could fix the recipe in
> jdk.javadoc/Gendata.gmk to only delete the files it intends to create. If I
> understand what files are created where correctly, you could generate a glob
> for the rm statement using something like this:
>
> $(RM) $(ELEMENT_LISTS_DIR)/element-list-{$(call CommaList,$(call
> sequence,11,$(VERSION_FEATURE)))}.txt
I'll give that a try, cheers
-
PR: https://git.openjdk.java.net/jdk/pull/6278
Re: RFR: 8276654: element-list order is non deterministic
On Fri, 5 Nov 2021 16:15:02 GMT, Erik Joelsson wrote:
>> Fixes: https://bugs.openjdk.java.net/browse/JDK-8276654
>>
>> A intermittent problem with the make dependencies means the jdk.javadoc
>> element-list-.txt generation can remove the src defined
>> element|package-list-<7,8,9,10>.txt files.
>> Recreatable by using --with-jobs=1 causing jdk.javadoc "gendata" to always
>> occur after "java" module build dependency.
>> This fix puts a dependency of jdk.javadoc-java on jdk.javadoc-gendata to
>> avoid this.
>>
>> Signed-off-by: Andrew Leonard
>
> Good catch, but wouldn't it be better if we could fix the recipe in
> jdk.javadoc/Gendata.gmk to only delete the files it intends to create. If I
> understand what files are created where correctly, you could generate a glob
> for the rm statement using something like this:
>
> $(RM) $(ELEMENT_LISTS_DIR)/element-list-{$(call CommaList,$(call
> sequence,11,$(VERSION_FEATURE)))}.txt
@erikj79 yep looks good:
-
PR: https://git.openjdk.java.net/jdk/pull/6278
Re: RFR: 8276654: element-list order is non deterministic [v2]
> Fixes: https://bugs.openjdk.java.net/browse/JDK-8276654 > > A intermittent problem with the make dependencies means the jdk.javadoc > element-list-.txt generation can remove the src defined > element|package-list-<7,8,9,10>.txt files. > Recreatable by using --with-jobs=1 causing jdk.javadoc "gendata" to always > occur after "java" module build dependency. > This fix puts a dependency of jdk.javadoc-java on jdk.javadoc-gendata to > avoid this. > > Signed-off-by: Andrew Leonard Andrew Leonard has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: 8276654: element-list order is non deterministic Signed-off-by: Andrew Leonard - Changes: - all: https://git.openjdk.java.net/jdk/pull/6278/files - new: https://git.openjdk.java.net/jdk/pull/6278/files/28fbedde..014534cf Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6278&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6278&range=00-01 Stats: 6 lines in 2 files changed: 0 ins; 5 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/6278.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6278/head:pull/6278 PR: https://git.openjdk.java.net/jdk/pull/6278
Re: RFR: 8276400: openjdk image Jars, Zips and Jmods built from the same source are not reproducible
On Thu, 4 Nov 2021 20:56:45 GMT, Andrew Leonard wrote: > This PR enables reproducible Jars, Jmods and openjdk image zip files > (eg.src.zip). > It provides support for SOURCE_DATE_EPOCH for Jar, Jmod and underlying > ZipOutputStream's. > It fixes the following keys issues relating to reproducibility: > - Jar and ZipOutputStream are not SOURCE_DATE_EPOCH aware > - Jar and ZipOutputStream now detect SOURCE_DATE_EPOCH environment setting > - Jar and Jmod file content ordering was non-determinsitic > - Fixes to Jar and Jmod main's to ensure sorted classes content ordering > - openjdk image zip file generation used "zip" which is non-determinsitic > - New openjdk build tool "GenerateZip" which produces the final > determinsitic zip files as part of the build and also detects > SOURCE_DATE_EPOCH > > Signed-off-by: Andrew Leonard @AlanBateman @magicus thank you both for your guidance. I have now split this bug into the 3 mentioned: - GenerateZip: https://bugs.openjdk.java.net/browse/JDK-8276743 - Jar/Jmod content ordering: https://bugs.openjdk.java.net/browse/JDK-8276764 - Jar/Jmod/ZipOutputStream timestamp option: https://bugs.openjdk.java.net/browse/JDK-8276766 **(requires CSR)** Closing this PR to be replaced with 3 new PRs for the above bugs. - PR: https://git.openjdk.java.net/jdk/pull/6268
Withdrawn: 8276400: openjdk image Jars, Zips and Jmods built from the same source are not reproducible
On Thu, 4 Nov 2021 20:56:45 GMT, Andrew Leonard wrote: > This PR enables reproducible Jars, Jmods and openjdk image zip files > (eg.src.zip). > It provides support for SOURCE_DATE_EPOCH for Jar, Jmod and underlying > ZipOutputStream's. > It fixes the following keys issues relating to reproducibility: > - Jar and ZipOutputStream are not SOURCE_DATE_EPOCH aware > - Jar and ZipOutputStream now detect SOURCE_DATE_EPOCH environment setting > - Jar and Jmod file content ordering was non-determinsitic > - Fixes to Jar and Jmod main's to ensure sorted classes content ordering > - openjdk image zip file generation used "zip" which is non-determinsitic > - New openjdk build tool "GenerateZip" which produces the final > determinsitic zip files as part of the build and also detects > SOURCE_DATE_EPOCH > > Signed-off-by: Andrew Leonard This pull request has been closed without being integrated. - PR: https://git.openjdk.java.net/jdk/pull/6268
Re: RFR: 8276746: Add section on reproducible builds in building.md
On Fri, 5 Nov 2021 16:30:26 GMT, Magnus Ihse Bursie wrote: > Reproducible builds are all the vogue. The JDK has been making great strides > in getting there, but still has some way to go. However, to get as close as > possible, some special configuration is needed. > > This has been "tribal knowledge" of a few persons in the build team. It needs > to be properly documented to help other users wanting to create reproducible > builds of the JDK. Looks good Magnus - Marked as reviewed by aleonard (Committer). PR: https://git.openjdk.java.net/jdk/pull/6279
Re: RFR: 8276654: element-list order is non deterministic [v2]
On Fri, 5 Nov 2021 19:20:10 GMT, Erik Joelsson wrote:
>> Andrew Leonard has refreshed the contents of this pull request, and previous
>> commits have been removed. The incremental views will show differences
>> compared to the previous content of the PR.
>
> make/modules/jdk.javadoc/Gendata.gmk line 75:
>
>> 73: $(call MakeTargetDir)
>> 74: $(call LogInfo, Creating javadoc element lists)
>> 75: $(RM) $(ELEMENT_LISTS_DIR)/element-list-{$(call CommaList,$(call
>> sequence,$(GENERATE_SYMBOLS_FROM_JDK_VERSION),$(JDK_SOURCE_TARGET_VERSION)))}.txt
>
> Good to see that it worked! I would only wish that you found a way to break
> up the line. Long lines make future side-by-side reviews and 3-way merges
> hard. We don't enforce strict 80, but try to stay in some reasonable ballpark
> in the build files.
>
> I think both CommaList and sequence are ok with whitespace in their
> parameters. Otherwise you could pre-calculate the numbers list in a variable
> before the recipe.
yep, i'll have a look to see what I can do, not helped with existing variables
being so long!! (GENERATE_SYMBOLS_FROM_JDK_VERSION)
-
PR: https://git.openjdk.java.net/jdk/pull/6278
Re: RFR: 8276654: element-list order is non deterministic [v3]
> Fixes: https://bugs.openjdk.java.net/browse/JDK-8276654 > > A intermittent problem with the make dependencies means the jdk.javadoc > element-list-.txt generation can remove the src defined > element|package-list-<7,8,9,10>.txt files. > Recreatable by using --with-jobs=1 causing jdk.javadoc "gendata" to always > occur after "java" module build dependency. > This fix puts a dependency of jdk.javadoc-java on jdk.javadoc-gendata to > avoid this. > > Signed-off-by: Andrew Leonard Andrew Leonard has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: 8276654: element-list order is non deterministic Signed-off-by: Andrew Leonard - Changes: - all: https://git.openjdk.java.net/jdk/pull/6278/files - new: https://git.openjdk.java.net/jdk/pull/6278/files/014534cf..9d7b338c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6278&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6278&range=01-02 Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/6278.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6278/head:pull/6278 PR: https://git.openjdk.java.net/jdk/pull/6278
Re: RFR: 8276654: element-list order is non deterministic [v2]
On Fri, 5 Nov 2021 19:26:56 GMT, Andrew Leonard wrote:
>> make/modules/jdk.javadoc/Gendata.gmk line 75:
>>
>>> 73: $(call MakeTargetDir)
>>> 74: $(call LogInfo, Creating javadoc element lists)
>>> 75: $(RM) $(ELEMENT_LISTS_DIR)/element-list-{$(call
>>> CommaList,$(call
>>> sequence,$(GENERATE_SYMBOLS_FROM_JDK_VERSION),$(JDK_SOURCE_TARGET_VERSION)))}.txt
>>
>> Good to see that it worked! I would only wish that you found a way to break
>> up the line. Long lines make future side-by-side reviews and 3-way merges
>> hard. We don't enforce strict 80, but try to stay in some reasonable
>> ballpark in the build files.
>>
>> I think both CommaList and sequence are ok with whitespace in their
>> parameters. Otherwise you could pre-calculate the numbers list in a variable
>> before the recipe.
>
> yep, i'll have a look to see what I can do, not helped with existing
> variables being so long!! (GENERATE_SYMBOLS_FROM_JDK_VERSION)
yep, split on CommaList seems better and works fine, see revision
-
PR: https://git.openjdk.java.net/jdk/pull/6278
Re: RFR: 8276654: element-list order is non deterministic [v3]
On Sat, 6 Nov 2021 20:54:27 GMT, Jonathan Gibbons wrote: > Can someone please verify that the contents of these files after the fix is > the same as the contents of the files for "good" builds before the fix? @jonathan-gibbons done, all good - PR: https://git.openjdk.java.net/jdk/pull/6278
Re: RFR: 8276654: element-list order is non deterministic [v3]
On Sun, 7 Nov 2021 11:55:19 GMT, Andrew Leonard wrote: >> Can someone please verify that the contents of these files after the fix is >> the same as the contents of the files for "good" builds before the fix? > >> Can someone please verify that the contents of these files after the fix is >> the same as the contents of the files for "good" builds before the fix? > > @jonathan-gibbons done, all good > @andrew-m-leonard Please refrain from force-pushing to a PR. It makes the > evolution of the patch hard to follow for reviewers. > > I thought I had commented on the commas, but I can't find it now so maybe I > failed to press the Comment button. So let me do it again. > > Can you verify if it is possible to add whitespace after the commas in the > "sequence" call? Normally, we use whitespace after all commas (except where > impossible, like for built-in make functions). If that does not work, we need > to fix the "sequence" method so it accepts whitespace after commas. > > Otherwise this looks good! @magicus Yes splits fine, updated... thanks - PR: https://git.openjdk.java.net/jdk/pull/6278
Re: RFR: 8276654: element-list order is non deterministic [v4]
> Fixes: https://bugs.openjdk.java.net/browse/JDK-8276654 > > A intermittent problem with the make dependencies means the jdk.javadoc > element-list-.txt generation can remove the src defined > element|package-list-<7,8,9,10>.txt files. > Recreatable by using --with-jobs=1 causing jdk.javadoc "gendata" to always > occur after "java" module build dependency. > > Signed-off-by: Andrew Leonard Andrew Leonard has updated the pull request incrementally with one additional commit since the last revision: 8276654: element-list order is non deterministic Signed-off-by: Andrew Leonard - Changes: - all: https://git.openjdk.java.net/jdk/pull/6278/files - new: https://git.openjdk.java.net/jdk/pull/6278/files/9d7b338c..a7474d01 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6278&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6278&range=02-03 Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/6278.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6278/head:pull/6278 PR: https://git.openjdk.java.net/jdk/pull/6278
Re: RFR: 8276654: element-list order is non deterministic [v5]
> Fixes: https://bugs.openjdk.java.net/browse/JDK-8276654 > > A intermittent problem with the make dependencies means the jdk.javadoc > element-list-.txt generation can remove the src defined > element|package-list-<7,8,9,10>.txt files. > Recreatable by using --with-jobs=1 causing jdk.javadoc "gendata" to always > occur after "java" module build dependency. > > Signed-off-by: Andrew Leonard Andrew Leonard has updated the pull request incrementally with one additional commit since the last revision: 8276654: element-list order is non deterministic Signed-off-by: Andrew Leonard - Changes: - all: https://git.openjdk.java.net/jdk/pull/6278/files - new: https://git.openjdk.java.net/jdk/pull/6278/files/a7474d01..85056a09 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6278&range=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6278&range=03-04 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/6278.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6278/head:pull/6278 PR: https://git.openjdk.java.net/jdk/pull/6278
Re: RFR: 8276654: element-list order is non deterministic [v4]
On Mon, 8 Nov 2021 13:29:57 GMT, Magnus Ihse Bursie wrote:
>> Andrew Leonard has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8276654: element-list order is non deterministic
>>
>> Signed-off-by: Andrew Leonard
>
> make/modules/jdk.javadoc/Gendata.gmk line 76:
>
>> 74: $(call LogInfo, Creating javadoc element lists)
>> 75: $(RM) $(ELEMENT_LISTS_DIR)/element-list-{$(call CommaList, \
>> 76: $(call sequence,$(GENERATE_SYMBOLS_FROM_JDK_VERSION), \
>
> Suggestion:
>
> $(call sequence, $(GENERATE_SYMBOLS_FROM_JDK_VERSION), \
done
-
PR: https://git.openjdk.java.net/jdk/pull/6278
Integrated: 8276654: element-list order is non deterministic
On Fri, 5 Nov 2021 15:26:40 GMT, Andrew Leonard wrote: > Fixes: https://bugs.openjdk.java.net/browse/JDK-8276654 > > A intermittent problem with the make dependencies means the jdk.javadoc > element-list-.txt generation can remove the src defined > element|package-list-<7,8,9,10>.txt files. > Recreatable by using --with-jobs=1 causing jdk.javadoc "gendata" to always > occur after "java" module build dependency. > > Signed-off-by: Andrew Leonard This pull request has now been integrated. Changeset: 14d66bd4 Author:Andrew Leonard URL: https://git.openjdk.java.net/jdk/commit/14d66bd438dfa1feeafaca39be8f79a91e2968e9 Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod 8276654: element-list order is non deterministic Reviewed-by: ihse - PR: https://git.openjdk.java.net/jdk/pull/6278
RFR: 8276743: Make openjdk build Zip Archive generation "reproducible"
This PR adds a new openjdk build tool GenerateZip, which generates a final "zip" file from an input folder, and creates it in a deterministic way, ensuring ordering and timestamps are set as specified. Using this tool in ZipArchive.gmk will ensure src.zip is then created deterministically. Signed-off-by: Andrew Leonard - Commit messages: - 8276743: Make openjdk build Zip Archive generation "reproducible" Changes: https://git.openjdk.java.net/jdk/pull/6311/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6311&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8276743 Stats: 303 lines in 3 files changed: 300 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/6311.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6311/head:pull/6311 PR: https://git.openjdk.java.net/jdk/pull/6311
Re: RFR: 8276743: Make openjdk build Zip Archive generation "reproducible"
On Tue, 9 Nov 2021 13:58:24 GMT, Erik Joelsson wrote: >> This PR adds a new openjdk build tool GenerateZip, which generates a final >> "zip" file from an input folder, and creates it in a deterministic way, >> ensuring ordering and timestamps are set as specified. >> >> Using this tool in ZipArchive.gmk will ensure src.zip is then created >> deterministically. >> >> Signed-off-by: Andrew Leonard > > make/common/ZipArchive.gmk line 178: > >> 176: (cd $$(SUPPORT_OUTPUTDIR)/ziptmp/$1/files && \ >> 177: $(RM) $$@ && \ >> 178: $(UNZIP) -q $$(SUPPORT_OUTPUTDIR)/ziptmp/$1/tmp.zip && \ > > Having to explode the zip here is unfortunate. This means we are creating an > almost full copy of the whole src tree in the build directory, something I > tried to avoid by leveraging the include/exclude functionality of zip, > instead of generating make rules for copying the files I wanted into a source > tree to run zip on. This may be a small overhead on Linux, but I'm pretty > sure it will be very noticeable on Windows. > > When reading about your tool at first, I assumed it would read the > intermediate zip file directly when rebuilding the zip. I don't think > modifying it to do that would be too complicated, basically read and > processing ZipEntrys instead of walking the file system. @erikj79 sure, I can look at doing that > make/jdk/src/classes/build/tools/generatezip/GenerateZip.java line 161: > >> 159: boolean pathIsDir = fpath.isDirectory(); >> 160: >> 161: // Keep a sorted Set of files to be processed, so that the Jmod >> is reproducible > > Aren't we generating zip files rather than jmod files here? copy&paste error... - PR: https://git.openjdk.java.net/jdk/pull/6311
Re: RFR: 8276743: Make openjdk build Zip Archive generation "reproducible"
On Tue, 9 Nov 2021 14:04:27 GMT, Erik Joelsson wrote: > I think it would it make sense add a configure parameter to to enable/disable > this new functionality. I think developers would want to opt out of extra > unnecessary build steps when building and testing locally. Not sure what the > default should be. For other reproducible measures, we require active > enabling today. Maybe default on for release builds and off for debug builds. @erikj79 so I was hoping to not add another configure arg, when the intentions of this should just all be positive, ordered&deterministic. Also @magicus I get the impression from your comments that we don't really want to end up with in order to build reproducibly you have to specify all these options x,y,z,... Maybe we should just have one big flag --with-reproducible-build ? - PR: https://git.openjdk.java.net/jdk/pull/6311
Re: RFR: 8276743: Make openjdk build Zip Archive generation "reproducible"
On Tue, 9 Nov 2021 13:58:24 GMT, Erik Joelsson wrote: >> This PR adds a new openjdk build tool GenerateZip, which generates a final >> "zip" file from an input folder, and creates it in a deterministic way, >> ensuring ordering and timestamps are set as specified. >> >> Using this tool in ZipArchive.gmk will ensure src.zip is then created >> deterministically. >> >> Signed-off-by: Andrew Leonard > > make/common/ZipArchive.gmk line 178: > >> 176: (cd $$(SUPPORT_OUTPUTDIR)/ziptmp/$1/files && \ >> 177: $(RM) $$@ && \ >> 178: $(UNZIP) -q $$(SUPPORT_OUTPUTDIR)/ziptmp/$1/tmp.zip && \ > > Having to explode the zip here is unfortunate. This means we are creating an > almost full copy of the whole src tree in the build directory, something I > tried to avoid by leveraging the include/exclude functionality of zip, > instead of generating make rules for copying the files I wanted into a source > tree to run zip on. This may be a small overhead on Linux, but I'm pretty > sure it will be very noticeable on Windows. > > When reading about your tool at first, I assumed it would read the > intermediate zip file directly when rebuilding the zip. I don't think > modifying it to do that would be too complicated, basically read and > processing ZipEntrys instead of walking the file system. @erikj79 so had a bit of a think, and part of the unzipping.. then re-gen'ing is not having to load all the entries into memory. You can't guarantee the order "zip" has created them in, so realistically i'd have to read all the ZipEntry's into memory, then re-write.. which we can do.. src.zip is only 55MB or so, so memory requirements won't be huge given src.zip is the only target here currently. - PR: https://git.openjdk.java.net/jdk/pull/6311
Re: RFR: 8276743: Make openjdk build Zip Archive generation "reproducible"
On Tue, 9 Nov 2021 15:00:59 GMT, Magnus Ihse Bursie wrote: > And have you verified that running with `zip -X` is not enough? I remember > checking into this before, but I don't remember the conclusions. The purpose > is to leave out some extra information, such as time stamps, but it might not > be enough to guarantee reproducible builds. Correct, I have tried that already, that does remove the extended timestamp information, and as long as the input source is already correctly timestamped it works to a point, that point being the "file ordering" is not-deterministic and I think is dependent on how the OS file queries returns files. From history this is what I tried: https://github.com/andrew-m-leonard/jdk-1/blob/f18b5826b9ae3e7e5750190a669edcfd8bb8b2cc/make/common/ZipArchive.gmk#L166 > Also, the name is not really a correct description at this point. Maybe a > straight `MakeZipReproducible` instead or something like that? MakeZipReproducible sounds good. - PR: https://git.openjdk.java.net/jdk/pull/6311
Re: RFR: 8276743: Make openjdk build Zip Archive generation "reproducible"
On Tue, 9 Nov 2021 14:57:52 GMT, Magnus Ihse Bursie wrote: > We already have an --enable-reproducible-builds. If (and I say if) we need to > turn this on/off with a flag, this would to fine. > > However, as I have said previously in a private discussion with Andrew, I > prefer it if we can make reproducible builds so cheap, reliable and > uncontroversial so we can always to reproducible builds, and remove that flag. > > For this case, I think it depends on two things: > > 1. the extra time to make the zip file reproducible. Some benchmarking on > the GenerateZip for src.zip would be good to have, at least for some ballpark > figures. > > 2. When is src.zip built? (I don't remember right now) If it is part of > the exploded-image, then it is really time sensitive (and should maybe move > out of that target). If it part of the jdk-image, I'd say it is not as > sensitive, due to >a) this is very much slower anyway, and >b) that part is much more parallelizible, so src.zip can be produced > while waiting for jlink or whatever. @magicus 1) I'll do some rough benchmarks 2) "zip-source" is only dependent on "gensrc", ie.all module -gensrc stages must have completed, so in theory it should run in parallel while the linking goes on, a quick scan of a parallel build log seems to confirm that, the "Updating support/src.zip" occurs quite some time before the exploded image linking: 20:34:45 Compiling 31 files for jdk.management.agent 20:34:45 Compiling 16 files for jdk.security.jgss 20:34:45 Compiling 30 files for jdk.security.auth 20:34:45 Creating support/native/java.security.jgss/libj2gss/static/libj2gss.a from 3 file(s) 20:34:45 Updating support/src.zip 20:34:46 Creating support/native/jdk.management.agent/libmanagement_agent/static/libmanagement_agent.a from 1 file(s) 20:34:46 Creating support/native/jdk.security.auth/libjaas/static/libjaas.a from 1 file(s) 20:34:48 Compiling 136 files for jdk.jdeps 20:34:48 Creating support/test/lib-test/jtreg/native/bin/jvm-test-launcher from 1 file(s) 20:34:49 Creating support/modules_libs/java.base/libverify.so from 1 file(s) . 20:36:17 Optimizing the exploded image 20:36:18 Creating java.datatransfer.jmod 20:36:18 Creating java.instrument.jmod 20:36:18 Creating java.compiler.jmod - PR: https://git.openjdk.java.net/jdk/pull/6311
Re: RFR: 8276743: Make openjdk build Zip Archive generation "reproducible"
On Tue, 9 Nov 2021 17:31:16 GMT, Erik Joelsson wrote: >> You are already keeping all the filenames in memory for sorting, so reading >> up the ZipEntry:s isn't that much more data, just some extra metadata for >> each entry. The actual file contents is not part of the ZipEntry object. >> When actually copying the files, you can use the ZipFile class to access >> ZipEntry's in arbitrary order to read their streams as InputStream. > > Actually, you don't even need to save the ZipEntry:s in memory, you can just > extract filenames from them on the first pass, sort them, then lookup the > entries in ZipFile again on the second lap. :) I don't think that's necessary > though. @erikj79 thanks I didn't realize you can do that: "you can use the ZipFile class to access ZipEntry's in arbitrary order" - PR: https://git.openjdk.java.net/jdk/pull/6311
Re: RFR: 8276743: Make openjdk build Zip Archive generation "reproducible" [v2]
> This PR adds a new openjdk build tool GenerateZip, which generates a final > "zip" file from an input folder, and creates it in a deterministic way, > ensuring ordering and timestamps are set as specified. > > Using this tool in ZipArchive.gmk will ensure src.zip is then created > deterministically. > > Signed-off-by: Andrew Leonard Andrew Leonard has updated the pull request incrementally with one additional commit since the last revision: 8276743: Make openjdk build Zip Archive generation "reproducible" Signed-off-by: Andrew Leonard - Changes: - all: https://git.openjdk.java.net/jdk/pull/6311/files - new: https://git.openjdk.java.net/jdk/pull/6311/files/dcf48d65..f8a816af Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6311&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6311&range=00-01 Stats: 544 lines in 5 files changed: 241 ins; 287 del; 16 mod Patch: https://git.openjdk.java.net/jdk/pull/6311.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6311/head:pull/6311 PR: https://git.openjdk.java.net/jdk/pull/6311
Re: RFR: 8276743: Make openjdk build Zip Archive generation "reproducible" [v2]
On Tue, 9 Nov 2021 14:00:18 GMT, Erik Joelsson wrote: >> Andrew Leonard has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8276743: Make openjdk build Zip Archive generation "reproducible" >> >> Signed-off-by: Andrew Leonard > > make/common/ZipArchive.gmk line 29: > >> 27: _ZIP_ARCHIVE_GMK := 1 >> 28: >> 29: include ../ToolsJdk.gmk > > Should probably add a comment about inclusion of this file now requires an > explicit dependency on build-tools in Main.gmk. done - PR: https://git.openjdk.java.net/jdk/pull/6311
Re: RFR: 8276743: Make openjdk build Zip Archive generation "reproducible" [v2]
On Wed, 10 Nov 2021 11:22:39 GMT, Andrew Leonard wrote: >> Actually, you don't even need to save the ZipEntry:s in memory, you can just >> extract filenames from them on the first pass, sort them, then lookup the >> entries in ZipFile again on the second lap. :) I don't think that's >> necessary though. > > @erikj79 thanks I didn't realize you can do that: "you can use the ZipFile > class to access ZipEntry's in arbitrary order" See new commit - PR: https://git.openjdk.java.net/jdk/pull/6311
Re: RFR: 8276743: Make openjdk build Zip Archive generation "reproducible" [v3]
> This PR adds a new openjdk build tool GenerateZip, which generates a final > "zip" file from an input folder, and creates it in a deterministic way, > ensuring ordering and timestamps are set as specified. > > Using this tool in ZipArchive.gmk will ensure src.zip is then created > deterministically. > > Signed-off-by: Andrew Leonard Andrew Leonard has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: 8276743: Make openjdk build Zip Archive generation "reproducible" Signed-off-by: Andrew Leonard - Changes: - all: https://git.openjdk.java.net/jdk/pull/6311/files - new: https://git.openjdk.java.net/jdk/pull/6311/files/f8a816af..44036af7 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6311&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6311&range=01-02 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/6311.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6311/head:pull/6311 PR: https://git.openjdk.java.net/jdk/pull/6311
Re: RFR: 8276743: Make openjdk build Zip Archive generation "reproducible"
On Wed, 10 Nov 2021 14:39:09 GMT, Erik Joelsson wrote: >> @erikj79 The flag --enable-reproducible-builds sets >> ENABLE_REPRODUCIBLE_BUILD in spec.gmk. This is set by our JIB profiles. I >> propose that we also turn it on for GHA builds. >> >> I think that the post-processing of the zip file can be dependent on this >> variable and that it serves no purpose to introduce a separate variable >> ENABLE_REPRODUCIBLE_ZIP that is set to the same value as >> ENABLE_REPRODUCIBLE_BUILD. Do you agree? > >> I think that the post-processing of the zip file can be dependent on this >> variable and that it serves no purpose to introduce a separate variable >> ENABLE_REPRODUCIBLE_ZIP that is set to the same value as >> ENABLE_REPRODUCIBLE_BUILD. Do you agree? > > Sure, that works for me. @erikj79 @magicus I have just pushed a new commit with the suggested changes, and it works well, thank you for the help I've also done a basic average benchmarking, on my rather slow Ubuntu VM: - Existing src.zip processing : 18 seconds - Additional MakeZipReproducible : 6 seconds - PR: https://git.openjdk.java.net/jdk/pull/6311
Re: RFR: 8276743: Make openjdk build Zip Archive generation "reproducible" [v4]
> This PR adds a new openjdk build tool GenerateZip, which generates a final > "zip" file from an input folder, and creates it in a deterministic way, > ensuring ordering and timestamps are set as specified. > > Using this tool in ZipArchive.gmk will ensure src.zip is then created > deterministically. > > Signed-off-by: Andrew Leonard Andrew Leonard has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: 8276743: Make openjdk build Zip Archive generation "reproducible" Signed-off-by: Andrew Leonard - Changes: - all: https://git.openjdk.java.net/jdk/pull/6311/files - new: https://git.openjdk.java.net/jdk/pull/6311/files/44036af7..8d148065 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6311&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6311&range=02-03 Stats: 0 lines in 0 files changed: 0 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/6311.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6311/head:pull/6311 PR: https://git.openjdk.java.net/jdk/pull/6311
Re: RFR: 8276743: Make openjdk build Zip Archive generation "reproducible" [v4]
On Thu, 11 Nov 2021 20:18:40 GMT, Magnus Ihse Bursie wrote: > In the future, please refrain from force pushing to a PR. It makes history > hard to follow for reviewers, and is generally strongly discouraged. OpenJDK > uses the Skara tools which will automatically squash and rebase the commits > in the PR. @magicus I needed to cause a re-submit tests due to a macos timeout, and there seems no github Action or PR command to cause that, so I just force pushed, couldn't see any other way I presume there is? @magicus is pushing an empty commit or dummy change preferable? - PR: https://git.openjdk.java.net/jdk/pull/6311
Re: RFR: 8276743: Make openjdk build Zip Archive generation "reproducible" [v4]
On Thu, 11 Nov 2021 19:48:04 GMT, Andrew Leonard wrote: >> This PR adds a new openjdk build tool GenerateZip, which generates a final >> "zip" file from an input folder, and creates it in a deterministic way, >> ensuring ordering and timestamps are set as specified. >> >> Using this tool in ZipArchive.gmk will ensure src.zip is then created >> deterministically. >> >> Signed-off-by: Andrew Leonard > > Andrew Leonard has refreshed the contents of this pull request, and previous > commits have been removed. The incremental views will show differences > compared to the previous content of the PR. Looks like other PRs also getting mac or windows test failures.. - PR: https://git.openjdk.java.net/jdk/pull/6311
Re: RFR: 8276743: Make openjdk build Zip Archive generation "reproducible" [v5]
> This PR adds a new openjdk build tool GenerateZip, which generates a final > "zip" file from an input folder, and creates it in a deterministic way, > ensuring ordering and timestamps are set as specified. > > Using this tool in ZipArchive.gmk will ensure src.zip is then created > deterministically. > > Signed-off-by: Andrew Leonard Andrew Leonard has updated the pull request incrementally with one additional commit since the last revision: 8276743: Make openjdk build Zip Archive generation "reproducible" Signed-off-by: Andrew Leonard - Changes: - all: https://git.openjdk.java.net/jdk/pull/6311/files - new: https://git.openjdk.java.net/jdk/pull/6311/files/8d148065..ea477b9c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6311&range=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6311&range=03-04 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/6311.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6311/head:pull/6311 PR: https://git.openjdk.java.net/jdk/pull/6311
Re: RFR: 8276743: Make openjdk build Zip Archive generation "reproducible"
On Wed, 10 Nov 2021 14:39:09 GMT, Erik Joelsson wrote: >> @erikj79 The flag --enable-reproducible-builds sets >> ENABLE_REPRODUCIBLE_BUILD in spec.gmk. This is set by our JIB profiles. I >> propose that we also turn it on for GHA builds. >> >> I think that the post-processing of the zip file can be dependent on this >> variable and that it serves no purpose to introduce a separate variable >> ENABLE_REPRODUCIBLE_ZIP that is set to the same value as >> ENABLE_REPRODUCIBLE_BUILD. Do you agree? > >> I think that the post-processing of the zip file can be dependent on this >> variable and that it serves no purpose to introduce a separate variable >> ENABLE_REPRODUCIBLE_ZIP that is set to the same value as >> ENABLE_REPRODUCIBLE_BUILD. Do you agree? > > Sure, that works for me. @erikj79 all tests pass, ready for re-review please, thanks - PR: https://git.openjdk.java.net/jdk/pull/6311
Re: RFR: 8276743: Make openjdk build Zip Archive generation "reproducible" [v5]
On Fri, 12 Nov 2021 14:34:50 GMT, Erik Joelsson wrote: >> Sorry, I'm mis-reading the lines here. It's of course for docs-zip. Then >> it's perfectly in order! :-) > > It's using JarArchive.gmk, which is a similar, but different and probably > also needs the same treatment. We should probably share more code between > them. I believe so, it's target docs-zip not jrtfs : https://github.com/openjdk/jdk/blob/51a5731d6dc4b6f6feac920a4b8b49c15fd6b34f/make/Docs.gmk#L712 - PR: https://git.openjdk.java.net/jdk/pull/6311
Integrated: 8276743: Make openjdk build Zip Archive generation "reproducible"
On Tue, 9 Nov 2021 12:59:17 GMT, Andrew Leonard wrote: > This PR adds a new openjdk build tool MakeZipReproducible, which if > ENABLE_REPRODUCIBLE_BUILD is enabled, generates a final "zip" file in a > deterministic way, ensuring ordering and timestamps are set as specified. > > Using this tool in ZipArchive.gmk will ensure src.zip is then created > deterministically. > > Signed-off-by: Andrew Leonard This pull request has now been integrated. Changeset: aeba6530 Author:Andrew Leonard URL: https://git.openjdk.java.net/jdk/commit/aeba65303479130d9bab74484accad5d7d116a40 Stats: 261 lines in 4 files changed: 254 ins; 0 del; 7 mod 8276743: Make openjdk build Zip Archive generation "reproducible" Reviewed-by: erikj, ihse - PR: https://git.openjdk.java.net/jdk/pull/6311
RFR: 8277762: Allow configuration of HOTSPOT_BUILD_USER
To better allow "reproducible builds", a new configure parameter is added to set the USERNAME build variable, rather than always using the current user: --with-build-user= HOTSPOT_BUILD_USER is then set to a reproducible USERNAME if required. Signed-off-by: Andrew Leonard - Commit messages: - 8277762: Allow configuration of HOTSPOT_BUILD_USER - 8277762: Allow configuration of HOTSPOT_BUILD_USER - 8277762: Allow configuration of HOTSPOT_BUILD_USER Changes: https://git.openjdk.java.net/jdk/pull/6542/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6542&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8277762 Stats: 9 lines in 1 file changed: 6 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/6542.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6542/head:pull/6542 PR: https://git.openjdk.java.net/jdk/pull/6542
Re: RFR: 8277762: Allow configuration of HOTSPOT_BUILD_USER
On Wed, 24 Nov 2021 17:36:03 GMT, Erik Joelsson wrote: >> To better allow "reproducible builds", a new configure parameter is added to >> set the USERNAME build variable, rather than always using the current user: >> --with-build-user= >> HOTSPOT_BUILD_USER is then set to a reproducible USERNAME if required. >> >> Signed-off-by: Andrew Leonard > > make/autoconf/basic.m4 line 99: > >> 97: >> 98: # Setup username (for use in adhoc version strings etc) >> 99: AC_ARG_WITH([build-user], [AS_HELP_STRING([--with-build-user], > > This variable is currently initiated in a rather weird location given what > it's used for. As it used to be a one liner, it didn't seem worth the effort > to move it to a more suitable location, but when expanding it with a --with > flag I think we should also move this to jdk-version.m4, which is the only > place where it's currently used. I think it makes sense to treat this as part > of the version variables as that's what it's used for. it's also used in make/hotspot/lib/CompileJvm.gmk, but agree moving to jdk-versions.m4 makes sense - PR: https://git.openjdk.java.net/jdk/pull/6542
Re: RFR: 8277762: Allow configuration of HOTSPOT_BUILD_USER [v2]
On Wed, 24 Nov 2021 18:59:03 GMT, Erik Joelsson wrote: >> it's also used in make/hotspot/lib/CompileJvm.gmk, but agree moving to >> jdk-versions.m4 makes sense > > Right, I meant the only place it's used within configure. done, moved - PR: https://git.openjdk.java.net/jdk/pull/6542
Re: RFR: 8277762: Allow configuration of HOTSPOT_BUILD_USER [v2]
> To better allow "reproducible builds", a new configure parameter is added to > set the USERNAME build variable, rather than always using the current user: > --with-build-user= > HOTSPOT_BUILD_USER is then set to a reproducible USERNAME if required. > > Signed-off-by: Andrew Leonard Andrew Leonard has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision: - 8277762: Allow configuration of HOTSPOT_BUILD_USER Signed-off-by: Andrew Leonard - Merge branch 'master' of https://github.com/openjdk/jdk into builduser - 8277762: Allow configuration of HOTSPOT_BUILD_USER Signed-off-by: Andrew Leonard - 8277762: Allow configuration of HOTSPOT_BUILD_USER Signed-off-by: Andrew Leonard - 8277762: Allow configuration of HOTSPOT_BUILD_USER Signed-off-by: Andrew Leonard - Changes: - all: https://git.openjdk.java.net/jdk/pull/6542/files - new: https://git.openjdk.java.net/jdk/pull/6542/files/f9fb274a..3896df55 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6542&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6542&range=00-01 Stats: 6325 lines in 236 files changed: 4145 ins; 1326 del; 854 mod Patch: https://git.openjdk.java.net/jdk/pull/6542.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6542/head:pull/6542 PR: https://git.openjdk.java.net/jdk/pull/6542
Re: RFR: 8277762: Allow configuration of HOTSPOT_BUILD_USER [v2]
On Wed, 1 Dec 2021 15:22:09 GMT, Andrew Leonard wrote: >> To better allow "reproducible builds", a new configure parameter is added to >> set the USERNAME build variable, rather than always using the current user: >> --with-build-user= >> HOTSPOT_BUILD_USER is then set to a reproducible USERNAME if required. >> >> Signed-off-by: Andrew Leonard > > Andrew Leonard has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains five additional > commits since the last revision: > > - 8277762: Allow configuration of HOTSPOT_BUILD_USER > >Signed-off-by: Andrew Leonard > - Merge branch 'master' of https://github.com/openjdk/jdk into builduser > - 8277762: Allow configuration of HOTSPOT_BUILD_USER > >Signed-off-by: Andrew Leonard > - 8277762: Allow configuration of HOTSPOT_BUILD_USER > >Signed-off-by: Andrew Leonard > - 8277762: Allow configuration of HOTSPOT_BUILD_USER > >Signed-off-by: Andrew Leonard @erikj79 ready for review again please thanks - PR: https://git.openjdk.java.net/jdk/pull/6542
Integrated: 8277762: Allow configuration of HOTSPOT_BUILD_USER
On Wed, 24 Nov 2021 16:49:31 GMT, Andrew Leonard wrote: > To better allow "reproducible builds", a new configure parameter is added to > set the USERNAME build variable, rather than always using the current user: > --with-build-user= > HOTSPOT_BUILD_USER is then set to a reproducible USERNAME if required. > > Signed-off-by: Andrew Leonard This pull request has now been integrated. Changeset: f41e768b Author:Andrew Leonard URL: https://git.openjdk.java.net/jdk/commit/f41e768bba2b2ce3b3cc5813ccb1ac4984dcefbd Stats: 17 lines in 2 files changed: 11 ins; 5 del; 1 mod 8277762: Allow configuration of HOTSPOT_BUILD_USER Reviewed-by: erikj - PR: https://git.openjdk.java.net/jdk/pull/6542
RFR: 8278080: Add --with-cacerts-src='user cacerts folder' to enable determinsitic cacerts generation
Addition of a configure option --with-cacerts-src='user cacerts folder' to allow developers to specify their own cacerts PEM folder for generation of the cacerts store using the deterministic openjdk GenerateCacerts tool. Signed-off-by: Andrew Leonard - Commit messages: - 8278080: Add --with-cacerts-src='user cacerts folder' to enable determinsitic cacerts generation - 8278080: Add --with-cacerts-src='user cacerts folder' to enable determinsitic cacerts generation Changes: https://git.openjdk.java.net/jdk/pull/6647/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6647&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8278080 Stats: 21 lines in 3 files changed: 21 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/6647.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6647/head:pull/6647 PR: https://git.openjdk.java.net/jdk/pull/6647
Re: RFR: 8278080: Add --with-cacerts-src='user cacerts folder' to enable deterministic cacerts generation
On Thu, 2 Dec 2021 00:09:31 GMT, Sergey Bylokhov wrote: > I have a question related to the custom cacerts which can be added to the > OpenJDK bundle. How do you pass the tests like > test/jdk/sun/security/lib/cacerts/VerifyCACerts.java using that custom jdk > bundle? Probably we can add an additional configuration to that test so it > will check the custom cacerts passed to the build as well? @mrserb So VerifyCACerts is specific to the make/data/cacerts certificates, the README specifically states there that when those are updated VerifyCACerts needs updating. It checks things like fingerprints etc.. If a developer or other provider decide to provide their own cacerts file, then it is up to them to have verified and trust those certificates. They won't run the VerifyCACerts which is specific to the openjdk certs. This is the case at Adoptium for example, which uses the Mozilla trusted CA certs. - PR: https://git.openjdk.java.net/jdk/pull/6647
Re: RFR: 8278080: Add --with-cacerts-src='user cacerts folder' to enable deterministic cacerts generation [v2]
> Addition of a configure option --with-cacerts-src='user cacerts folder' to > allow developers to specify their own cacerts PEM folder for generation of > the cacerts store using the deterministic openjdk GenerateCacerts tool. > > Signed-off-by: Andrew Leonard Andrew Leonard has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision: - 8278080: Add --with-cacerts-src='user cacerts folder' to enable deterministic cacerts generation Signed-off-by: Andrew Leonard - Merge branch 'master' of https://github.com/openjdk/jdk into cacertssrc - 8278080: Add --with-cacerts-src='user cacerts folder' to enable determinsitic cacerts generation Signed-off-by: Andrew Leonard - 8278080: Add --with-cacerts-src='user cacerts folder' to enable determinsitic cacerts generation Signed-off-by: Andrew Leonard - Changes: - all: https://git.openjdk.java.net/jdk/pull/6647/files - new: https://git.openjdk.java.net/jdk/pull/6647/files/1084c4e1..16c5ca6b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6647&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6647&range=00-01 Stats: 1879 lines in 62 files changed: 1045 ins; 297 del; 537 mod Patch: https://git.openjdk.java.net/jdk/pull/6647.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6647/head:pull/6647 PR: https://git.openjdk.java.net/jdk/pull/6647
Re: RFR: 8278080: Add --with-cacerts-src='user cacerts folder' to enable deterministic cacerts generation [v2]
On Wed, 1 Dec 2021 18:47:41 GMT, Erik Joelsson wrote: >> Andrew Leonard has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains four additional >> commits since the last revision: >> >> - 8278080: Add --with-cacerts-src='user cacerts folder' to enable >> deterministic cacerts generation >> >>Signed-off-by: Andrew Leonard >> - Merge branch 'master' of https://github.com/openjdk/jdk into cacertssrc >> - 8278080: Add --with-cacerts-src='user cacerts folder' to enable >> determinsitic cacerts generation >> >>Signed-off-by: Andrew Leonard >> - 8278080: Add --with-cacerts-src='user cacerts folder' to enable >> determinsitic cacerts generation >> >>Signed-off-by: Andrew Leonard > > make/autoconf/jdk-options.m4 line 176: > >> 174: [specify alternative cacerts source folder containing >> certificates])]) >> 175: AC_MSG_CHECKING([for cacerts source]) >> 176: if test "x$with_cacerts_src" == x; then > > Before this if block, please assign an empty value to CACERTS_SRC. Otherwise, > if the user happens to have that variable set in the environment, that value > will get recorded by configure, which is usually not something we want. done - PR: https://git.openjdk.java.net/jdk/pull/6647
Re: RFR: 8278080: Add --with-cacerts-src='user cacerts folder' to enable deterministic cacerts generation [v2]
On Thu, 2 Dec 2021 14:29:00 GMT, Sean Mullan wrote: > I don’t have any major concerns with this change, as long as the default > cacerts are still the ones that are in the JDK. As an aside, using Mozilla's > root certificates might be fine for TLS certificates, but if you need to > support code signing certificates you may run into issues with missing CAs as > Mozilla's Root program does not support that use case. Also, by overriding > the roots included in the JDK, you are taking on the responsibility (which is > significant, in my opinion) of ensuring that those roots are trusted and have > not been compromised, revoked, have weak keys, etc. @seanjmullan Thanks Sean, I'll pass your comment on, cheers Andrew - PR: https://git.openjdk.java.net/jdk/pull/6647
Integrated: 8278080: Add --with-cacerts-src='user cacerts folder' to enable deterministic cacerts generation
On Wed, 1 Dec 2021 18:30:06 GMT, Andrew Leonard wrote: > Addition of a configure option --with-cacerts-src='user cacerts folder' to > allow developers to specify their own cacerts PEM folder for generation of > the cacerts store using the deterministic openjdk GenerateCacerts tool. > > Signed-off-by: Andrew Leonard This pull request has now been integrated. Changeset: dc2abc9f Author:Andrew Leonard URL: https://git.openjdk.java.net/jdk/commit/dc2abc9f05c2b7c52aeb242082359c48963f9854 Stats: 22 lines in 3 files changed: 22 ins; 0 del; 0 mod 8278080: Add --with-cacerts-src='user cacerts folder' to enable deterministic cacerts generation Reviewed-by: erikj - PR: https://git.openjdk.java.net/jdk/pull/6647
Re: RFR: 8278080: Add --with-cacerts-src='user cacerts folder' to enable deterministic cacerts generation [v2]
On Thu, 2 Dec 2021 17:35:36 GMT, Magnus Ihse Bursie wrote: >> make/modules/java.base/Gendata.gmk line 76: >> >>> 74: ifneq ($(CACERTS_SRC), ) >>> 75: GENDATA_CACERTS_SRC := $(CACERTS_SRC) >>> 76: endif >> >> Does this even work?! You are reassigning the variable after it has been >> used. The := assignment means that it not a macro. > > I would have expected to see something like: > > ifneq ($(CACERTS_SRC), ) > GENDATA_CACERTS_SRC := $(CACERTS_SRC) > else > GENDATA_CACERTS_SRC := $(TOPDIR)/make/data/cacerts/ > endif > > at line 63. you make a valid point, but i've tested this numerous times, but let me check again - PR: https://git.openjdk.java.net/jdk/pull/6647
Re: RFR: 8278080: Add --with-cacerts-src='user cacerts folder' to enable deterministic cacerts generation [v2]
On Thu, 2 Dec 2021 17:46:35 GMT, Andrew Leonard wrote: >> I would have expected to see something like: >> >> ifneq ($(CACERTS_SRC), ) >> GENDATA_CACERTS_SRC := $(CACERTS_SRC) >> else >> GENDATA_CACERTS_SRC := $(TOPDIR)/make/data/cacerts/ >> endif >> >> at line 63. > > you make a valid point, but i've tested this numerous times, but let me check > again my assumption was the recipe gets resolved later - PR: https://git.openjdk.java.net/jdk/pull/6647
Re: RFR: 8278080: Add --with-cacerts-src='user cacerts folder' to enable deterministic cacerts generation [v2]
On Thu, 2 Dec 2021 17:48:04 GMT, Andrew Leonard wrote: >> you make a valid point, but i've tested this numerous times, but let me >> check again > > my assumption was the recipe gets resolved later this was my understanding: https://www.gnu.org/software/make/manual/html_node/Variables-in-Recipes.html This occurs after make has finished reading all the makefiles and the target is determined to be out of date; so, the recipes for targets which are not rebuilt are never expanded. but i'm going to double check I was checking the resultant cacerts correctly in my tests - PR: https://git.openjdk.java.net/jdk/pull/6647
Re: RFR: 8278080: Add --with-cacerts-src='user cacerts folder' to enable deterministic cacerts generation [v2]
On Thu, 2 Dec 2021 18:46:09 GMT, Erik Joelsson wrote: >> this was my understanding: >> https://www.gnu.org/software/make/manual/html_node/Variables-in-Recipes.html >> >> This occurs after make has finished reading all the makefiles and the target >> is determined to be out of date; so, the recipes for targets which are not >> rebuilt are never expanded. >> >> but i'm going to double check I was checking the resultant cacerts correctly >> in my tests > > Oh, I didn't expand the diff far enough to actually see the context correctly > when I reviewed this as I would never have imagined the conditional to be > placed after the rule. While this will work as so far as using the correct > files, incremental builds will not be correct, because the rules are defined > in the first pass. > > I very much agree with Magnus that this conditional belongs around line 63. yes, thanks, feeling rather stupid here! i'll raise an issue to fix - PR: https://git.openjdk.java.net/jdk/pull/6647
RFR: 8278163: --with-cacerts-src variable resolved after GenerateCacerts recipe setup
PR https://github.com/openjdk/jdk/pull/6647 resolved the GENDATA_CACERTS_SRC fir --with-cacerts-src after the recipe, which meant the dependencies were wrong. This PR moves it before the recipe. Signed-off-by: Andrew Leonard - Commit messages: - 8278163: --with-cacerts-src variable resolved after GenerateCacerts recipe setup Changes: https://git.openjdk.java.net/jdk/pull/6680/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6680&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8278163 Stats: 8 lines in 1 file changed: 4 ins; 3 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/6680.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6680/head:pull/6680 PR: https://git.openjdk.java.net/jdk/pull/6680
Integrated: 8278163: --with-cacerts-src variable resolved after GenerateCacerts recipe setup
On Thu, 2 Dec 2021 21:07:30 GMT, Andrew Leonard wrote: > PR https://github.com/openjdk/jdk/pull/6647 resolved the GENDATA_CACERTS_SRC > fir --with-cacerts-src after the recipe, which meant the dependencies were > wrong. > This PR moves it before the recipe. > > Signed-off-by: Andrew Leonard This pull request has now been integrated. Changeset: 45da3aea Author:Andrew Leonard URL: https://git.openjdk.java.net/jdk/commit/45da3aea22fd85f214e661b2c98631cb91ddb55d Stats: 8 lines in 1 file changed: 4 ins; 3 del; 1 mod 8278163: --with-cacerts-src variable resolved after GenerateCacerts recipe setup Reviewed-by: ihse - PR: https://git.openjdk.java.net/jdk/pull/6680
RFR: 8278766: Enable OpenJDK build support for reproducible jars and jmods using --date
If "reproducible build" is enabled, then utilize the new --date option in the building of jar's and jmods. Validating the boot jdk supports --date for the building of the jars. Signed-off-by: Andrew Leonard - Commit messages: - 8278766: Enable OpenJDK build support for reproducible jars and jmods using --date Changes: https://git.openjdk.java.net/jdk/pull/6878/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6878&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8278766 Stats: 55 lines in 6 files changed: 40 ins; 3 del; 12 mod Patch: https://git.openjdk.java.net/jdk/pull/6878.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6878/head:pull/6878 PR: https://git.openjdk.java.net/jdk/pull/6878
Re: RFR: 8278766: Enable OpenJDK build support for reproducible jars and jmods using --date
On Fri, 17 Dec 2021 11:18:10 GMT, Magnus Ihse Bursie wrote: >> If "reproducible build" is enabled, then utilize the new --date option in >> the building of jar's and jmods. >> Validating the boot jdk supports --date for the building of the jars. >> >> Signed-off-by: Andrew Leonard > > make/common/MakeBase.gmk line 145: > >> 143: # If reproducible builds are enabled then set SOURCE_DATE_ISO_8601 >> string variable >> 144: ifeq ($(ENABLE_REPRODUCIBLE_BUILD), true) >> 145: SOURCE_DATE_ISO_8601 := $(shell $(DATE) --utc >> --date="@$(SOURCE_DATE_EPOCH)" +"%Y-%m-%dT%H:%M:%SZ" 2> /dev/null) > > This should be set up by the configure script. Look for where we are > currently doing the SOURCE_DATE_EPOCH stuff. > > Also consider the possibility to *just* use this variable, and pass it on to > jar if it is present, and not if it's missing. That way you can get rid of > the redundant boolean variable. @magicus I looked at that, but the problem is SOURCE_DATE_EPOCH is set in InitSupport.gmk depending on whether SOURCE_DATE=="updated" : https://github.com/openjdk/jdk/blob/739769c8fc4b496f08a92225a12d07414537b6c0/make/InitSupport.gmk#L315 I also couldn't add it in InitSupport.gmk because that marco is not included from every place SetupJarArchive is resolved. Thoughts? - PR: https://git.openjdk.java.net/jdk/pull/6878
Re: RFR: 8278766: Enable OpenJDK build support for reproducible jars and jmods using --date
On Fri, 17 Dec 2021 17:54:46 GMT, Erik Joelsson wrote: >> If "reproducible build" is enabled, then utilize the new --date option in >> the building of jar's and jmods. >> Validating the boot jdk supports --date for the building of the jars. >> >> Signed-off-by: Andrew Leonard > > make/common/MakeBase.gmk line 148: > >> 146: ifeq ($(SOURCE_DATE_ISO_8601), ) >> 147: # GNU date format did not work, try BSD date options >> 148: SOURCE_DATE_ISO_8601 := $(shell $(DATE) -u -j -f "%s" >> "$(SOURCE_DATE_EPOCH)" +"%Y-%m-%dT%H:%M:%SZ" 2> /dev/null) > > Could we maybe figure out the date command line to use in configure instead > to avoid the trial and error at build time? I based this trial and error based on util.m4 code and copied the method, so yes I can store the result from then... - PR: https://git.openjdk.java.net/jdk/pull/6878
