Re: [PATCH] scripts: dev-needs.sh: Enforce bash usage

[Saravana Kannan]

On Thu, Nov 10, 2022 at 9:48 AM Steffen Kothe <steffen.kothe@xxxxxxxxx> wrote:
>
> Am Wed, Nov 09, 2022 at 02:58:15PM -0800 schrieb Saravana Kannan:
> > On Wed, Nov 9, 2022 at 9:04 AM Steffen Kothe <steffen.kothe@xxxxxxxxx> wrote:
> > >
> > > Calling the script from a system which does not invoke bash
> > > by default causes a return with a syntax error like:
> > >
> > >         ./dev-needs.sh: 6: Syntax error: "(" unexpected
> > >
> > > /bin/sh invokes on most distributions a symbolic link to a
> > > default shell like dash (Debian) or bash (Ubuntu).
> > >
> > > Since the script depends on bash syntax, enforce the same by
> > > default to prevent syntax errors caused by wrong shell type usage.
> >
> > I wrote this so that it can run on an Android target that runs toybox.
> > Sadly toybox doesn't like have /bin/bash. This will break my use case.
> > So I'll have to Nak this.
>
> Ok, I see.
>
> > I'm open to other ideas though as I'd like to this to work in as many
> > cases as possible. Should we just add a wrapper that has /bin/bash and
> > then sources this file?
>
> I mean, we could leave at least a warning on top above the first
> function via a simple echo.
>
> Sth. like:
>
>         echo "Warning: $0 is only tested for following shell variants
>               [toybox, bash]. Other shells might be not following the
>               specific syntax of this script."
>
>         echo "Shell is: $(readlink /bin/sh)"
>
> I would prevent to add another wrapper to it, because its fairly easy
> to cp this file to a target. An embedded system with a pure POSIX compliant
> shell will never be compatible to this kind of syntax.
>
> What do you think about a more describing approach of the error instead
> of handling it with some weird logic and wrapping?

Are you suggesting we check for the shell being toybox/bash and then
printing this? Always printing it isn't nice because it'll mess up all
the script that expect the output to be just what it is today.

But if you want to add an error check, I'm okay with that.

-Saravana

>
> Cheers,
>         skothe
>
> > Also looks like multiple #! lines aren't supported by bash, so we
> > can't add multiple lines either.
> >
> > -Saravana
> >
> > >
> > > Signed-off-by: Steffen Kothe <steffen.kothe@xxxxxxxxx>
> > > ---
> > >  scripts/dev-needs.sh | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/scripts/dev-needs.sh b/scripts/dev-needs.sh
> > > index 454cc304fb448..46537859727bc 100755
> > > --- a/scripts/dev-needs.sh
> > > +++ b/scripts/dev-needs.sh
> > > @@ -1,4 +1,4 @@
> > > -#! /bin/sh
> > > +#! /bin/bash
> > >  # SPDX-License-Identifier: GPL-2.0
> > >  # Copyright (c) 2020, Google LLC. All rights reserved.
> > >  # Author: Saravana Kannan <saravanak@xxxxxxxxxx>
> > > --
> > > 2.30.2
> > >
> > >
>
> --
> Cheers,
>         Steffen
>