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

[Steffen Kothe <steffen.kothe@skothe.de>]

Am Thu, Nov 10, 2022 at 11:32:12AM -0800 schrieb Saravana Kannan:
> On Thu, Nov 10, 2022 at 9:48 AM Steffen Kothe <steffen.kothe@skothe.de> 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@skothe.de> 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.

What about a re-write of the script in POSIX compliant style? This
should work then in every shell derivate without weird checks.

This would remove for example function declarations, arrays and so on,
targeting to the same printable output but with different logic in the
background.

> -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@skothe.de>
> > > > ---
> > > >  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@google.com>
> > > > --
> > > > 2.30.2
> > > >
> > > >
> >
> > --
> > Cheers,
> >         Steffen
> >

--
Cheers,
	Steffen