| Message ID | 20180420165743.223978-1-pshier@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Apr 20, 2018 at 09:57:43AM -0700, Peter Shier wrote: > KVM unit tests scripts/arch-run.bash uses test -v to check whether a variable > was set. The -v switch was introduced in Bash 4.2. This patch uses test -n to > to achieve equivalent functionality that is backward compatible. > > Add double quotes around source strings for read commands. On older Bash > versions, without the quotes the read will parse the string into its components > but then place them all into a single variable separated by spaces rather than > into separate variables as the script intends. > > Add check for missing commit in errata file. > > Tested: > Ran with an injected empty commit field in errata.txt to verify with > git://git.kernel.org/pub/scm/virt/kvm/kvm-unit-tests.git at > 073ea627a4268333e0e2245382ecf5fabc28f594. This isn't an upstream commit hash. The latest upstream hash is 2352e986e4599bc4842c225762c78fa49f18648d. > > Ran full test suite on Bash 4.1 > > Signed-off-by: Peter Shier <pshier@google.com> > --- > scripts/arch-run.bash | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash > index e13af8e8064a..7410cf4b87cd 100644 > --- a/scripts/arch-run.bash > +++ b/scripts/arch-run.bash > @@ -209,7 +209,7 @@ env_add_errata () > if [ -f "$ENV" ] && grep -q '^ERRATA_' <(env); then > for line in $(grep '^ERRATA_' "$ENV"); do > errata=${line%%=*} > - test -v $errata && continue > + [[ -n "${!errata}" ]] && continue [...] would also work, but OK > eval export "$line" > done > elif [ ! -f "$ENV" ]; then > @@ -234,8 +234,8 @@ env_generate_errata () > local kernel_version kernel_patchlevel kernel_sublevel kernel_extraversion > local line commit minver errata rest v p s x have > > - IFS=. read -r kernel_version kernel_patchlevel rest <<<$kernel_version_string > - IFS=- read -r kernel_sublevel kernel_extraversion <<<$rest > + IFS=. read -r kernel_version kernel_patchlevel rest <<<"$kernel_version_string" > + IFS=- read -r kernel_sublevel kernel_extraversion <<<"$rest" > kernel_sublevel=${kernel_sublevel%%[!0-9]*} > kernel_extraversion=${kernel_extraversion%%[!0-9]*} > > @@ -249,8 +249,13 @@ env_generate_errata () > commit=${line%:*} > minver=${line#*:} > > + if [ -z "$commit" ]; then > + echo "Errata file $ERRATATXT corrupt. Missing commit on line: $line" Adding a counter to the loop in order to output line number would be nicer, but then we'd have to rework things to also count the comment lines, so it's probably not worth it. > + return 2 > + fi > + > errata="ERRATA_$commit" > - test -v $errata && continue > + [[ -n "${!errata}" ]] && continue > > IFS=. read -r v p rest <<<"$minver" > IFS=- read -r s x <<<"$rest" > -- > 2.17.0.484.g0c8726318c-goog > Besides the hash in the commit message, which can probably be fixed while being applied Reviewed-by: Andrew Jones <drjones@redhat.com>
On Thu, Jun 14, 2018 at 11:38:45AM -0700, Peter Shier wrote: > Hi Andrew, > > I just noticed that this patch has not been merged. I was under the > impression that you did not want any other changes. Is there something else > that needs to be done? Just Paolo or Radim need to commit it. I've CC'ed them. Thanks, drew > > Thx, > Peter > > On Mon, Apr 23, 2018 at 2:02 AM Andrew Jones <drjones@redhat.com> wrote: > > > On Fri, Apr 20, 2018 at 09:57:43AM -0700, Peter Shier wrote: > > > KVM unit tests scripts/arch-run.bash uses test -v to check whether a > > variable > > > was set. The -v switch was introduced in Bash 4.2. This patch uses test > > -n to > > > to achieve equivalent functionality that is backward compatible. > > > > > > Add double quotes around source strings for read commands. On older Bash > > > versions, without the quotes the read will parse the string into its > > components > > > but then place them all into a single variable separated by spaces > > rather than > > > into separate variables as the script intends. > > > > > > Add check for missing commit in errata file. > > > > > > Tested: > > > Ran with an injected empty commit field in errata.txt to verify with > > > git://git.kernel.org/pub/scm/virt/kvm/kvm-unit-tests.git at > > > 073ea627a4268333e0e2245382ecf5fabc28f594. > > > > This isn't an upstream commit hash. The latest upstream hash is > > 2352e986e4599bc4842c225762c78fa49f18648d. > > > > > > > > Ran full test suite on Bash 4.1 > > > > > > Signed-off-by: Peter Shier <pshier@google.com> > > > --- > > > scripts/arch-run.bash | 13 +++++++++---- > > > 1 file changed, 9 insertions(+), 4 deletions(-) > > > > > > diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash > > > index e13af8e8064a..7410cf4b87cd 100644 > > > --- a/scripts/arch-run.bash > > > +++ b/scripts/arch-run.bash > > > @@ -209,7 +209,7 @@ env_add_errata () > > > if [ -f "$ENV" ] && grep -q '^ERRATA_' <(env); then > > > for line in $(grep '^ERRATA_' "$ENV"); do > > > errata=${line%%=*} > > > - test -v $errata && continue > > > + [[ -n "${!errata}" ]] && continue > > > > [...] would also work, but OK > > > > > eval export "$line" > > > done > > > elif [ ! -f "$ENV" ]; then > > > @@ -234,8 +234,8 @@ env_generate_errata () > > > local kernel_version kernel_patchlevel kernel_sublevel > > kernel_extraversion > > > local line commit minver errata rest v p s x have > > > > > > - IFS=. read -r kernel_version kernel_patchlevel rest > > <<<$kernel_version_string > > > - IFS=- read -r kernel_sublevel kernel_extraversion <<<$rest > > > + IFS=. read -r kernel_version kernel_patchlevel rest > > <<<"$kernel_version_string" > > > + IFS=- read -r kernel_sublevel kernel_extraversion <<<"$rest" > > > kernel_sublevel=${kernel_sublevel%%[!0-9]*} > > > kernel_extraversion=${kernel_extraversion%%[!0-9]*} > > > > > > @@ -249,8 +249,13 @@ env_generate_errata () > > > commit=${line%:*} > > > minver=${line#*:} > > > > > > + if [ -z "$commit" ]; then > > > + echo "Errata file $ERRATATXT corrupt. Missing > > commit on line: $line" > > > > Adding a counter to the loop in order to output line number would be > > nicer, but then we'd have to rework things to also count the comment > > lines, so it's probably not worth it. > > > > > + return 2 > > > + fi > > > + > > > errata="ERRATA_$commit" > > > - test -v $errata && continue > > > + [[ -n "${!errata}" ]] && continue > > > > > > IFS=. read -r v p rest <<<"$minver" > > > IFS=- read -r s x <<<"$rest" > > > -- > > > 2.17.0.484.g0c8726318c-goog > > > > > > > Besides the hash in the commit message, which can probably be fixed while > > being applied > > > > Reviewed-by: Andrew Jones <drjones@redhat.com> > >
2018-06-15 08:57+0200, Andrew Jones: > On Thu, Jun 14, 2018 at 11:38:45AM -0700, Peter Shier wrote: > > Hi Andrew, > > > > I just noticed that this patch has not been merged. I was under the > > impression that you did not want any other changes. Is there something else > > that needs to be done? > > Just Paolo or Radim need to commit it. I've CC'ed them. Before I push it, I'd like to confirm that you meant to say "... compatible with Bash 4.1 and *earlier*" in the subject. We've been already 4.2 compatible and the new construction probably was in bash since version 2, so I would change it. Thanks.
diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash index e13af8e8064a..7410cf4b87cd 100644 --- a/scripts/arch-run.bash +++ b/scripts/arch-run.bash @@ -209,7 +209,7 @@ env_add_errata () if [ -f "$ENV" ] && grep -q '^ERRATA_' <(env); then for line in $(grep '^ERRATA_' "$ENV"); do errata=${line%%=*} - test -v $errata && continue + [[ -n "${!errata}" ]] && continue eval export "$line" done elif [ ! -f "$ENV" ]; then @@ -234,8 +234,8 @@ env_generate_errata () local kernel_version kernel_patchlevel kernel_sublevel kernel_extraversion local line commit minver errata rest v p s x have - IFS=. read -r kernel_version kernel_patchlevel rest <<<$kernel_version_string - IFS=- read -r kernel_sublevel kernel_extraversion <<<$rest + IFS=. read -r kernel_version kernel_patchlevel rest <<<"$kernel_version_string" + IFS=- read -r kernel_sublevel kernel_extraversion <<<"$rest" kernel_sublevel=${kernel_sublevel%%[!0-9]*} kernel_extraversion=${kernel_extraversion%%[!0-9]*} @@ -249,8 +249,13 @@ env_generate_errata () commit=${line%:*} minver=${line#*:} + if [ -z "$commit" ]; then + echo "Errata file $ERRATATXT corrupt. Missing commit on line: $line" + return 2 + fi + errata="ERRATA_$commit" - test -v $errata && continue + [[ -n "${!errata}" ]] && continue IFS=. read -r v p rest <<<"$minver" IFS=- read -r s x <<<"$rest"
KVM unit tests scripts/arch-run.bash uses test -v to check whether a variable was set. The -v switch was introduced in Bash 4.2. This patch uses test -n to to achieve equivalent functionality that is backward compatible. Add double quotes around source strings for read commands. On older Bash versions, without the quotes the read will parse the string into its components but then place them all into a single variable separated by spaces rather than into separate variables as the script intends. Add check for missing commit in errata file. Tested: Ran with an injected empty commit field in errata.txt to verify with git://git.kernel.org/pub/scm/virt/kvm/kvm-unit-tests.git at 073ea627a4268333e0e2245382ecf5fabc28f594. Ran full test suite on Bash 4.1 Signed-off-by: Peter Shier <pshier@google.com> --- scripts/arch-run.bash | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)