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-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 )
