Hi, I've checked if there are scripts (*.sh) that miss shebang and found some (~170 in test/). As an example those:
... test/jdk/java/lang/instrument/StressGetObjectSizeTest.sh test/jdk/java/lang/instrument/RedefineClassWithNativeMethod.sh test/jdk/java/lang/instrument/RedefineMethodWithAnnotations.sh test/jdk/java/io/File/GetXSpace.sh test/jdk/java/rmi/activation/Activatable/extLoadedImpl/ext.sh test/jdk/java/util/Formatter/Basic.sh test/jdk/java/util/zip/3GBZipFiles.sh test/jdk/tools/launcher/6842838/Test6842838.sh test/jdk/tools/jjs/common.sh ... test/jdk/sun/rmi/rmic/covariantReturns/run.sh test/jdk/sun/rmi/rmic/manifestClassPath/Util.sh test/jdk/sun/rmi/rmic/oldjavacRemoved/sunToolsJavacMain.sh test/jdk/sun/management/jmxremote/RunTest.sh test/jdk/sun/management/jmxremote/bootstrap/RmiBootstrapTest.sh test/jdk/sun/management/jmxremote/bootstrap/RmiSslBootstrapTest.sh test/jdk/sun/management/jmxremote/bootstrap/RmiSslNoKeyStoreTest.sh test/jdk/sun/management/jmxremote/bootstrap/GeneratePropertyPassword.sh test/jdk/sun/net/sdp/sanity.sh test/jdk/sun/jvmstat/testlibrary/utils.sh ... Some of them are tests others are utility scripts. I wonder what we can do in that case? Can we fill an RFE? Relying on default here might lead to a nasty situations in future, as it was with Ubuntu sh -> dash. As a small remark all the scripts that miss shebang are located in the test/ dir. Regards, Sergei On Fri, 28 Dec 2018 at 20:01, Hohensee, Paul <hohen...@amazon.com> wrote: > Send a patch directly to me (hohen...@amazon.com) and I’ll file an RFE > and post a webrev so you can ask for a review. > > > > Thanks, > > > > Paul > > > > *From: *Sergei Ustimenko <merke...@gmail.com> > *Date: *Friday, December 28, 2018 at 10:55 AM > *To: *"Hohensee, Paul" <hohen...@amazon.com> > *Cc: *serviceability-dev <serviceability-dev@openjdk.java.net>, Martin > Buchholz <marti...@google.com> > *Subject: *Re: RFR(XS): 8215966: GeneratePropertyPassword.sh uses bash > syntax (was Re: [PATCH] JDK-8025886: Replace [[ and == bash extensions in > tests) > > > > Hi Paul, > > > > Thanks for taking care of this and for your time! > > > > Agree, initial patch was mainly about forward porting > > lost changes as you've just mentioned. > > > > Regarding filling another RFE as I don't have a JBS account it > > would be a bit of a trouble for me I guess. I could try my best at > > https://bugreport.java.com/bugreport/ though I guess that's > > not the right place for it. So if I may borrow a bit more of your > > time, could you please help me with filling the RFE? I guess missing > > shebang is not critical, but might lead to a nasty situation. > > > > Thanks, > > Sergei > > > > On Fri, 28 Dec 2018 at 19:05, Hohensee, Paul <hohen...@amazon.com> 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. > > > > 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> 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 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> 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> > 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> 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> > 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> 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 > > ) > >
