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" <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" <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. 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"
<aarch64-port-dev-boun...@openjdk.java.net on behalf of xxin...@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, "a...@redhat.com" <a...@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
> >
> >
> >
>
