Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports (build system)
> On May 4, 2020, at 12:44 AM, John Paul Adrian Glaubitz > wrote: > > On 5/4/20 9:39 AM, John Paul Adrian Glaubitz wrote: >> I haven't looked at the changes yet, but please make sure you don't kill the >> possibility to build Zero on SPARC on Linux because that should still work >> without many extra ado. > This change should be dropped from make/autoconf/platform.m4: > > @@ -148,22 +148,10 @@ > VAR_CPU=sh > VAR_CPU_ARCH=sh > 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]) > ;; > esac > ]) Is that the only part that needs to be preserved for linux/sparc+zero? Nothing needed in the rest of the make files, source code, tests, etc.? > And wouldn't it make sense to keep this change for the future for the > future removal of other architectures? > > @@ -564,29 +524,10 @@ > PLATFORM_EXTRACT_TARGET_AND_BUILD > PLATFORM_SETUP_TARGET_CPU_BITS > PLATFORM_SET_MODULE_TARGET_OS_VALUES > PLATFORM_SET_RELEASE_FILE_OS_VALUES > PLATFORM_SETUP_LEGACY_VARS > - PLATFORM_CHECK_DEPRECATION > -]) > - > -AC_DEFUN([PLATFORM_CHECK_DEPRECATION], > -[ > - UTIL_ARG_ENABLE(NAME: deprecated-ports, DEFAULT: false, > - RESULT: ENABLE_DEPRECATED_PORTS, > - DESC: [suppress the error when configuring for a deprecated port]) > - > - if test "x$OPENJDK_TARGET_OS" = xsolaris || \ > - (test "x$OPENJDK_TARGET_CPU_ARCH" = xsparc && \ > - test "x$with_jvm_variants" != xzero); then > -if test "x$ENABLE_DEPRECATED_PORTS" = "xtrue"; then > - AC_MSG_WARN([The Solaris and SPARC ports are deprecated and may be > removed in a future release.]) > -else > - AC_MSG_ERROR(m4_normalize([The Solaris and SPARC ports are deprecated > and may be removed in a > -future release. Use --enable-deprecated-ports=yes to suppress this > error.])) > -fi > - fi > ]) > > AC_DEFUN_ONCE([PLATFORM_SETUP_OPENJDK_BUILD_OS_VERSION], > [ > > ### What are you planning on removing? ;) Jokes aside - that logic is not complex enough to motivate having around just in case. If we need to reintroduce it we can always go back and get some inspiration from the VCS history. Cheers, Mikael > > 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: 8243656: Shell built-in test in configure depends on help
On Tue, 2020-05-05 at 08:24 -0700, Erik Joelsson wrote: > Looks good to me. Thanks for the review, Erik! Cheers, Severin > /Erik > > On 2020-05-05 08:15, Severin Gehwolf wrote: > > Hi, > > > > Could I please get a review of this trivial change? Apparently using > > the help builtin for determining whether or not a builtin is available > > is not a good idea. A more portable way to do this is to use "command > > -v" or "type". Thanks to Michael Zucchi for contributing this fix. > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8243656 > > webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8243656/02/webrev/ > > > > Testing: Local builds. jdk/submit. > > > > Thoughts? > > > > Thanks, > > Severin > >
Re: RFR: 8243656: Shell built-in test in configure depends on help
Looks good to me. /Erik On 2020-05-05 08:15, Severin Gehwolf wrote: Hi, Could I please get a review of this trivial change? Apparently using the help builtin for determining whether or not a builtin is available is not a good idea. A more portable way to do this is to use "command -v" or "type". Thanks to Michael Zucchi for contributing this fix. Bug: https://bugs.openjdk.java.net/browse/JDK-8243656 webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8243656/02/webrev/ Testing: Local builds. jdk/submit. Thoughts? Thanks, Severin
RFR: 8243656: Shell built-in test in configure depends on help
Hi, Could I please get a review of this trivial change? Apparently using the help builtin for determining whether or not a builtin is available is not a good idea. A more portable way to do this is to use "command -v" or "type". Thanks to Michael Zucchi for contributing this fix. Bug: https://bugs.openjdk.java.net/browse/JDK-8243656 webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8243656/02/webrev/ Testing: Local builds. jdk/submit. Thoughts? Thanks, Severin
Re: RFR(XS):8244248: boot-jdk.m4 captures the version line using regex
On 2020-05-04 20:57, Liu, Xin wrote:
Hi, Magnus and Erik,
Thank you to look into it. Here is a new revision.
https://cr.openjdk.java.net/~xliu/8244248/02/webrev/
Looks good to me!
/Magnus
When I enclosed the whole statement with [], autoconf/m4 told me that it's
syntax error for [$] 0.
So, I added an extra whitespace to avoid m4 substitution.
[ BUILD_JDK_VERSION=`"$BUILD_JDK/bin/java" -version 2>&1 | $AWK '/version
\"[0-9\._\-a-zA-Z]+\"/{print $ 0; exit;}'` ]
I took it from here. I think it remains the maximal readability.
https://lists.gnu.org/archive/html/autoconf/2005-07/msg00088.html
Could you sponsor that if it works?
Thanks,
--lx
From: Magnus Ihse Bursie
Date: Monday, May 4, 2020 at 2:58 AM
To: "Liu, Xin" , Erik Joelsson ,
"build-dev@openjdk.java.net"
Subject: RE: [EXTERNAL] RFR(XS):8244248: boot-jdk.m4 captures the version line
using regex
CAUTION: This email originated from outside of the organization. Do not click
links or open attachments unless you can confirm the sender and know the
content is safe.
On 2020-05-02 01:57, Liu, Xin wrote:
HI, Eric,
Thanks for the feedback. I use [] to wrap the regex for readability. Here is
a new webrev:
https://cr.openjdk.java.net/~xliu/8244248/01/webrev/make/autoconf/boot-jdk.m4.udiff.html
This looks better, but I'd prefer it if you add a comment about this, like this:
# Additional [] needed to keep m4 from mangling shell constructs.
and encapsulate the entire row in the additional [ ], like this:
[ BUILD_JDK_VERSION=`"$BUILD_JDK/bin/java" -version 2>&1 | $AWK '/version
\"[0-9\._\-a-zA-Z]+\"/{print [$]0; exit;}'` ]
/Magnus
thanks,
--lx
On 5/1/20, 2:15 PM, "Erik Joelsson" mailto:erik.joels...@oracle.com wrote:
CAUTION: This email originated from outside of the organization. Do not
click links or open attachments unless you can confirm the sender and know the
content is safe.
On 2020-05-01 13:44, Liu, Xin wrote:
> Hello, Erik,
>
> Thank you for your comments. I made some change. Could you review it
again?
> JBS: https://bugs.openjdk.java.net/browse/JDK-8244248
> Webrev: https://cr.openjdk.java.net/~xliu/8244248/00/webrev/
This looks good.
> Some new comments:
> 1) I don't know that there're subtle difference between '-version' and
'--version'. This patch only captures '-version'.
> Thanks to point out that LAUNCH_NAME is configurable. I think your regex
is better.
>
> 2) From my personal experience, I feel awk regex is more portable than
grep.
> The reason that '[' and ']' look so strange because it's m4 file. I
can't find an effective way to escape them but quadrigrpahs.
>
https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Quadrigraphs.html#Quadrigraphs
You can escape with more [] as long as they are balanced, which they are
here. I think it's generally more readable if you enclose a whole
expression, like the whole awk script, inside [] rather than just
doubling them up inside the regexp. You can see examples of how we use
this in jdk-version.m4.
> Testing:
> I use the awk around different jdks.
> i) oraclejdk
> ~ /Library/Java/JavaVirtualMachines/jdk-14.0.1.jdk/Contents/Home/bin/java -version
2>&1 | awk '/version \"[0-9\._\-a-zA-Z]+\"/{print $0;exit;}'
> java version "14.0.1" 2020-04-14
> i) openjdk
> ./build/linux-x86_64-server-fastdebug/jdk/bin/java -version 2>&1 | awk '/version
\"[0-9\._\-a-zA-Z]+\"/{print $0;exit;}'
> openjdk version "15-internal" 2020-09-15
> i) zulu
> docker run -it --rm azul/zulu-openjdk:latest java -version |& awk '/version
\"[0-9\._\-a-zA-Z]+\"/{print $0;exit;}'
> openjdk version "1.8.0_242"
I tried this and it seems to work for me too.
/Erik
> thanks,
> --lx
>
>
> On 5/1/20, 6:06 AM, "Erik Joelsson" mailto:erik.joels...@oracle.com
wrote:
>
> CAUTION: This email originated from outside of the organization. Do
not click links or open attachments unless you can confirm the sender and know the
content is safe.
>
>
>
> Hello,
>
> My OracleJDK 14 displays:
>
> java version "14" 2020-03-17
>
> Not sure where you found a version output without the word
"version" in it.
>
> From what I understand, any distributor is free to change the
"openjdk"
> prefix of this line, so relying on there only being 2 cases is not a
> good idea. If we assume the boot jdk must be an OpenJDK derivative,
then
> a regular expression that tries to capture the line in more detail
would
> be preferred. The aspects I would try to capture would be the word
> version and a bunch of numbers and dots (and possibly underscore if
we
> want to keep it compatible with even older java versions for easier
> backports) inside double quotes.
Re: RFR: JDK-8244247: Build failures after sjavac cleanup
On 2020-05-04 23:38, Erik Joelsson wrote: On 2020-05-04 02:51, Magnus Ihse Bursie wrote: On 2020-05-01 19:33, Erik Joelsson wrote: After the sjavac cleanup in JDK-8244036 (and the subsequent fix of the javac server usage in JDK-8244210), two more build failures have been noted. The bootcycle build fails and the test-make target fails the tests of SetupJavaCompilation. The bootcycle build fails because a space has crept into the variable SJAVAC_SERVER_DIR when it's overridden in bootcycle-spec.gmk.in This space splits the sjavac server argument. The tests for make just need to be updated to use the new TARGET_RELEASE arg instead of the old SetupJavaCompiler construct. Bug: https://bugs.openjdk.java.net/browse/JDK-8244247 Webrev: http://cr.openjdk.java.net/~erikj/8244247/webrev.01/index.html The change looks good. However, it makes me realize that the variable should really have been renamed JAVAC_SERVER_DIR. Do you think you could fix that in the same bug? Otherwise I'll do it separately. Updated webrev: http://cr.openjdk.java.net/~erikj/8244247/webrev.02/ Looks good to me. Thanks for fixing! I also just wanted to note that technically, the assignment in bootcycle-spec.gmk.in has not changed -- the extra space has always been there. But due to how it was processed, that additional space is no longer stripped when the variable is read. It was not explicitly stripped before either, but it went a second round through the makefile parsing by being used in SetupJavaCompiler as "SERVER_DIR := $(SJAVAC_SERVER_DIR)", which effectively stripped it. Interesting, I didn't bother looking at the history, just looked for the fix. I understand. It was probably not that important, I was just perplexed at how this could have happened and wanted to shed some light on it. /Magnus /Erik /Magnus /Erik
