Hi Paul,
On 29/12/2018 4:05 am, Hohensee, Paul wrote:
I’ll sponsor it. I’ve filed JDK-8215966
<https://bugs.openjdk.java.net/browse/JDK-8215966> for it: it’s a simple
forward port.
To be clear, it isn't that this change didn't make it into post 9, the
change was reverted by JDK-8192953 in June 2018.
Bug: https://bugs.openjdk.java.net/browse/JDK-8215966
Webrev: http://cr.openjdk.java.net/~phh/8215966/webrev.00/
While I agree with Martin that we might as well specify bash, imo in
this specific case it’s better to do a forward port since the code
already exists in JDK9. The webrev therefore dups the JDK-8025886
<https://bugs.openjdk.java.net/browse/JDK-8025886> patch. If we want to
make scripts explicitly depend on bash, I’d prefer to see a separate RFE
for it.
The patch looks good to me. Reviewed.
Lgtm, so that’s one review. May we have another please?
Point of order: the change proposed is not the change that Sergei
requested a review of. This is new code proposed by you, so you can't
review your own contribution.
But I've Reviewed it, and Sergei is okay with it too so that's a
"r"eview and so it is good to go.
Thanks,
David
Thanks,
Paul
*From: *serviceability-dev <serviceability-dev-boun...@openjdk.java.net>
on behalf of Sergei Ustimenko <merke...@gmail.com>
*Date: *Friday, December 28, 2018 at 8:40 AM
*To: *serviceability-dev <serviceability-dev@openjdk.java.net>
*Subject: *Re: [PATCH] JDK-8025886: Replace [[ and == bash extensions in
tests
Hi,
Could anyone please help with review?
Regards,
Sergei
On Sat, 22 Dec 2018 at 17:09, Sergei Ustimenko <merke...@gmail.com
<mailto:merke...@gmail.com>> wrote:
Hi,
Could anyone please review and sponsor
this small patch to add a shebang line to
the test script?
diff --git
a/test/jdk/sun/management/jmxremote/bootstrap/GeneratePropertyPassword.sh
b/test/jdk/sun/management/jmxremote/bootstrap/GeneratePropertyPassword.sh
---
a/test/jdk/sun/management/jmxremote/bootstrap/GeneratePropertyPassword.sh
+++
b/test/jdk/sun/management/jmxremote/bootstrap/GeneratePropertyPassword.sh
@@ -1,3 +1,5 @@
+#!/bin/bash
+
#
# Copyright (c) 2003, 2018, Oracle and/or its affiliates. All
rights reserved.
# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
Thanks,
Sergei
On Wed, Dec 19, 2018, 21:04 Sergei Ustimenko <merke...@gmail.com
<mailto:merke...@gmail.com> wrote:
Quickfix on my previous mail: I've found no scripts
with the same problem, except this one in the patch.
>Yeah, let's fix !
Perfect!
I will just need some help with sponsorship to push this.
I've signed the OCA, so no problems from my side.
Regards,
Sergei
On Wed, 19 Dec 2018 at 20:22, Martin Buchholz
<marti...@google.com <mailto:marti...@google.com>> wrote:
Did we really have shell scripts without a shebang line?
Yeah, let's fix !
On Wed, Dec 19, 2018 at 11:03 AM Sergei Ustimenko
<merke...@gmail.com <mailto:merke...@gmail.com>> wrote:
HI Martin,
As you've suggested I've simply added bash's shebang.
It wouldn't add any problem since, as David have mentioned,
no bash - no build. I've also quickly checked for
similar cases
and found one.
An updated patch is below.
diff --git
a/test/jdk/sun/management/jmxremote/bootstrap/GeneratePropertyPassword.sh
b/test/jdk/sun/management/jmxremote/bootstrap/GeneratePropertyPassword.sh
---
a/test/jdk/sun/management/jmxremote/bootstrap/GeneratePropertyPassword.sh
+++
b/test/jdk/sun/management/jmxremote/bootstrap/GeneratePropertyPassword.sh
@@ -1,3 +1,5 @@
+#!/bin/bash
+
#
# Copyright (c) 2003, 2018, Oracle and/or its
affiliates. All rights reserved.
# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS
FILE HEADER.
Regards,
Sergei
On Mon, 10 Dec 2018 at 22:27, Martin Buchholz
<marti...@google.com <mailto:marti...@google.com>> wrote:
I don't know if there's an official policy on how
ultra-portable tests are supposed to be. In
practice, you probably won't be able to build
openjdk on a system without bash.
On Mon, Dec 10, 2018 at 1:12 PM Sergei Ustimenko
<merke...@gmail.com <mailto:merke...@gmail.com>> wrote:
Hi Martin,
That sounds good!
I've counted all the sh-shebangs and it appears that
there are at least 66 of them inside the test/
directory,
where only 12 bashes.
I've also ran the search in order to identify
all the
occurrences that use either [[ or == and found only
three of them that use "==". That one for example:
http://hg.openjdk.java.net/jdk/sandbox/file/f94276ccc9fc/test/hotspot/jtreg/vmTestbase/jit/tiered/tieredTest.sh#l63
of course `dash` reports failure in that case.
So I'm quite hesitant in that case and not
really sure
what to do. I haven't also found any existent
JBS ticket
for such /bin/sh => /bin/bash a replacement.
So any advise in this case would be appreciated!
Regards,
Sergei
On Mon, 10 Dec 2018 at 18:32, Martin Buchholz
<marti...@google.com
<mailto:marti...@google.com>> wrote:
I would not try to remove all bash-isms from
openjdk. Instead I would find instances of
/bin/sh that need to be changed to /bin/bash.
(Ubuntu's use of /bin/sh -> /bin/dash is
technically correct, but caused much suffering
https://bugs.launchpad.net/ubuntu/+source/dash/+bug/61463
)
