Re: [PATCH] Remove bashisms from spectrum shell scripts

[Henri Rosten <henri.rosten@unikie.com>]

On Tue, Nov 08, 2022 at 01:27:27PM +0000, Alyssa Ross wrote:
> On Tue, Nov 08, 2022 at 12:05:44PM +0200, Henri Rosten wrote:
> > This commit removes bashisms from spectrum shell scripts. This change is
> > needed to be able to use the scripts from POSIX-compliant shells which
> > are not bash compatible - such as dash.
> >
> > Signed-off-by: Henri Rosten <henri.rosten@unikie.com>
> > ---
> >  Documentation/scripts/undocumented-uuids.sh |  6 +++++-
> >  scripts/format-uuid.sh                      | 15 ++++++++++++++-
> >  scripts/make-gpt.sh                         |  7 +++----
> >  3 files changed, 22 insertions(+), 6 deletions(-)
> >
> 
> Thanks!  I definitely prefer to stick to portable shell, awk, make etc.
> functionality, so I'm happy to receive this patch.  I'll look into
> integrating shellcheck into the build to ensure the bashisms don't creep
> back in.
> 
> I've left some comments below.
> 
> > diff --git a/Documentation/scripts/undocumented-uuids.sh b/Documentation/scripts/undocumented-uuids.sh
> > index 34c2d22..0434407 100755
> > --- a/Documentation/scripts/undocumented-uuids.sh
> > +++ b/Documentation/scripts/undocumented-uuids.sh
> > @@ -1,5 +1,6 @@
> >  #!/bin/sh -eu
> >  # SPDX-FileCopyrightText: 2022 Alyssa Ross <hi@alyssa.is>
> > +# SPDX-FileCopyrightText: 2022 Unikie
> >  # SPDX-License-Identifier: EUPL-1.2+
> >
> >  cd "$(dirname "$0")/../.."
> > @@ -7,8 +8,11 @@ cd "$(dirname "$0")/../.."
> >  PATTERN='\b[A-F0-9]{8}-[A-F0-9]{4}-[A-F0-9]{4}-[A-F0-9]{4}-[A-F0-9]{12}\b'
> >  UUID_REFERENCE_PATH=Documentation/uuid-reference.adoc
> >
> > +tmp=$(mktemp)
> 
> Let's clean this up with a trap, so that it gets cleaned up even if
> something fails:
> 
>     trap "rm -f $tmp" EXIT
> 
> And since we have to give it a name, maybe we could use the opportunity
> to make it descriptive?  "documented_uuids", maybe?
> 

I'll fix this in a follow-up version.

> > +grep -Eio "$PATTERN" "$UUID_REFERENCE_PATH" | sort -u >$tmp
> >  git ls-files -coz --exclude-standard |
> >      grep -Fxvz "$UUID_REFERENCE_PATH" |
> >      xargs -0 git grep -Ehio --no-index --no-line-number "$PATTERN" -- |
> >      sort -u |
> > -    comm -23 - <(grep -Eio "$PATTERN" "$UUID_REFERENCE_PATH" | sort -u)
> > +    comm -23 - $tmp
> > +rm -f $tmp
> > diff --git a/scripts/format-uuid.sh b/scripts/format-uuid.sh
> > index fa07eb9..c7b94da 100755
> > --- a/scripts/format-uuid.sh
> > +++ b/scripts/format-uuid.sh
> > @@ -1,6 +1,19 @@
> >  #!/bin/sh -eu
> >  #
> >  # SPDX-FileCopyrightText: 2021-2022 Alyssa Ross <hi@alyssa.is>
> > +# SPDX-FileCopyrightText: 2022 Unikie
> >  # SPDX-License-Identifier: EUPL-1.2+
> >
> > -printf "%s\n" "${1:0:8}-${1:8:4}-${1:12:4}-${1:16:4}-${1:20}"
> > +substr () {
> > +    str=$1
> > +    beg=$2
> > +    end=$3
> > +    echo $(echo $str | cut -c $beg-$end)
> 
> This should work fine if we drop the echo $(...), right?
> 

True, the outer echo is not needed. I'll fix this too in a follow-up 
version.

> > +}
> > +
> > +u1=$(substr $1 1 8)
> > +u2=$(substr $1 9 12)
> > +u3=$(substr $1 13 16)
> > +u4=$(substr $1 17 20)
> > +u5=$(substr $1 21 32)
> > +printf "%s\n" "$u1-$u2-$u3-$u4-$u5"
> > diff --git a/scripts/make-gpt.sh b/scripts/make-gpt.sh
> > index 56076d3..351aa69 100755
> > --- a/scripts/make-gpt.sh
> > +++ b/scripts/make-gpt.sh
> > @@ -1,6 +1,7 @@
> >  #!/bin/sh -eu
> >  #
> >  # SPDX-FileCopyrightText: 2021-2022 Alyssa Ross <hi@alyssa.is>
> > +# SPDX-FileCopyrightText: 2022 Unikie
> >  # SPDX-License-Identifier: EUPL-1.2+
> >  #
> >  # usage: make-gpt.sh GPT_PATH PATH:PARTTYPE[:PARTUUID]...
> > @@ -38,7 +39,7 @@ scriptsDir="$(dirname "$0")"
> >  out="$1"
> >  shift
> >
> > -nl=$'\n'
> > +nl='\n'
> >  table="label: gpt"
> >
> >  # Keep 1MiB free at the start, and 1MiB free at the end.
> > @@ -51,9 +52,7 @@ done
> >
> >  rm -f "$out"
> >  truncate -s "$gptBytes" "$out"
> > -sfdisk "$out" <<EOF
> > -$table
> > -EOF
> > +printf "$table" | sfdisk "$out"
> 
> The heredoc previously used here should be POSIX:
> 
>   https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_07_04
> 
> Does it not work in dash?
>

Heredoc works in dash too, but I couldn't find out how to expand the 
newlines in variable inside heredoc so that it would work in dash.
Therefore I simply removed the heredoc and used printf instead.

> >
> >  n=0
> >  for partition; do
> > --
> > 2.25.1
> >
> >