Re: [PATCH 5/8] run-coverity-scan: add --no-update-tools option

[Peter Maydell <peter.maydell@linaro.org>]

On Wed, 22 Apr 2020 at 18:24, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> Provide a quick way to skip building the container while we figure out how
> to get caching right.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  scripts/coverity-scan/run-coverity-scan | 37 +++++++++++++++----------
>  1 file changed, 23 insertions(+), 14 deletions(-)
>
> diff --git a/scripts/coverity-scan/run-coverity-scan b/scripts/coverity-scan/run-coverity-scan
> index ae1fc7ae76..9403429849 100755
> --- a/scripts/coverity-scan/run-coverity-scan
> +++ b/scripts/coverity-scan/run-coverity-scan
> @@ -31,6 +31,7 @@
>  #   --dry-run : run the tools, but don't actually do the upload
>  #   --docker : create and work inside a docker container
>  #   --update-tools-only : update the cached copy of the tools, but don't run them
> +#   --no-update-tools : do not update the cached copy of the tools
>  #   --tokenfile : file to read Coverity token from
>  #   --version ver : specify version being analyzed (default: ask git)
>  #   --description desc : specify description of this version (default: ask git)
> @@ -128,7 +129,7 @@ update_coverity_tools () {
>
>  # Check user-provided environment variables and arguments
>  DRYRUN=no
> -UPDATE_ONLY=no
> +UPDATE=yes
>  DOCKER=no
>
>  while [ "$#" -ge 1 ]; do
> @@ -137,9 +138,13 @@ while [ "$#" -ge 1 ]; do
>              shift
>              DRYRUN=yes
>              ;;
> +        --no-update-tools)
> +            shift
> +            UPDATE=no
> +            ;;
>          --update-tools-only)
>              shift
> -            UPDATE_ONLY=yes
> +            UPDATE=only
>              ;;
>          --version)
>              shift
> @@ -238,12 +243,12 @@ fi
>  PROJNAME=QEMU
>  TARBALL=cov-int.tar.xz
>
> -if [ "$UPDATE_ONLY" = yes ] && [ "$DOCKER" = yes ]; then
> +if [ "$UPDATE" = only ] && [ "$DOCKER" = yes ]; then
>      echo "Combining --docker and --update-only is not supported"

Pre-existing bug,but this error message should say
"--update-tools-only".

>      exit 1
>  fi

> @@ -343,7 +350,9 @@ fi
>
>  check_upload_permissions
>
> -update_coverity_tools
> +if [ "$UPDATE" != no ]; then
> +    update_coverity_tools
> +fi
>
>  TOOLBIN="$(cd "$COVERITY_TOOL_BASE" && echo $PWD/coverity_tool/cov-analysis-*/bin)"

Do we fail in a confusing or a clean way if you try --no-update-tools
and you didn't actually have a pre-existing cached copy of them?
I guess it doesn't matter much since this isn't intended to be
used by a lot of people.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM