RFR(XS):8244248: boot-jdk.m4 captures the version line using regex
Magnus Ihse Bursie
magnus.ihse.bursie at oracle.com
Tue May 5 09:22:03 UTC 2020
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 <magnus.ihse.bursie at oracle.com>
> Date: Monday, May 4, 2020 at 2:58 AM
> To: "Liu, Xin" <xxinliu at amazon.com>, Erik Joelsson <erik.joelsson at oracle.com>, "build-dev at openjdk.java.net" <build-dev at 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.joelsson at 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.joelsson at 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. Something like this perhaps:
> >
> > grep "version \"[0-9\._]*\""
> >
> > I tried that expression manually on Mac, Linux and Solaris so should be
> > portable enough.
> >
> > /Erik
> >
> > On 2020-04-30 16:48, Liu, Xin wrote:
> > > Hi, Andrew,
> > >
> > > How about this? I can use awk to capture java -version. There're 2 cases.
> > >
> > > I) openjdk
> > > openjdk version "14.0.1" 2020-04-14
> > > 2) oraclejdk
> > > java 14.0.1 2020-04-14
> > >
> > > if somehow java displays some error/warning messages, awk can filter them out and capture the version line.
> > > Eg.
> > > $ ~/builds/jdk-14.0.1+7/bin/java -version
> > > [0.009s][error][cds] Unable to map CDS archive -- os::vm_allocation_granularity() expected: 65536 actual: 4096
> > > openjdk version "14.0.1" 2020-04-14
> > > OpenJDK Runtime Environment AdoptOpenJDK (build 14.0.1+7)
> > > OpenJDK 64-Bit Server VM AdoptOpenJDK (build 14.0.1+7, mixed mode)
> > > $ ~/builds/jdk-14.0.1+7/bin/java -version 2>&1 | awk '/^(openjdk version|java)/ {print $0}'
> > > openjdk version "14.0.1" 2020-04-14
> > >
> > > I think this awk stmt is portable, but it's always good to ask experts to review it, so I cc build-dev.
> > >
> > > Hers is the change.
> > >
> > > diff --git a/make/autoconf/boot-jdk.m4 b/make/autoconf/boot-jdk.m4
> > > --- a/make/autoconf/boot-jdk.m4
> > > +++ b/make/autoconf/boot-jdk.m4
> > > @@ -74,7 +74,7 @@
> > > BOOT_JDK_FOUND=no
> > > else
> > > # Oh, this is looking good! We probably have found a proper JDK. Is it the correct version?
> > > - BOOT_JDK_VERSION=`"$BOOT_JDK/bin/java$EXE_SUFFIX" $USER_BOOT_JDK_OPTIONS -version 2>&1 | $HEAD -n 1`
> > > + BOOT_JDK_VERSION=`"$BOOT_JDK/bin/java$EXE_SUFFIX" $USER_BOOT_JDK_OPTIONS -version 2>&1 | $AWK '/^(openjdk version|java)/ {print [$]0}'`
> > > if [ [[ "$BOOT_JDK_VERSION" =~ "Picked up" ]] ]; then
> > > AC_MSG_NOTICE([You have _JAVA_OPTIONS or JAVA_TOOL_OPTIONS set. This can mess up the build. Please use --with-boot-jdk-jvmargs instead.])
> > > AC_MSG_NOTICE([Java reports: "$BOOT_JDK_VERSION".])
> > > @@ -529,7 +529,7 @@
> > > BUILD_JDK_FOUND=no
> > > else
> > > # Oh, this is looking good! We probably have found a proper JDK. Is it the correct version?
> > > - BUILD_JDK_VERSION=`"$BUILD_JDK/bin/java" -version 2>&1 | $HEAD -n 1`
> > > + BUILD_JDK_VERSION=`"$BUILD_JDK/bin/java" -version 2>&1 | $AWK '/^(openjdk version|java)/ {print [$]0}'`
> > >
> > > # Extra M4 quote needed to protect [] in grep expression.
> > > [FOUND_CORRECT_VERSION=`echo $BUILD_JDK_VERSION | $EGREP "\"$VERSION_FEATURE([\.+-].*)?\""`]
> > >
> > >
> > >
> > > On 4/30/20, 2:52 PM, "aarch64-port-dev on behalf of Liu, Xin" mailto:aarch64-port-dev-bounces at openjdk.java.netonbehalfofxxinliu@amazon.com wrote:
> > >
> > > Hi, Andrew,
> > >
> > > That's a hack. A general way should use grep or sed to capture the needed line instead of hardcoding first or second line.
> > > Okay, Let me try to do that.
> > >
> > > Thanks,
> > > --lx
> > >
> > >
> > > On 4/30/20, 1:19 AM, mailto:aph at redhat.com mailto:aph at redhat.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 4/30/20 12:43 AM, Liu, Xin wrote:
> > > > One trick here. It's very easy to cheat configure by hacking the boot-jdk.m4 to "$HEAD -n 2". Everything looks fine then.
> > >
> > > The fix should be submitted to build-dev.
> > >
> > > --
> > > Andrew Haley (he/him)
> > > Java Platform Lead Engineer
> > > Red Hat UK Ltd. https://www.redhat.com
> > > https://keybase.io/andrewhaley
> > > EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
> > >
> > >
> > >
> >
>
>
>
More information about the build-dev
mailing list