RFR(XS): 8215966: GeneratePropertyPassword.sh uses bash syntax (was Re: [PATCH] JDK-8025886: Replace [[ and == bash extensions in tests)

[Hohensee, Paul]

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.

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.

Lgtm, so that’s one review. May we have another please?

Thanks,

Paul

From: serviceability-dev <serviceability-dev-bounces at openjdk.java.net> on behalf of Sergei Ustimenko <merkel05 at gmail.com>
Date: Friday, December 28, 2018 at 8:40 AM
To: serviceability-dev <serviceability-dev at 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 <merkel05 at gmail.com<mailto:merkel05 at 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 <merkel05 at gmail.com<mailto:merkel05 at 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 <martinrb at google.com<mailto:martinrb at 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 <merkel05 at gmail.com<mailto:merkel05 at 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 <martinrb at google.com<mailto:martinrb at 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 <merkel05 at gmail.com<mailto:merkel05 at 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 <martinrb at google.com<mailto:martinrb at 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
)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20181228/343883ed/attachment-0001.html>