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