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

Paolo Bonzini posted 8 patches 5 years, 9 months ago
Maintainers: "Philippe Mathieu-Daudé" <philmd@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Fam Zheng <fam@euphon.net>, "Alex Bennée" <alex.bennee@linaro.org>
There is a newer version of this series
[PATCH 5/8] run-coverity-scan: add --no-update-tools option
Posted by Paolo Bonzini 5 years, 9 months ago
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"
     exit 1
 fi
 
-if [ "$UPDATE_ONLY" = yes ]; then
+if [ "$UPDATE" = only ]; then
     # Just do the tools update; we don't need to check whether
     # we are in a source tree or have upload rights for this,
     # so do it before some of the command line and source tree checks.
@@ -286,7 +291,6 @@ fi
 
 # Run ourselves inside docker if that's what the user wants
 if [ "$DOCKER" = yes ]; then
-    # build docker container including the coverity-scan tools
     # Put the Coverity token into a temporary file that only
     # we have read access to, and then pass it to docker build
     # using a volume.  A volume is enough for the token not to
@@ -301,14 +305,17 @@ if [ "$DOCKER" = yes ]; then
     echo "Created temporary directory $SECRETDIR"
     SECRET="$SECRETDIR/token"
     echo "$COVERITY_TOKEN" > "$SECRET"
-    echo "Building docker container..."
-    # TODO: This re-downloads the tools every time, rather than
-    # caching and reusing the image produced with the downloaded tools.
-    # Not sure why.
-    tests/docker/docker.py --engine ${DOCKER_ENGINE} build \
-                   -t coverity-scanner -f scripts/coverity-scan/coverity-scan.docker \
-                   -v "$SECRETDIR:/work" \
-                   --extra-files scripts/coverity-scan/run-coverity-scan
+    if [ "$UPDATE" != no ]; then
+        # build docker container including the coverity-scan tools
+        echo "Building docker container..."
+        # TODO: This re-downloads the tools every time, rather than
+        # caching and reusing the image produced with the downloaded tools.
+        # Not sure why.
+        tests/docker/docker.py --engine ${DOCKER_ENGINE} build \
+                       -t coverity-scanner -f scripts/coverity-scan/coverity-scan.docker \
+                       -v "$SECRETDIR:/work" \
+                       --extra-files scripts/coverity-scan/run-coverity-scan
+    fi
     echo "Archiving sources to be analyzed..."
     ./scripts/archive-source.sh "$SECRETDIR/qemu-sources.tgz"
     if [ "$DRYRUN" = yes ]; then
@@ -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)"
 
-- 
2.18.2



Re: [PATCH 5/8] run-coverity-scan: add --no-update-tools option
Posted by Peter Maydell 5 years, 9 months ago
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

Re: [PATCH 5/8] run-coverity-scan: add --no-update-tools option
Posted by Paolo Bonzini 5 years, 9 months ago
On 27/04/20 14:46, Peter Maydell wrote:
>> -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".
> 

I noticed, however by the end of the series it was easier to add the
support...

Paolo