Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports (build system)

[Mikael Vidstedt]

Good catch on the deprecation argument! Updated webrev here:

webrev: 
http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.02/build/open/webrev/ 

incremental: 
http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.02/build.incr/open/webrev/
 


I did not include the zero related part of the patch to avoid conflating things 
- I’ll wait for Adrian to provide a complete patch instead.

Cheers,
Mikael

> On May 7, 2020, at 8:53 AM, Magnus Ihse Bursie 
>  wrote:
> 
> 
> 
> On 2020-05-07 13:18, John Paul Adrian Glaubitz wrote:
>> Hi Mikael!
>> 
>> On 5/7/20 2:47 AM, Mikael Vidstedt wrote:
>>> New webrev available here:
>>> 
>>> webrev: 
>>> http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.01/build/open/webrev/
>>>  
>>> 
>>> incremental: 
>>> http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.01/build.incr/open/webrev/
>>>  
>>> 
>>> 
>>> I believe I have addressed all the review comments apart from:
>>> 
>>> * GNM - Magnus to file a follow-up RFE
>>> 
>>> * Zero - Waiting for somebody (Adrian?) to provide more information about 
>>> what needs to be preserved
>> I haven't had the time to look at this yet due to $DAYJOB@SUSE, but I should 
>> have time tomorrow
>> as there is a public holiday tomorrow.
>> 
>> Also, Magnus should get to a SPARC-T5 running Debian unstable within the 
>> next hours so he
>> will be able to perform build tests as well and provide feedback.
> That was quite painless, actually. Two files needed partial restoration, then 
> everything worked fine.
> 
> Mikael, if you apply this patch on top of your patchset, it should back out 
> those of your changes that broke linux-sparc-zero. Also, I missed that you 
> removed a configure option without deprecating it, something we have as a 
> policy not to do. This patch also fixes that.
> 
> diff --git a/make/autoconf/platform.m4 b/make/autoconf/platform.m4
> --- a/make/autoconf/platform.m4
> +++ b/make/autoconf/platform.m4
> @@ -150,6 +150,18 @@
>VAR_CPU_BITS=32
>VAR_CPU_ENDIAN=little
>;;
> +sparc)
> +  VAR_CPU=sparc
> +  VAR_CPU_ARCH=sparc
> +  VAR_CPU_BITS=32
> +  VAR_CPU_ENDIAN=big
> +  ;;
> +sparcv9|sparc64)
> +  VAR_CPU=sparcv9
> +  VAR_CPU_ARCH=sparc
> +  VAR_CPU_BITS=64
> +  VAR_CPU_ENDIAN=big
> +  ;;
>  *)
>AC_MSG_ERROR([unsupported cpu $1])
>;;
> @@ -308,8 +320,10 @@
>OPENJDK_TARGET_CPU_BITS=32
>if test "x$OPENJDK_TARGET_CPU_ARCH" = "xx86"; then
>  OPENJDK_TARGET_CPU=x86
> +  elif test "x$OPENJDK_TARGET_CPU_ARCH" = "xsparc"; then
> +OPENJDK_TARGET_CPU=sparc
>else
> -AC_MSG_ERROR([Reduced build (--with-target-bits=32) is only 
> supported on x86_64])
> +AC_MSG_ERROR([Reduced build (--with-target-bits=32) is only 
> supported on x86_64 and sparcv9])
>fi
>  elif test "x$with_target_bits" = x64 && test "x$OPENJDK_TARGET_CPU_BITS" 
> = x32; then
>AC_MSG_ERROR([It is not possible to use --with-target-bits=64 on a 32 
> bit system. Use proper cross-compilation instead.])
> @@ -422,6 +436,8 @@
>HOTSPOT_$1_CPU=${OPENJDK_$1_CPU}
>if test "x$OPENJDK_$1_CPU" = xx86; then
>  HOTSPOT_$1_CPU=x86_32
> +  elif test "x$OPENJDK_$1_CPU" = xsparcv9; then
> +HOTSPOT_$1_CPU=sparc
>elif test "x$OPENJDK_$1_CPU" = xppc64; then
>  HOTSPOT_$1_CPU=ppc_64
>elif test "x$OPENJDK_$1_CPU" = xppc64le; then
> @@ -440,6 +456,8 @@
>  HOTSPOT_$1_CPU_DEFINE=AMD64
>elif test "x$OPENJDK_$1_CPU" = xx32; then
>  HOTSPOT_$1_CPU_DEFINE=X32
> +  elif test "x$OPENJDK_$1_CPU" = xsparcv9; then
> +HOTSPOT_$1_CPU_DEFINE=SPARC
>elif test "x$OPENJDK_$1_CPU" = xaarch64; then
>  HOTSPOT_$1_CPU_DEFINE=AARCH64
>elif test "x$OPENJDK_$1_CPU" = xppc64; then
> @@ -448,6 +466,8 @@
>  HOTSPOT_$1_CPU_DEFINE=PPC64
> 
># The cpu defines below are for zero, we don't support them directly.
> +  elif test "x$OPENJDK_$1_CPU" = xsparc; then
> +HOTSPOT_$1_CPU_DEFINE=SPARC
>elif test "x$OPENJDK_$1_CPU" = xppc; then
>  HOTSPOT_$1_CPU_DEFINE=PPC32
>elif test "x$OPENJDK_$1_CPU" = xs390; then
> @@ -526,6 +546,9 @@
>PLATFORM_SET_MODULE_TARGET_OS_VALUES
>PLATFORM_SET_RELEASE_FILE_OS_VALUES
>PLATFORM_SETUP_LEGACY_VARS
> +
> +  # Deprecated in JDK 15
> +  UTIL_DEPRECATED_ARG_ENABLE(deprecated-ports)
>  ])
> 
>  AC_DEFUN_ONCE([PLATFORM_SETUP_OPENJDK_BUILD_OS_VERSION],
> diff --git a/src/hotspot/os/linux/os_linux.cpp 
> b/src/hotspot/os/linux/os_linux.cpp
> --- a/src/hotspot/os/linux/os_linux.cpp
> +++ b/src/hotspot/os/linux/os_linux.cpp
> @@ -318,7 +318,7 @@
> 
> 
>  #ifndef SYS_gettid
> -// i386: 

Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports (build system)

[Magnus Ihse Bursie]

On 2020-05-07 13:18, John Paul Adrian Glaubitz wrote:

Hi Mikael!

On 5/7/20 2:47 AM, Mikael Vidstedt wrote:

New webrev available here:

webrev: 
http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.01/build/open/webrev/ 

incremental: 
http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.01/build.incr/open/webrev/ 


I believe I have addressed all the review comments apart from:

* GNM - Magnus to file a follow-up RFE

* Zero - Waiting for somebody (Adrian?) to provide more information about what 
needs to be preserved

I haven't had the time to look at this yet due to $DAYJOB@SUSE, but I should 
have time tomorrow
as there is a public holiday tomorrow.

Also, Magnus should get to a SPARC-T5 running Debian unstable within the next 
hours so he
will be able to perform build tests as well and provide feedback.
That was quite painless, actually. Two files needed partial restoration, 
then everything worked fine.


Mikael, if you apply this patch on top of your patchset, it should back 
out those of your changes that broke linux-sparc-zero. Also, I missed 
that you removed a configure option without deprecating it, something we 
have as a policy not to do. This patch also fixes that.


diff --git a/make/autoconf/platform.m4 b/make/autoconf/platform.m4
--- a/make/autoconf/platform.m4
+++ b/make/autoconf/platform.m4
@@ -150,6 +150,18 @@
   VAR_CPU_BITS=32
   VAR_CPU_ENDIAN=little
   ;;
+    sparc)
+  VAR_CPU=sparc
+  VAR_CPU_ARCH=sparc
+  VAR_CPU_BITS=32
+  VAR_CPU_ENDIAN=big
+  ;;
+    sparcv9|sparc64)
+  VAR_CPU=sparcv9
+  VAR_CPU_ARCH=sparc
+  VAR_CPU_BITS=64
+  VAR_CPU_ENDIAN=big
+  ;;
 *)
   AC_MSG_ERROR([unsupported cpu $1])
   ;;
@@ -308,8 +320,10 @@
   OPENJDK_TARGET_CPU_BITS=32
   if test "x$OPENJDK_TARGET_CPU_ARCH" = "xx86"; then
 OPENJDK_TARGET_CPU=x86
+  elif test "x$OPENJDK_TARGET_CPU_ARCH" = "xsparc"; then
+    OPENJDK_TARGET_CPU=sparc
   else
-    AC_MSG_ERROR([Reduced build (--with-target-bits=32) is only 
supported on x86_64])
+    AC_MSG_ERROR([Reduced build (--with-target-bits=32) is only 
supported on x86_64 and sparcv9])

   fi
 elif test "x$with_target_bits" = x64 && test 
"x$OPENJDK_TARGET_CPU_BITS" = x32; then
   AC_MSG_ERROR([It is not possible to use --with-target-bits=64 on 
a 32 bit system. Use proper cross-compilation instead.])

@@ -422,6 +436,8 @@
   HOTSPOT_$1_CPU=${OPENJDK_$1_CPU}
   if test "x$OPENJDK_$1_CPU" = xx86; then
 HOTSPOT_$1_CPU=x86_32
+  elif test "x$OPENJDK_$1_CPU" = xsparcv9; then
+    HOTSPOT_$1_CPU=sparc
   elif test "x$OPENJDK_$1_CPU" = xppc64; then
 HOTSPOT_$1_CPU=ppc_64
   elif test "x$OPENJDK_$1_CPU" = xppc64le; then
@@ -440,6 +456,8 @@
 HOTSPOT_$1_CPU_DEFINE=AMD64
   elif test "x$OPENJDK_$1_CPU" = xx32; then
 HOTSPOT_$1_CPU_DEFINE=X32
+  elif test "x$OPENJDK_$1_CPU" = xsparcv9; then
+    HOTSPOT_$1_CPU_DEFINE=SPARC
   elif test "x$OPENJDK_$1_CPU" = xaarch64; then
 HOTSPOT_$1_CPU_DEFINE=AARCH64
   elif test "x$OPENJDK_$1_CPU" = xppc64; then
@@ -448,6 +466,8 @@
 HOTSPOT_$1_CPU_DEFINE=PPC64

   # The cpu defines below are for zero, we don't support them directly.
+  elif test "x$OPENJDK_$1_CPU" = xsparc; then
+    HOTSPOT_$1_CPU_DEFINE=SPARC
   elif test "x$OPENJDK_$1_CPU" = xppc; then
 HOTSPOT_$1_CPU_DEFINE=PPC32
   elif test "x$OPENJDK_$1_CPU" = xs390; then
@@ -526,6 +546,9 @@
   PLATFORM_SET_MODULE_TARGET_OS_VALUES
   PLATFORM_SET_RELEASE_FILE_OS_VALUES
   PLATFORM_SETUP_LEGACY_VARS
+
+  # Deprecated in JDK 15
+  UTIL_DEPRECATED_ARG_ENABLE(deprecated-ports)
 ])

 AC_DEFUN_ONCE([PLATFORM_SETUP_OPENJDK_BUILD_OS_VERSION],
diff --git a/src/hotspot/os/linux/os_linux.cpp 
b/src/hotspot/os/linux/os_linux.cpp

--- a/src/hotspot/os/linux/os_linux.cpp
+++ b/src/hotspot/os/linux/os_linux.cpp
@@ -318,7 +318,7 @@


 #ifndef SYS_gettid
-// i386: 224, ia64: 1105, amd64: 186
+// i386: 224, ia64: 1105, amd64: 186, sparc 143
   #ifdef __ia64__
 #define SYS_gettid 1105
   #else
@@ -328,7 +328,11 @@
   #ifdef __amd64__
 #define SYS_gettid 186
   #else
-    #error define gettid for the arch
+    #ifdef __sparc__
+  #define SYS_gettid 143
+    #else
+  #error define gettid for the arch
+    #endif
   #endif
 #endif
   #endif
@@ -404,7 +408,7 @@
   //    ...
   //    7: The default directories, normally /lib and /usr/lib.
 #ifndef OVERRIDE_LIBPATH
-  #if defined(AMD64) || defined(PPC64) || defined(S390)
+  #if defined(AMD64) || (defined(_LP64) && defined(SPARC)) || 
defined(PPC64) || defined(S390)

 #define DEFAULT_LIBPATH "/usr/lib64:/lib64:/lib:/usr/lib"
   #else
 #define DEFAULT_LIBPATH "/lib:/usr/lib"
@@ -1854,6 +1858,9 @@
 {EM_486, EM_386, ELFCLASS32, ELFDATA2LSB, 

Re: RFR: JDK-8241616: Timestamps on ct.sym entries lead to non-reproducible builds

[Magnus Ihse Bursie]

On 2020-04-30 14:50, Jan Lahoda wrote:

On 30. 04. 20 14:29, Magnus Ihse Bursie wrote:

On 2020-04-30 12:41, Alan Bateman wrote:



On 30/04/2020 08:03, Jan Lahoda wrote:

Hi,

The building of lib/ct.sym is not reproducible, due to timestamps 
of files inside the file (which is basically a zip file).


The proposed solution is to use a constant timestamp for the files 
inside the ct.sym file. To simplify the construction, the 
CreateSymbols tool does not produce files on the filesystem 
anymore, but rather constructs the ct.sym directly.


Proposed webrev: 
http://cr.openjdk.java.net/~jlahoda/8241616/webrev.00/

JBS: https://bugs.openjdk.java.net/browse/JDK-8241616
1587656816359 = 2020-04-23T15:46:56.359Z. Is there anything 
significant with this timestamp? Magnus might have suggestions but 
maybe it would be saner to pick up the value of DEFAULT_VERSION_DATE.
There is an officially suggested standard, SOURCE_DATE_EPOCH for 
reproducible builds. [1] I have long-term vision to include this in 
the entire JDK build, but has of yet not even started on that. :-(


This might be a good first step to start using it. I can assist in 
making the needed makefile changes to have that environment variable 
available.


I think a standard mechanism would be great. I don't think this patch 
is very urgent, so I am happy to wait some time (rather than add a 
timestamp to symbols and then remove it when there's a standard 
mechanism).


JDK-8244592 is now in mainline, and you can start relying on 
SOURCE_DATE_EPOCH being present in the environment when building.


/Magnus


Thanks,
    Jan



/Magnus

[1] https://reproducible-builds.org/specs/source-date-epoch/


There is some curious code that generates the timestamp with:
   Long.toString(FileTime.from(Instant.now()).toMillis())
Could this use Instant.now().toEpochMilli() instead?

-Alan

Re: RFR: JDK-8244592 Start supporting SOURCE_DATE_EPOCH

[Magnus Ihse Bursie]

On 2020-05-07 17:34, Erik Joelsson wrote:

Looks good.

What tools do we know are picking up this variable today?
gcc will use it when replacing the __DATE__ and __TIME__ macros. [1] I 
know of no other tools that use it, but that might just be my limited 
knowledge. That's the reason I used "updated" as the default method, to 
set it to the time of building, which would minimize any differences 
produced by tools.


/Magnus

[1] https://gcc.gnu.org/onlinedocs/cpp/Environment-Variables.html


/Erik

On 2020-05-07 07:29, Magnus Ihse Bursie wrote:
As a part of the effort of creating a reproducible build, the build 
system should know about SOURCE_DATE_EPOCH and allow it to be set 
using different methods.


This fix is needed to unblock JDK-8241616. With this patch, the 
SOURCE_DATE_EPOCH variable will be available in the environment 
during the build.


I have added two options, --with-source-date and 
--enable-reproducible-build. The former controls what value 
SOURCE_DATE_EPOCH will get. The latter is a currently just a 
placeholder, but my intention is to continue adding functionality for 
making reproducible builds. (The reason that you might not always 
want to have reproducible builds enabled is that, for instance, one 
thing reproducible builds will do is set the timestamp for all 
created files, and this is not generally a good idea.) The idea is 
still that build changes that result in reproducible builds should be 
enabled always, if there is no downside to them.


If --with-source-date is specified on the command line, 
--enable-reproducible-build will be turned on by default, otherwise 
it will be turned off.


The --with-source-date option takes either a keyword option, an 
integer option, or a date/time string.
* If "updated" is specified (this is also the default), 
SOURCE_DATE_EPOCH will be set to the time when make is run.
* If "current" is specified, the value is "hard-coded" to the time 
when configure was run.
* If "version" is specified, the value is from DEFAULT_VERSION_DATE 
in make/autoconf/version-numbers is used. (This is probably not as 
good an idea as it might sound, but it can be used as a simple way to 
get the same value between different runs/points in time of the 
source code, but for the same version.)
* If an integer is used, that very number is used as the epoch-based 
timestamp.
* And finally, if an ISO-8059 date string (like "2020-05-07") is 
used, it will be converted to a timestamp. (On systems with GNU date, 
the parsing of the string is rather forgiving, on other systems 
(read: macOS) it will be more strict.)


Finally, this patch carries two unrelated changes.
* In make/autoconf/jdk-version.m4, I noticed an error with the bash 
regexp (which I used as starting point for parsing the timestamp 
number). It would have accepted e.g. "123banana" as a number, but not 
000 (even though 001 was accepted).
* In UTIL_ARG_ENABLE, I added a new argument IF_NOT_GIVEN. In the 
end, I did not need this, but I still think the functionality is 
worth keeping.


Bug: https://bugs.openjdk.java.net/browse/JDK-8244592
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8244592-add-SOURCE_DATE_EPOCH/webrev.01


/Magnus

Re: RFR: JDK-8244615 build-performance.m4 is not always parsing /proc/cpuinfo correctly

[Erik Joelsson]

Looks good.

/Erik

On 2020-05-07 08:11, Magnus Ihse Bursie wrote:
On some systems the format of /proc/cpuinfo differs from what 
build-performance.m4 expects. This patch will look for lines beginning 
with "CPU" if no lines with "processor" is found.


Bug: https://bugs.openjdk.java.net/browse/JDK-8244615
Patch inline:
diff --git a/make/autoconf/build-performance.m4 
b/make/autoconf/build-performance.m4

--- a/make/autoconf/build-performance.m4
+++ b/make/autoconf/build-performance.m4
@@ -32,7 +32,12 @@
   if test -f /proc/cpuinfo; then
 # Looks like a Linux (or cygwin) system
 NUM_CORES=`cat /proc/cpuinfo  | grep -c processor`
-    FOUND_CORES=yes
+    if test "$NUM_CORES" -eq "0"; then
+  NUM_CORES=`cat /proc/cpuinfo  | grep -c ^CPU`
+    fi
+    if test "$NUM_CORES" -ne "0"; then
+  FOUND_CORES=yes
+    fi
   elif test -x /usr/sbin/psrinfo; then
 # Looks like a Solaris system
 NUM_CORES=`/usr/sbin/psrinfo -v | grep -c on-line`

/Magnus

Re: RFR: JDK-8244592 Start supporting SOURCE_DATE_EPOCH

[Erik Joelsson]

Looks good.

What tools do we know are picking up this variable today?

/Erik

On 2020-05-07 07:29, Magnus Ihse Bursie wrote:
As a part of the effort of creating a reproducible build, the build 
system should know about SOURCE_DATE_EPOCH and allow it to be set 
using different methods.


This fix is needed to unblock JDK-8241616. With this patch, the 
SOURCE_DATE_EPOCH variable will be available in the environment during 
the build.


I have added two options, --with-source-date and 
--enable-reproducible-build. The former controls what value 
SOURCE_DATE_EPOCH will get. The latter is a currently just a 
placeholder, but my intention is to continue adding functionality for 
making reproducible builds. (The reason that you might not always want 
to have reproducible builds enabled is that, for instance, one thing 
reproducible builds will do is set the timestamp for all created 
files, and this is not generally a good idea.) The idea is still that 
build changes that result in reproducible builds should be enabled 
always, if there is no downside to them.


If --with-source-date is specified on the command line, 
--enable-reproducible-build will be turned on by default, otherwise it 
will be turned off.


The --with-source-date option takes either a keyword option, an 
integer option, or a date/time string.
* If "updated" is specified (this is also the default), 
SOURCE_DATE_EPOCH will be set to the time when make is run.
* If "current" is specified, the value is "hard-coded" to the time 
when configure was run.
* If "version" is specified, the value is from DEFAULT_VERSION_DATE in 
make/autoconf/version-numbers is used. (This is probably not as good 
an idea as it might sound, but it can be used as a simple way to get 
the same value between different runs/points in time of the source 
code, but for the same version.)
* If an integer is used, that very number is used as the epoch-based 
timestamp.
* And finally, if an ISO-8059 date string (like "2020-05-07") is used, 
it will be converted to a timestamp. (On systems with GNU date, the 
parsing of the string is rather forgiving, on other systems (read: 
macOS) it will be more strict.)


Finally, this patch carries two unrelated changes.
* In make/autoconf/jdk-version.m4, I noticed an error with the bash 
regexp (which I used as starting point for parsing the timestamp 
number). It would have accepted e.g. "123banana" as a number, but not 
000 (even though 001 was accepted).
* In UTIL_ARG_ENABLE, I added a new argument IF_NOT_GIVEN. In the end, 
I did not need this, but I still think the functionality is worth 
keeping.


Bug: https://bugs.openjdk.java.net/browse/JDK-8244592
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8244592-add-SOURCE_DATE_EPOCH/webrev.01


/Magnus

RFR: JDK-8244615 build-performance.m4 is not always parsing /proc/cpuinfo correctly

[Magnus Ihse Bursie]

On some systems the format of /proc/cpuinfo differs from what 
build-performance.m4 expects. This patch will look for lines beginning 
with "CPU" if no lines with "processor" is found.


Bug: https://bugs.openjdk.java.net/browse/JDK-8244615
Patch inline:
diff --git a/make/autoconf/build-performance.m4 
b/make/autoconf/build-performance.m4

--- a/make/autoconf/build-performance.m4
+++ b/make/autoconf/build-performance.m4
@@ -32,7 +32,12 @@
   if test -f /proc/cpuinfo; then
 # Looks like a Linux (or cygwin) system
 NUM_CORES=`cat /proc/cpuinfo  | grep -c processor`
-    FOUND_CORES=yes
+    if test "$NUM_CORES" -eq "0"; then
+  NUM_CORES=`cat /proc/cpuinfo  | grep -c ^CPU`
+    fi
+    if test "$NUM_CORES" -ne "0"; then
+  FOUND_CORES=yes
+    fi
   elif test -x /usr/sbin/psrinfo; then
 # Looks like a Solaris system
 NUM_CORES=`/usr/sbin/psrinfo -v | grep -c on-line`

/Magnus

RFR: JDK-8244592 Start supporting SOURCE_DATE_EPOCH

[Magnus Ihse Bursie]

As a part of the effort of creating a reproducible build, the build 
system should know about SOURCE_DATE_EPOCH and allow it to be set using 
different methods.


This fix is needed to unblock JDK-8241616. With this patch, the 
SOURCE_DATE_EPOCH variable will be available in the environment during 
the build.


I have added two options, --with-source-date and 
--enable-reproducible-build. The former controls what value 
SOURCE_DATE_EPOCH will get. The latter is a currently just a 
placeholder, but my intention is to continue adding functionality for 
making reproducible builds. (The reason that you might not always want 
to have reproducible builds enabled is that, for instance, one thing 
reproducible builds will do is set the timestamp for all created files, 
and this is not generally a good idea.) The idea is still that build 
changes that result in reproducible builds should be enabled always, if 
there is no downside to them.


If --with-source-date is specified on the command line, 
--enable-reproducible-build will be turned on by default, otherwise it 
will be turned off.


The --with-source-date option takes either a keyword option, an integer 
option, or a date/time string.
* If "updated" is specified (this is also the default), 
SOURCE_DATE_EPOCH will be set to the time when make is run.
* If "current" is specified, the value is "hard-coded" to the time when 
configure was run.
* If "version" is specified, the value is from DEFAULT_VERSION_DATE in 
make/autoconf/version-numbers is used. (This is probably not as good an 
idea as it might sound, but it can be used as a simple way to get the 
same value between different runs/points in time of the source code, but 
for the same version.)
* If an integer is used, that very number is used as the epoch-based 
timestamp.
* And finally, if an ISO-8059 date string (like "2020-05-07") is used, 
it will be converted to a timestamp. (On systems with GNU date, the 
parsing of the string is rather forgiving, on other systems (read: 
macOS) it will be more strict.)


Finally, this patch carries two unrelated changes.
* In make/autoconf/jdk-version.m4, I noticed an error with the bash 
regexp (which I used as starting point for parsing the timestamp 
number). It would have accepted e.g. "123banana" as a number, but not 
000 (even though 001 was accepted).
* In UTIL_ARG_ENABLE, I added a new argument IF_NOT_GIVEN. In the end, I 
did not need this, but I still think the functionality is worth keeping.


Bug: https://bugs.openjdk.java.net/browse/JDK-8244592
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8244592-add-SOURCE_DATE_EPOCH/webrev.01


/Magnus

[8u] RFR(XS): 8243059: Build fails when --with-vendor-name contains a comma

[Severin Gehwolf]

Hi,

Could I please get a review of this OpenJDK 8u backport for JDK-
8243059? The build system is wildly different to JDK 11 and later, thus
is the patch. In turns out on JDK 8, SetupLauncher isn't using eval()
so the evaluation of the comma too early isn't an issue there. However,
on JDK 8u, the same issue is present when COMPANY_NAME is being
evaluated too early in SetupArchive. COMPANY_NAME may contain a comma
an that breaks the embedded rule for creating the jars.

Bug: https://bugs.openjdk.java.net/browse/JDK-8243059
webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8243059/jdk8/01/webrev/

Testing: building with --with-vendor-name="foo, bar". Fails before,
passes after patch.

Thoughts?

Thanks,
Severin

Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports (build system)

[John Paul Adrian Glaubitz]

Hi Mikael!

On 5/7/20 2:47 AM, Mikael Vidstedt wrote:
> New webrev available here:
> 
> webrev: 
> http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.01/build/open/webrev/
>  
> 
> incremental: 
> http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.01/build.incr/open/webrev/
>  
> 
> 
> I believe I have addressed all the review comments apart from:
> 
> * GNM - Magnus to file a follow-up RFE
> 
> * Zero - Waiting for somebody (Adrian?) to provide more information about 
> what needs to be preserved
I haven't had the time to look at this yet due to $DAYJOB@SUSE, but I should 
have time tomorrow
as there is a public holiday tomorrow.

Also, Magnus should get to a SPARC-T5 running Debian unstable within the next 
hours so he
will be able to perform build tests as well and provide feedback.

Sorry for not being able to reply earlier.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaub...@debian.org
`. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
  `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports (build system)

[Kim Barrett]

> On May 6, 2020, at 8:47 PM, Mikael Vidstedt  
> wrote:
> 
> 
> New webrev available here:
> 
> webrev: 
> http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.01/build/open/webrev/
>  
> 
> incremental: 
> http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.01/build.incr/open/webrev/
>  
> 
> 
> I believe I have addressed all the review comments apart from:
> 
> * GNM - Magnus to file a follow-up RFE
> 
> * Zero - Waiting for somebody (Adrian?) to provide more information about 
> what needs to be preserved

Looks good.

Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports (build system)

[Magnus Ihse Bursie]

On 2020-05-07 02:47, Mikael Vidstedt wrote:

New webrev available here:

webrev: 
http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.01/build/open/webrev/ 

incremental: 
http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.01/build.incr/open/webrev/ 


Looks good to me.


I believe I have addressed all the review comments apart from:

* GNM - Magnus to file a follow-up RFE

Created https://bugs.openjdk.java.net/browse/JDK-8244593.

/Magnus


* Zero - Waiting for somebody (Adrian?) to provide more information about what 
needs to be preserved

Cheers,
Mikael


On May 3, 2020, at 10:12 PM, Mikael Vidstedt  wrote:


Please review this change which implements part of JEP 381:

JBS: https://bugs.openjdk.java.net/browse/JDK-8244224
webrev: 
http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.00/build/open/webrev/
JEP: https://bugs.openjdk.java.net/browse/JDK-8241787


Note: When reviewing this, please be aware that this exercise was *extremely* 
mind-numbing, so I appreciate your help reviewing all the individual changes 
carefully. You may want to get that coffee cup filled up (or whatever keeps you 
awake)!


Background:

Because of the size of the total patch and wide range of areas touched, this 
patch is one out of in total six partial patches which together make up the 
necessary changes to remove the Solaris and SPARC ports. The other patches are 
being sent out for review to mailing lists appropriate for the respective areas 
the touch. An email will be sent to jdk-dev summarizing all the 
patches/reviews. To be clear: this patch is *not* in itself complete and 
stand-alone - all of the (six) patches are needed to form a complete patch. 
Some changes in this patch may look wrong or incomplete unless also looking at 
the corresponding changes in other areas.

For convenience, I’m including a link below[1] to the full webrev, but in case 
you have comments on changes in other areas, outside of the files included in 
this thread, please provide those comments directly in the thread on the 
appropriate mailing list for that area if possible.

In case it helps, the changes were effectively produced by searching for and 
updating any code mentioning “solaris", “sparc”, “solstudio”, “sunos”, etc. 
More information about the areas impacted can be found in the JEP itself.


Testing:

A slightly earlier version of this change successfully passed tier1-8, as well 
as client tier1-2. Additional testing will be done after the first round of 
reviews has been completed.

Cheers,
Mikael

[1] 
http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.00/all/open/webrev/

Re: Faster incremental OpenJDK compilation

[Jan Lahoda]

On 06. 05. 20 19:23, Jonathan Gibbons wrote:
You could refine the imports by filtering them according to whether they 
are 'star-imports' or the simple name matches a simple name that has 
been seen by the TreeScanner.  In other words, try and filter out 
imports that are definitely not relevant.


Good idea. I've tried to do that, and also fixed some bugs in the AST 
hashing, and enhanced tests.


All is still on "jlahoda-depend-in-module" branch in jdk/sandbox.

Jan



-- Jon


On 5/6/20 9:49 AM, Jan Lahoda wrote:

Hi Jon,

Good question. I was first experimenting with hashing Elements, as we 
do currently for the module API hashes, but it was too slow to enter 
the sources (although it is not completely impossible). So I did a new 
hashing based on the AST - basically, it is a TreeScanner sending Tree 
kinds, names and sub-trees into a MessageDigest. It ignores things 
like method bodies, class initializers, and private class members. 
Does not currently skip imports, although that may be seen as too 
conservative. But tweaks to the AST hashing are surely still needed, 
so we can tweak the exact meaning of a "significant change".


Jan

On 06. 05. 20 18:07, Jonathan Gibbons wrote:

Jan,

This seems like an interesting approach.

How are you determining "significant change"?  I could imagine trying 
to do this by looking at the changed lines, to see if they only 
within method bodies and comments (for example), or by doing some 
sort of lexical hash on the signatures, assuming that folk aren't 
messing with imports to change the resolution of simple type names in 
the signature.


-- Jon

On 5/6/20 8:30 AM, Jan Lahoda wrote:

Hi,

Triggered by Magnus' recent e-mail on adjusting the location of the 
IDE files, I looked at possibilities to improve speed of incremental 
compilation using make. About 3 years ago, we have sped up 
incremental build by only rebuilding modules when API of modules 
they depend on changed. But the module which contains modified 
sources is always compiled in full. So, for changes in java.base, 
this change improved the incremental build time significantly (by 
not recompiling e.g. java.desktop), but it can still take many 
seconds to build java.base after a trivial change. So, this time, I 
am thinking of speeding up module builds by not rebuilding all the 
source if possible.


What I am thinking of is a relatively simple approach: detect 
changed files in a module and check if their "interface" changed. If 
it did, recompile the whole module. If it didn't, only compile the 
modified files. As a consequence, a small change inside a method 
body should lead to a fast build. Changes outside of method bodies 
may trigger longer build, but my hope would be that these would be 
less common.


So far, I unfortunately don't know how to efficiently do this as 
nicely as the API digests used for inter-module dependencies. The 
approach that seems might work is this: the Depend plugin hooks 
itself into javac internals, and filters the incoming files - it 
first parses the modified ones, and if it cannot find a significant 
change, it will throw away the unmodified files, and only compile 
the modified ones. (In principle, it could also do dependency 
analysis, if we at some point decide it is important.)


For a simple "touch Object.java && make", the wall-clock time is 
less then 5s, which sounds interesting:

---
$ touch src/java.base/share/classes/java/lang/Object.java && time make
Building target 'default (exploded-image)' in configuration 
'linux-x86_64-server-release'

Compiling 3028 files for java.base
Stopping sjavac server
Finished building target 'default (exploded-image)' in configuration 
'linux-x86_64-server-release'


real    0m3,104s
user    0m5,731s
sys 0m1,086s
---

My current prototype is in the jdk/sandbox repository, branch 
"jlahoda-depend-in-module":
http://hg.openjdk.java.net/jdk/sandbox/shortlog/jlahoda-depend-in-module 



I wonder if this would sound interesting to developers working on 
base modules, like java.base.


What do you think? Any ideas/feedback?

Thanks,
 Jan

Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports (build system)

[Magnus Ihse Bursie]

On 2020-05-07 00:52, Mikael Vidstedt wrote:


Magnus, thank you so much for the thorough review!! Will send out a 
new webrev in a bit, meanwhile some comments inline..


On May 6, 2020, at 4:04 AM, Magnus Ihse Bursie 
> wrote:


Hi Mikael,

On 2020-05-04 07:12, Mikael Vidstedt wrote:

Please review this change which implements part of JEP 381:

JBS: https://bugs.openjdk.java.net/browse/JDK-8244224
webrev: 
http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.00/build/open/webrev/


It's a massive change. So you're getting a massive review from me. :-)

In the following comments, the line numbers refer to the old files. 
(I realize now that it might have been more convenient for you to 
have the new numbers. I'm sorry I did not think of it before.)


Not a problem!



---

First of all, I notice that you have not updated any copyright years. 
Do you plan to do that by a script before pushing?


Correct - whether I’ll end up using a script or not remains to be 
seen, but I’ll fix the copyright years when the reviews/changes have 
settled down a bit. I chose not to change all the copyright years 
since that would distract from the “actual” changes, and also pretty 
much guarantee conflicts when I pull in the latest changes.. It’s 
challenging enough as it is :)



---

Several variables are not needed anymore and can be removed. These 
include:


* TAR_CREATE_EXTRA_PARAM. Remove from Bundles.gmk, 
autoconf/basic_tools.m4 and autoconf/spec.gmk.in.


* ELFEDIT. Remove from autoconf/spec.gmk.in.

* STLPORT_LIB. Remove from autoconf/spec.gmk.in.

* SA_LDFLAGS. Remove from modules/jdk.hotspot.agent/Lib.gmk.

* GLOBAL_LIBS (as has been noted by Kim as well). Remove from 
common/NativeCompilation.gmk, autoconf/spec.gmk.in and 
autoconf/libraries.m4.


* LDFLAGS_WARNINGS_ARE_ERRORS was (unfortunately!) only supported on 
solstudio. No reason to keep it anymore. Remove from 
common/NativeCompilation.gmk, autoconf/flags-ldflags.m4 and 
autoconf/spec.gmk.in.


Fixed all of the above.

* GNM is not needed anymore, but there is certainly a need for 
related cleanup (we do not need the if statement, but could use 
"UTIL_CHECK_TOOLS(NM, nm gcc-nm)" for all platforms). I understand if 
you do not want to mess around with it, let me know and I'll file a 
separate follow-up bug.


Would definitely appreciate if you could file a follow-up, thank you!

Ok, will do.



---

In some places we used to have a variable that was passed into 
Setup*Compilation, but the variable was removed. However, the 
argument to the function was left in place. (Make do not warn for 
unknown variables, unfortunately.)


lib/CompileJvm.gmk:    EXTRA_OBJECT_FILES := 
$(DTRACE_EXTRA_OBJECT_FILES), \


Awt2dLibraries.gmk:    ASFLAGS := $(LIBAWT_ASFLAGS), \


Fixed.

Also, the documentation of SetupNativeCompilation has not been 
updated to match the implementation. Please remove the the "REORDER" 
line there as well.


I’ve removed REORDER completely now. For completeness: In webrev.00 I 
(only) removed C_FLAG_REORDER, and didn’t touch the REORDER argument 
to SetupNativeCompilation. As you noted though, the REORDER 
functionality is not actually used anywhere - that is true even in 
mainline today. Turns out the last uses of the REORDER functionality 
disappeared with https://bugs.openjdk.java.net/browse/JDK-8200178 / 
http://hg.openjdk.java.net/jdk/jdk/rev/396ea30afbd5 authored by … 
would you look at that - “ihse”! Wonder who that is? ;)
Ah, I see. I missed the fact that you removed C_FLAG_REORDER and not 
REORDER. Thank you for cleaning up after me anyway. :-)



---

For dtrace, I want to make sure that the removal is complete (cleanse 
it with fire, please! :-)).


The build tool generateJvmOffsets is not used anymore, so 
generateJvmOffsets.cpp can be removed (actually, that means the 
entire hotspot/src/native/).


I assume you remove src/java.base/solaris/native/libjvm_dtrace and 
libjvm_db in the core-libs patch, right?


I also want to make sure that you actually remove hotspot_jni.d, 
hotspot.d and hs_private.d from src/hotspot/os/posix/dtrace, and 
src/hotspot/os/solaris/dtrace/jhelper.d. (Done in the hotspot patch, 
I hope.)


I will not remove all traces of dtrace (no pun intended) as part of 
this JEP, but (separate from this JEP) we should certainly discuss and 
decide how to move forward with dtrace on the other platforms. The 
stuff under src/java.base/solaris is indeed removed as part of the 
core-libs patch, and the src/hotspot/os/solaris stuff is removed as 
part of the hotspot patch, but AFAICT the src/hotspot/os/posix/dtrace 
files still do get picked up by the logic in GensrcDtrace.gmk so I’m 
going to leave those files untouched. Let me know if I’m missing 
something.
Ok, I probably came across as too aggressive towards dtrace here. I do 
not mean that I want all of dtrace removed. The parts that are 
cross-platform are quite sane, at least from a build perspective. It's 
all the