[PATCH v2 0/8] run-coverity-scan: misc improvements, especially for docker mode

[PATCH v2 0/8] run-coverity-scan: misc improvements, especially for docker mode

[Paolo Bonzini]

These include:

1) podman support through tests/docker/docker.py

2) avoiding repeated downloading of the tools in the container, by
   sharing the cache with the host

3) support for --update-tools-only --docker (though unlike regular
   --update-tools-only it must be run from within a QEMU source tree)

4) not related to docker mode, but used by it, a new --no-update-tools
   option that does not check for tool updates

5) the ability to get the Coverity token from git configuration, and
   also to have email from git configuration if it is not equal to
   user.email.

Patches 1 and 2 are tweaks to tests/docker/docker.py, while the others
are for run-coverity-scan.

v1->v2: adjust comments, new option --docker-engine

Paolo Bonzini (8):
  docker.py/build: support -t and -f arguments
  docker.py/build: support binary files in --extra-files
  run-coverity-scan: get Coverity token and email from special git
    config section
  run-coverity-scan: use docker.py
  run-coverity-scan: add --no-update-tools option
  run-coverity-scan: use --no-update-tools in docker run
  run-coverity-scan: download tools outside the container
  run-coverity-scan: support --update-tools-only --docker

 scripts/coverity-scan/coverity-scan.docker |   3 +-
 scripts/coverity-scan/run-coverity-scan    | 139 +++++++++++++--------
 tests/docker/Makefile.include              |   2 +-
 tests/docker/docker.py                     |  14 ++-
 4 files changed, 98 insertions(+), 60 deletions(-)

-- 
2.26.2

[PATCH v2 1/8] docker.py/build: support -t and -f arguments

[Paolo Bonzini]

The docker.py command line is subtly different from docker and podman's,
in that the tag and Dockerfile are passed via positional arguments.
Remove this gratuitous difference and just parse -f and -t.

-f was previously used by --extra-files, only keep the long option.

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/docker/Makefile.include | 2 +-
 tests/docker/docker.py        | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index 43a8678688..262704663f 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -55,7 +55,7 @@ docker-image-%: $(DOCKER_FILES_DIR)/%.docker
 else
 docker-image-%: $(DOCKER_FILES_DIR)/%.docker
 	$(call quiet-command,\
-		$(DOCKER_SCRIPT) build qemu:$* $< \
+		$(DOCKER_SCRIPT) build -t qemu:$* -f $< \
 		$(if $V,,--quiet) $(if $(NOCACHE),--no-cache) \
 		$(if $(NOUSER),,--add-current-user) \
 		$(if $(EXTRA_FILES),--extra-files $(EXTRA_FILES))\
diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index d8268c1111..ad61747bae 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -391,16 +391,16 @@ class BuildCommand(SubCommand):
                             help="""Specify a binary that will be copied to the
                             container together with all its dependent
                             libraries""")
-        parser.add_argument("--extra-files", "-f", nargs='*',
+        parser.add_argument("--extra-files", nargs='*',
                             help="""Specify files that will be copied in the
                             Docker image, fulfilling the ADD directive from the
                             Dockerfile""")
         parser.add_argument("--add-current-user", "-u", dest="user",
                             action="store_true",
                             help="Add the current user to image's passwd")
-        parser.add_argument("tag",
+        parser.add_argument("-t", dest="tag",
                             help="Image Tag")
-        parser.add_argument("dockerfile",
+        parser.add_argument("-f", dest="dockerfile",
                             help="Dockerfile name")
 
     def run(self, args, argv):
-- 
2.26.2

[PATCH v2 2/8] docker.py/build: support binary files in --extra-files

[Paolo Bonzini]

Read the --extra-files in binary mode to avoid encoding errors.

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/docker/docker.py | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index ad61747bae..85e1dda10f 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -56,15 +56,19 @@ class EngineEnum(enum.IntEnum):
 
 USE_ENGINE = EngineEnum.AUTO
 
+def _bytes_checksum(bytes):
+    """Calculate a digest string unique to the text content"""
+    return hashlib.sha1(bytes).hexdigest()
+
 def _text_checksum(text):
     """Calculate a digest string unique to the text content"""
-    return hashlib.sha1(text.encode('utf-8')).hexdigest()
+    return _bytes_checksum(text.encode('utf-8'))
 
 def _read_dockerfile(path):
     return open(path, 'rt', encoding='utf-8').read()
 
 def _file_checksum(filename):
-    return _text_checksum(_read_dockerfile(filename))
+    return _bytes_checksum(open(filename, 'rb').read())
 
 
 def _guess_engine_command():
-- 
2.26.2

[PATCH v2 3/8] run-coverity-scan: get Coverity token and email from special git config section

[Paolo Bonzini]

Support a [coverity] section in .git/config.  It can be used to retrieve the
token and also, if it is different from user.email, the username of the
submitter.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/coverity-scan/run-coverity-scan | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/scripts/coverity-scan/run-coverity-scan b/scripts/coverity-scan/run-coverity-scan
index 2e067ef5cf..990f75138d 100755
--- a/scripts/coverity-scan/run-coverity-scan
+++ b/scripts/coverity-scan/run-coverity-scan
@@ -41,9 +41,10 @@
 #                   is intended mainly for internal use by the Docker support
 #
 # User-specifiable environment variables:
-#  COVERITY_TOKEN -- Coverity token
+#  COVERITY_TOKEN -- Coverity token (default: looks at your
+#                    coverity.token config)
 #  COVERITY_EMAIL -- the email address to use for uploads (default:
-#                    looks at your git user.email config)
+#                    looks at your git coverity.email or user.email config)
 #  COVERITY_BUILD_CMD -- make command (default: 'make -jN' where N is
 #                    number of CPUs as determined by 'nproc')
 #  COVERITY_TOOL_BASE -- set to directory to put coverity tools
@@ -58,11 +59,11 @@ check_upload_permissions() {
     # with status 1 if the check failed (usually a bad token);
     # will exit the script with status 0 if the check indicated that we
     # can't upload yet (ie we are at quota)
-    # Assumes that PROJTOKEN, PROJNAME and DRYRUN have been initialized.
+    # Assumes that COVERITY_TOKEN, PROJNAME and DRYRUN have been initialized.
 
     echo "Checking upload permissions..."
 
-    if ! up_perm="$(wget https://scan.coverity.com/api/upload_permitted --post-data "token=$PROJTOKEN&project=$PROJNAME" -q -O -)"; then
+    if ! up_perm="$(wget https://scan.coverity.com/api/upload_permitted --post-data "token=$COVERITY_TOKEN&project=$PROJNAME" -q -O -)"; then
         echo "Coverity Scan API access denied: bad token?"
         exit 1
     fi
@@ -94,20 +95,20 @@ check_upload_permissions() {
 update_coverity_tools () {
     # Check for whether we need to download the Coverity tools
     # (either because we don't have a copy, or because it's out of date)
-    # Assumes that COVERITY_TOOL_BASE, PROJTOKEN and PROJNAME are set.
+    # Assumes that COVERITY_TOOL_BASE, COVERITY_TOKEN and PROJNAME are set.
 
     mkdir -p "$COVERITY_TOOL_BASE"
     cd "$COVERITY_TOOL_BASE"
 
     echo "Checking for new version of coverity build tools..."
-    wget https://scan.coverity.com/download/linux64 --post-data "token=$PROJTOKEN&project=$PROJNAME&md5=1" -O coverity_tool.md5.new
+    wget https://scan.coverity.com/download/linux64 --post-data "token=$COVERITY_TOKEN&project=$PROJNAME&md5=1" -O coverity_tool.md5.new
 
     if ! cmp -s coverity_tool.md5 coverity_tool.md5.new; then
         # out of date md5 or no md5: download new build tool
         # blow away the old build tool
         echo "Downloading coverity build tools..."
         rm -rf coverity_tool coverity_tool.tgz
-        wget https://scan.coverity.com/download/linux64 --post-data "token=$PROJTOKEN&project=$PROJNAME" -O coverity_tool.tgz
+        wget https://scan.coverity.com/download/linux64 --post-data "token=$COVERITY_TOKEN&project=$PROJNAME" -O coverity_tool.tgz
         if ! (cat coverity_tool.md5.new; echo "  coverity_tool.tgz") | md5sum -c --status; then
             echo "Downloaded tarball didn't match md5sum!"
             exit 1
@@ -205,6 +206,9 @@ while [ "$#" -ge 1 ]; do
     esac
 done
 
+if [ -z "$COVERITY_TOKEN" ]; then
+    COVERITY_TOKEN="$(git config coverity.token)"
+fi
 if [ -z "$COVERITY_TOKEN" ]; then
     echo "COVERITY_TOKEN environment variable not set"
     exit 1
@@ -225,7 +229,6 @@ if [ -z "$SRCDIR" ]; then
     SRCDIR="$PWD"
 fi
 
-PROJTOKEN="$COVERITY_TOKEN"
 PROJNAME=QEMU
 TARBALL=cov-int.tar.xz
 
@@ -268,6 +271,9 @@ if [ -z "$DESCRIPTION" ]; then
     DESCRIPTION="$(git rev-parse HEAD)"
 fi
 
+if [ -z "$COVERITY_EMAIL" ]; then
+    COVERITY_EMAIL="$(git config coverity.email)"
+fi
 if [ -z "$COVERITY_EMAIL" ]; then
     COVERITY_EMAIL="$(git config user.email)"
 fi
@@ -393,7 +399,7 @@ if [ "$DRYRUN" = yes ]; then
     exit 0
 fi
 
-curl --form token="$PROJTOKEN" --form email="$COVERITY_EMAIL" \
+curl --form token="$COVERITY_TOKEN" --form email="$COVERITY_EMAIL" \
      --form file=@"$TARBALL" --form version="$VERSION" \
      --form description="$DESCRIPTION" \
      https://scan.coverity.com/builds?project="$PROJNAME"
-- 
2.26.2

[PATCH v2 4/8] run-coverity-scan: use docker.py

[Paolo Bonzini]

Our trusted docker wrapper allows run-coverity-scan to run with both
docker and podman.

For the "run" phase this is transparent; for the "build" phase however
scripts are replaced with a bind mount (-v).  This is not an issue
because the secret option is meant for secrets stored globally in the
system and bind mounts are a valid substitute for secrets that are known
to whoever builds the container.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/coverity-scan/coverity-scan.docker |  2 +-
 scripts/coverity-scan/run-coverity-scan    | 32 ++++++++++++++--------
 2 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/scripts/coverity-scan/coverity-scan.docker b/scripts/coverity-scan/coverity-scan.docker
index a4f64d1283..6f0460b66c 100644
--- a/scripts/coverity-scan/coverity-scan.docker
+++ b/scripts/coverity-scan/coverity-scan.docker
@@ -128,4 +128,4 @@ RUN rpm -q $PACKAGES | sort > /packages.txt
 ENV PATH $PATH:/usr/libexec/python3-sphinx/
 ENV COVERITY_TOOL_BASE=/coverity-tools
 COPY run-coverity-scan run-coverity-scan
-RUN --mount=type=secret,id=coverity.token,required ./run-coverity-scan --update-tools-only --tokenfile /run/secrets/coverity.token
+RUN ./run-coverity-scan --update-tools-only --tokenfile /work/token
diff --git a/scripts/coverity-scan/run-coverity-scan b/scripts/coverity-scan/run-coverity-scan
index 990f75138d..e926623b3b 100755
--- a/scripts/coverity-scan/run-coverity-scan
+++ b/scripts/coverity-scan/run-coverity-scan
@@ -29,7 +29,9 @@
 
 # Command line options:
 #   --dry-run : run the tools, but don't actually do the upload
-#   --docker : create and work inside a docker container
+#   --docker : create and work inside a container
+#   --docker-engine : specify the container engine to use (docker/podman/auto);
+#                     implies --docker
 #   --update-tools-only : update the cached copy of the tools, but don't run them
 #   --tokenfile : file to read Coverity token from
 #   --version ver : specify version being analyzed (default: ask git)
@@ -197,6 +199,17 @@ while [ "$#" -ge 1 ]; do
             ;;
         --docker)
             DOCKER=yes
+            DOCKER_ENGINE=auto
+            shift
+            ;;
+        --docker-engine)
+            shift
+            if [ $# -eq 0 ]; then
+                echo "--docker-engine needs an argument"
+                exit 1
+            fi
+            DOCKER=yes
+            DOCKER_ENGINE="$1"
             shift
             ;;
         *)
@@ -283,9 +296,8 @@ 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 --secret. This requires at least Docker 18.09.
-    # Mostly what we are trying to do here is ensure we don't leak
-    # the token into the Docker image.
+    # using a volume.  A volume is enough for the token not to
+    # leak into the Docker image.
     umask 077
     SECRETDIR=$(mktemp -d)
     if [ -z "$SECRETDIR" ]; then
@@ -300,12 +312,10 @@ if [ "$DOCKER" = yes ]; then
     # TODO: This re-downloads the tools every time, rather than
     # caching and reusing the image produced with the downloaded tools.
     # Not sure why.
-    # TODO: how do you get 'docker build' to print the output of the
-    # commands it is running to its stdout? This would be useful for debug.
-    DOCKER_BUILDKIT=1 docker build -t coverity-scanner \
-                   --secret id=coverity.token,src="$SECRET" \
-                   -f scripts/coverity-scan/coverity-scan.docker \
-                   scripts/coverity-scan
+    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
     echo "Archiving sources to be analyzed..."
     ./scripts/archive-source.sh "$SECRETDIR/qemu-sources.tgz"
     if [ "$DRYRUN" = yes ]; then
@@ -323,7 +333,7 @@ if [ "$DOCKER" = yes ]; then
     # Arrange for this docker run to get access to the sources with -v.
     # We pass through all the configuration from the outer script to the inner.
     export COVERITY_EMAIL COVERITY_BUILD_CMD
-    docker run -it --env COVERITY_EMAIL --env COVERITY_BUILD_CMD \
+    tests/docker/docker.py run -it --env COVERITY_EMAIL --env COVERITY_BUILD_CMD \
            -v "$SECRETDIR:/work" coverity-scanner \
            ./run-coverity-scan --version "$VERSION" \
            --description "$DESCRIPTION" $DRYRUNARG --tokenfile /work/token \
-- 
2.26.2

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

[Paolo Bonzini]

Provide a quick way to skip building the container while we figure out how
to get caching right.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
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 e926623b3b..bc9e12670b 100755
--- a/scripts/coverity-scan/run-coverity-scan
+++ b/scripts/coverity-scan/run-coverity-scan
@@ -33,6 +33,7 @@
 #   --docker-engine : specify the container engine to use (docker/podman/auto);
 #                     implies --docker
 #   --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)
@@ -130,7 +131,7 @@ update_coverity_tools () {
 
 # Check user-provided environment variables and arguments
 DRYRUN=no
-UPDATE_ONLY=no
+UPDATE=yes
 DOCKER=no
 
 while [ "$#" -ge 1 ]; do
@@ -139,9 +140,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
@@ -245,12 +250,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.
@@ -293,7 +298,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
@@ -308,14 +312,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
@@ -350,7 +357,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.26.2

[PATCH v2 6/8] run-coverity-scan: use --no-update-tools in docker run

[Paolo Bonzini]

Tools are already updated via the docker build.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/coverity-scan/run-coverity-scan | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/scripts/coverity-scan/run-coverity-scan b/scripts/coverity-scan/run-coverity-scan
index bc9e12670b..6bb38b4f48 100755
--- a/scripts/coverity-scan/run-coverity-scan
+++ b/scripts/coverity-scan/run-coverity-scan
@@ -325,17 +325,16 @@ if [ "$DOCKER" = yes ]; then
     fi
     echo "Archiving sources to be analyzed..."
     ./scripts/archive-source.sh "$SECRETDIR/qemu-sources.tgz"
+    ARGS="--no-update-tools"
     if [ "$DRYRUN" = yes ]; then
-        DRYRUNARG=--dry-run
+        ARGS="$ARGS --dry-run"
     fi
     echo "Running scanner..."
     # If we need to capture the output tarball, get the inner run to
     # save it to the secrets directory so we can copy it out before the
     # directory is cleaned up.
     if [ ! -z "$RESULTSTARBALL" ]; then
-        RTARGS="--results-tarball /work/cov-int.tar.xz"
-    else
-        RTARGS=""
+        ARGS="$ARGS --results-tarball /work/cov-int.tar.xz"
     fi
     # Arrange for this docker run to get access to the sources with -v.
     # We pass through all the configuration from the outer script to the inner.
@@ -343,8 +342,8 @@ if [ "$DOCKER" = yes ]; then
     tests/docker/docker.py run -it --env COVERITY_EMAIL --env COVERITY_BUILD_CMD \
            -v "$SECRETDIR:/work" coverity-scanner \
            ./run-coverity-scan --version "$VERSION" \
-           --description "$DESCRIPTION" $DRYRUNARG --tokenfile /work/token \
-           --srcdir /qemu --src-tarball /work/qemu-sources.tgz $RTARGS
+           --description "$DESCRIPTION" $ARGS --tokenfile /work/token \
+           --srcdir /qemu --src-tarball /work/qemu-sources.tgz
     if [ ! -z "$RESULTSTARBALL" ]; then
         echo "Copying results tarball to $RESULTSTARBALL..."
         cp "$SECRETDIR/cov-int.tar.xz" "$RESULTSTARBALL"
-- 
2.26.2

[PATCH v2 7/8] run-coverity-scan: download tools outside the container

[Paolo Bonzini]

This lets us look at coverity_tool.md5 across executions of run-coverity-scan
and skip the download.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/coverity-scan/coverity-scan.docker |  3 +-
 scripts/coverity-scan/run-coverity-scan    | 42 +++++++++++-----------
 2 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/scripts/coverity-scan/coverity-scan.docker b/scripts/coverity-scan/coverity-scan.docker
index 6f0460b66c..efcf63224d 100644
--- a/scripts/coverity-scan/coverity-scan.docker
+++ b/scripts/coverity-scan/coverity-scan.docker
@@ -127,5 +127,6 @@ RUN dnf install -y $PACKAGES
 RUN rpm -q $PACKAGES | sort > /packages.txt
 ENV PATH $PATH:/usr/libexec/python3-sphinx/
 ENV COVERITY_TOOL_BASE=/coverity-tools
+COPY coverity_tool.tgz coverity_tool.tgz
+RUN mkdir -p /coverity-tools/coverity_tool && cd /coverity-tools/coverity_tool && tar xf /coverity_tool.tgz
 COPY run-coverity-scan run-coverity-scan
-RUN ./run-coverity-scan --update-tools-only --tokenfile /work/token
diff --git a/scripts/coverity-scan/run-coverity-scan b/scripts/coverity-scan/run-coverity-scan
index 6bb38b4f48..8bff952bf5 100755
--- a/scripts/coverity-scan/run-coverity-scan
+++ b/scripts/coverity-scan/run-coverity-scan
@@ -116,15 +116,17 @@ update_coverity_tools () {
             echo "Downloaded tarball didn't match md5sum!"
             exit 1
         fi
-        # extract the new one, keeping it corralled in a 'coverity_tool' directory
-        echo "Unpacking coverity build tools..."
-        mkdir -p coverity_tool
-        cd coverity_tool
-        tar xf ../coverity_tool.tgz
-        cd ..
-        mv coverity_tool.md5.new coverity_tool.md5
-    fi
 
+        if [ "$DOCKER" != yes ]; then
+            # extract the new one, keeping it corralled in a 'coverity_tool' directory
+            echo "Unpacking coverity build tools..."
+            mkdir -p coverity_tool
+            cd coverity_tool
+            tar xf ../coverity_tool.tgz
+            cd ..
+            mv coverity_tool.md5.new coverity_tool.md5
+        fi
+    fi
     rm -f coverity_tool.md5.new
 }
 
@@ -296,6 +298,14 @@ if [ -z "$COVERITY_EMAIL" ]; then
     COVERITY_EMAIL="$(git config user.email)"
 fi
 
+# Otherwise, continue with the full build and upload process.
+
+check_upload_permissions
+
+if [ "$UPDATE" != no ]; then
+    update_coverity_tools
+fi
+
 # Run ourselves inside docker if that's what the user wants
 if [ "$DOCKER" = yes ]; then
     # Put the Coverity token into a temporary file that only
@@ -315,13 +325,13 @@ if [ "$DOCKER" = yes ]; then
     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.
+        # TODO: This re-unpacks the tools every time, rather than caching
+        # and reusing the image produced by the COPY of the .tgz file.
         # 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
+                       --extra-files scripts/coverity-scan/run-coverity-scan \
+                                     "$COVERITY_TOOL_BASE"/coverity_tool.tgz
     fi
     echo "Archiving sources to be analyzed..."
     ./scripts/archive-source.sh "$SECRETDIR/qemu-sources.tgz"
@@ -352,14 +362,6 @@ if [ "$DOCKER" = yes ]; then
     exit 0
 fi
 
-# Otherwise, continue with the full build and upload process.
-
-check_upload_permissions
-
-if [ "$UPDATE" != no ]; then
-    update_coverity_tools
-fi
-
 TOOLBIN="$(cd "$COVERITY_TOOL_BASE" && echo $PWD/coverity_tool/cov-analysis-*/bin)"
 
 if ! test -x "$TOOLBIN/cov-build"; then
-- 
2.26.2

[PATCH v2 8/8] run-coverity-scan: support --update-tools-only --docker

[Paolo Bonzini]

Just build the container when run-coverity-scan is invoked with
--update-tools-only --docker.  This requires moving the "docker build"
logic into the update_coverity_tools function.

The only snag is that --update-tools-only --docker requires access to
the dockerfile.  For now just report an error for --src-tarball, and
"docker build" will fail if not in a source tree.  Another possibility
could be to host our container images on a public registry, and use
"FROM qemu:fedora" to make the Dockerfile small enough that it can be
included directly in the run-coverity-scan script.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/coverity-scan/run-coverity-scan | 39 +++++++++++++++----------
 1 file changed, 23 insertions(+), 16 deletions(-)

diff --git a/scripts/coverity-scan/run-coverity-scan b/scripts/coverity-scan/run-coverity-scan
index 8bff952bf5..03a791dec9 100755
--- a/scripts/coverity-scan/run-coverity-scan
+++ b/scripts/coverity-scan/run-coverity-scan
@@ -95,6 +95,18 @@ check_upload_permissions() {
 }
 
 
+build_docker_image() {
+    # build docker container including the coverity-scan tools
+    echo "Building docker container..."
+    # TODO: This re-unpacks the tools every time, rather than caching
+    # and reusing the image produced by the COPY of the .tgz file.
+    # Not sure why.
+    tests/docker/docker.py --engine ${DOCKER_ENGINE} build \
+                   -t coverity-scanner -f scripts/coverity-scan/coverity-scan.docker \
+                   --extra-files scripts/coverity-scan/run-coverity-scan \
+                                 "$COVERITY_TOOL_BASE"/coverity_tool.tgz
+}
+
 update_coverity_tools () {
     # Check for whether we need to download the Coverity tools
     # (either because we don't have a copy, or because it's out of date)
@@ -128,6 +140,11 @@ update_coverity_tools () {
         fi
     fi
     rm -f coverity_tool.md5.new
+    cd "$SRCDIR"
+
+    if [ "$DOCKER" = yes ]; then
+        build_docker_image
+    fi
 }
 
 
@@ -252,15 +269,16 @@ fi
 PROJNAME=QEMU
 TARBALL=cov-int.tar.xz
 
-if [ "$UPDATE" = only ] && [ "$DOCKER" = yes ]; then
-    echo "Combining --docker and --update-only is not supported"
-    exit 1
-fi
-
 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.
+
+    if [ "$DOCKER" = yes ] && [ ! -z "$SRCTARBALL" ]; then
+        echo --update-tools-only --docker is incompatible with --src-tarball.
+        exit 1
+    fi
+
     update_coverity_tools
     exit 0
 fi
@@ -322,17 +340,6 @@ if [ "$DOCKER" = yes ]; then
     echo "Created temporary directory $SECRETDIR"
     SECRET="$SECRETDIR/token"
     echo "$COVERITY_TOKEN" > "$SECRET"
-    if [ "$UPDATE" != no ]; then
-        # build docker container including the coverity-scan tools
-        echo "Building docker container..."
-        # TODO: This re-unpacks the tools every time, rather than caching
-        # and reusing the image produced by the COPY of the .tgz file.
-        # Not sure why.
-        tests/docker/docker.py --engine ${DOCKER_ENGINE} build \
-                       -t coverity-scanner -f scripts/coverity-scan/coverity-scan.docker \
-                       --extra-files scripts/coverity-scan/run-coverity-scan \
-                                     "$COVERITY_TOOL_BASE"/coverity_tool.tgz
-    fi
     echo "Archiving sources to be analyzed..."
     ./scripts/archive-source.sh "$SECRETDIR/qemu-sources.tgz"
     ARGS="--no-update-tools"
-- 
2.26.2

Re: [PATCH v2 4/8] run-coverity-scan: use docker.py

[Peter Maydell]

On Thu, 21 May 2020 at 13:45, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> Our trusted docker wrapper allows run-coverity-scan to run with both
> docker and podman.
>
> For the "run" phase this is transparent; for the "build" phase however
> scripts are replaced with a bind mount (-v).  This is not an issue
> because the secret option is meant for secrets stored globally in the
> system and bind mounts are a valid substitute for secrets that are known
> to whoever builds the container.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  scripts/coverity-scan/coverity-scan.docker |  2 +-
>  scripts/coverity-scan/run-coverity-scan    | 32 ++++++++++++++--------
>  2 files changed, 22 insertions(+), 12 deletions(-)

> @@ -300,12 +312,10 @@ if [ "$DOCKER" = yes ]; then
>      # TODO: This re-downloads the tools every time, rather than
>      # caching and reusing the image produced with the downloaded tools.
>      # Not sure why.
> -    # TODO: how do you get 'docker build' to print the output of the
> -    # commands it is running to its stdout? This would be useful for debug.
> -    DOCKER_BUILDKIT=1 docker build -t coverity-scanner \
> -                   --secret id=coverity.token,src="$SECRET" \
> -                   -f scripts/coverity-scan/coverity-scan.docker \
> -                   scripts/coverity-scan
> +    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

Generally this script uses a "./" prefix for invoking scripts
that are within the current directory...

>      echo "Archiving sources to be analyzed..."
>      ./scripts/archive-source.sh "$SECRETDIR/qemu-sources.tgz"

...as for instance here. It would be nice to follow that convention.

>      if [ "$DRYRUN" = yes ]; then
> @@ -323,7 +333,7 @@ if [ "$DOCKER" = yes ]; then
>      # Arrange for this docker run to get access to the sources with -v.
>      # We pass through all the configuration from the outer script to the inner.
>      export COVERITY_EMAIL COVERITY_BUILD_CMD
> -    docker run -it --env COVERITY_EMAIL --env COVERITY_BUILD_CMD \
> +    tests/docker/docker.py run -it --env COVERITY_EMAIL --env COVERITY_BUILD_CMD \
>             -v "$SECRETDIR:/work" coverity-scanner \
>             ./run-coverity-scan --version "$VERSION" \
>             --description "$DESCRIPTION" $DRYRUNARG --tokenfile /work/token \

Ditto.

otherwise

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

thanks
-- PMM