[PATCH] coverity_scan: switch to vpath build

Paolo Bonzini posted 1 patch 3 years, 7 months ago
Test docker-quick@centos7 passed
Test docker-mingw@fedora passed
Test checkpatch passed
Test FreeBSD passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200922130806.1506324-1-pbonzini@redhat.com
Maintainers: Peter Maydell <peter.maydell@linaro.org>
There is a newer version of this series
scripts/coverity-scan/run-coverity-scan | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
[PATCH] coverity_scan: switch to vpath build
Posted by Paolo Bonzini 3 years, 7 months ago
This is the patch that has been running on the coverity cronjob
for a few weeks now.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/coverity-scan/run-coverity-scan | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/scripts/coverity-scan/run-coverity-scan b/scripts/coverity-scan/run-coverity-scan
index 6eefb4b558..7395bbfad4 100755
--- a/scripts/coverity-scan/run-coverity-scan
+++ b/scripts/coverity-scan/run-coverity-scan
@@ -380,15 +380,17 @@ export PATH="$TOOLBIN:$PATH"
 
 cd "$SRCDIR"
 
-echo "Doing make distclean..."
-make distclean
+echo "Nuking build directory..."
+rm -rf +build
+mkdir +build
+cd +build
 
 echo "Configuring..."
 # We configure with a fixed set of enables here to ensure that we don't
 # accidentally reduce the scope of the analysis by doing the build on
 # the system that's missing a dependency that we need to build part of
 # the codebase.
-./configure --disable-modules --enable-sdl --enable-gtk \
+../configure --disable-modules --enable-sdl --enable-gtk \
     --enable-opengl --enable-vte --enable-gnutls \
     --enable-nettle --enable-curses --enable-curl \
     --audio-drv-list=oss,alsa,sdl,pa --enable-virtfs \
-- 
2.26.2


Re: [PATCH] coverity_scan: switch to vpath build
Posted by Philippe Mathieu-Daudé 3 years, 7 months ago
On 9/22/20 3:08 PM, Paolo Bonzini wrote:
> This is the patch that has been running on the coverity cronjob
> for a few weeks now.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  scripts/coverity-scan/run-coverity-scan | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/scripts/coverity-scan/run-coverity-scan b/scripts/coverity-scan/run-coverity-scan
> index 6eefb4b558..7395bbfad4 100755
> --- a/scripts/coverity-scan/run-coverity-scan
> +++ b/scripts/coverity-scan/run-coverity-scan
> @@ -380,15 +380,17 @@ export PATH="$TOOLBIN:$PATH"
>  
>  cd "$SRCDIR"
>  
> -echo "Doing make distclean..."
> -make distclean
> +echo "Nuking build directory..."
> +rm -rf +build

^ OK

v Why prepend '+' to the dirname?

> +mkdir +build
> +cd +build
>  
>  echo "Configuring..."
>  # We configure with a fixed set of enables here to ensure that we don't
>  # accidentally reduce the scope of the analysis by doing the build on
>  # the system that's missing a dependency that we need to build part of
>  # the codebase.
> -./configure --disable-modules --enable-sdl --enable-gtk \
> +../configure --disable-modules --enable-sdl --enable-gtk \
>      --enable-opengl --enable-vte --enable-gnutls \
>      --enable-nettle --enable-curses --enable-curl \
>      --audio-drv-list=oss,alsa,sdl,pa --enable-virtfs \
> 


Re: [PATCH] coverity_scan: switch to vpath build
Posted by Peter Maydell 3 years, 7 months ago
On Tue, 22 Sep 2020 at 14:08, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> This is the patch that has been running on the coverity cronjob
> for a few weeks now.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  scripts/coverity-scan/run-coverity-scan | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/coverity-scan/run-coverity-scan b/scripts/coverity-scan/run-coverity-scan
> index 6eefb4b558..7395bbfad4 100755
> --- a/scripts/coverity-scan/run-coverity-scan
> +++ b/scripts/coverity-scan/run-coverity-scan
> @@ -380,15 +380,17 @@ export PATH="$TOOLBIN:$PATH"
>
>  cd "$SRCDIR"
>
> -echo "Doing make distclean..."
> -make distclean
> +echo "Nuking build directory..."
> +rm -rf +build

As Philippe points out, odd name choice.

It might also be nice to steal the logic from configure
that avoids blowing away the build directory if it
wasn't created by this script in the first place.

> +mkdir +build
> +cd +build

I think this 'cd' will break use of the --results-tarball
option with a relative path (eg "--results-tarball my-tarball.tgz")
because it will now end up interpreted relative to the build
subdir rather than relative to the source directory.

>  echo "Configuring..."
>  # We configure with a fixed set of enables here to ensure that we don't
>  # accidentally reduce the scope of the analysis by doing the build on
>  # the system that's missing a dependency that we need to build part of
>  # the codebase.
> -./configure --disable-modules --enable-sdl --enable-gtk \
> +../configure --disable-modules --enable-sdl --enable-gtk \
>      --enable-opengl --enable-vte --enable-gnutls \
>      --enable-nettle --enable-curses --enable-curl \
>      --audio-drv-list=oss,alsa,sdl,pa --enable-virtfs \

This comment at the top of the script:

# This script assumes that you're running it from a QEMU source
# tree, and that tree is a fresh clean one, because we do an in-tree
# build. (This is necessary so that the filenames that the Coverity
# Scan server sees are relative paths that match up with the component
# regular expressions it uses; an out-of-tree build won't work for this.)

is now out of date and needs rephrasing.

PS: on the subject of component regexes, they seem to have
vanished from the Coverity website. I don't suppose you have
a backup of them, do you ? (I have a list of what the component
names were, but not the associated regexes.)

thanks
-- PMM

Re: [PATCH] coverity_scan: switch to vpath build
Posted by Philippe Mathieu-Daudé 3 years, 7 months ago
On 9/22/20 3:18 PM, Peter Maydell wrote:
> On Tue, 22 Sep 2020 at 14:08, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>> This is the patch that has been running on the coverity cronjob
>> for a few weeks now.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  scripts/coverity-scan/run-coverity-scan | 8 +++++---
>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/scripts/coverity-scan/run-coverity-scan b/scripts/coverity-scan/run-coverity-scan
>> index 6eefb4b558..7395bbfad4 100755
>> --- a/scripts/coverity-scan/run-coverity-scan
>> +++ b/scripts/coverity-scan/run-coverity-scan
>> @@ -380,15 +380,17 @@ export PATH="$TOOLBIN:$PATH"
>>
>>  cd "$SRCDIR"
>>
>> -echo "Doing make distclean..."
>> -make distclean
>> +echo "Nuking build directory..."
>> +rm -rf +build
> 
> As Philippe points out, odd name choice.
> 
> It might also be nice to steal the logic from configure
> that avoids blowing away the build directory if it
> wasn't created by this script in the first place.
> 
>> +mkdir +build
>> +cd +build
> 
> I think this 'cd' will break use of the --results-tarball
> option with a relative path (eg "--results-tarball my-tarball.tgz")
> because it will now end up interpreted relative to the build
> subdir rather than relative to the source directory.
> 
>>  echo "Configuring..."
>>  # We configure with a fixed set of enables here to ensure that we don't
>>  # accidentally reduce the scope of the analysis by doing the build on
>>  # the system that's missing a dependency that we need to build part of
>>  # the codebase.
>> -./configure --disable-modules --enable-sdl --enable-gtk \
>> +../configure --disable-modules --enable-sdl --enable-gtk \
>>      --enable-opengl --enable-vte --enable-gnutls \
>>      --enable-nettle --enable-curses --enable-curl \
>>      --audio-drv-list=oss,alsa,sdl,pa --enable-virtfs \
> 
> This comment at the top of the script:
> 
> # This script assumes that you're running it from a QEMU source
> # tree, and that tree is a fresh clean one, because we do an in-tree
> # build. (This is necessary so that the filenames that the Coverity
> # Scan server sees are relative paths that match up with the component
> # regular expressions it uses; an out-of-tree build won't work for this.)
> 
> is now out of date and needs rephrasing.

Or we can keep it as it, since commit dedad027205
("configure: add support for pseudo-"in source tree" builds")
already create a 'build/' directory.

> 
> PS: on the subject of component regexes, they seem to have
> vanished from the Coverity website. I don't suppose you have
> a backup of them, do you ? (I have a list of what the component
> names were, but not the associated regexes.)
> 
> thanks
> -- PMM
> 


Re: [PATCH] coverity_scan: switch to vpath build
Posted by Peter Maydell 3 years, 7 months ago
On Tue, 22 Sep 2020 at 14:27, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> On 9/22/20 3:18 PM, Peter Maydell wrote:
> > This comment at the top of the script:
> >
> > # This script assumes that you're running it from a QEMU source
> > # tree, and that tree is a fresh clean one, because we do an in-tree
> > # build. (This is necessary so that the filenames that the Coverity
> > # Scan server sees are relative paths that match up with the component
> > # regular expressions it uses; an out-of-tree build won't work for this.)
> >
> > is now out of date and needs rephrasing.
>
> Or we can keep it as it, since commit dedad027205
> ("configure: add support for pseudo-"in source tree" builds")
> already create a 'build/' directory.

No, because even with the pseudo-in-tree build the paths will
no longer be the relative ones that the real pre-meson in-tree
build had, so the comment is no longer correct.

thanks
-- PMM

Re: [PATCH] coverity_scan: switch to vpath build
Posted by Paolo Bonzini 3 years, 7 months ago
On 22/09/20 15:18, Peter Maydell wrote:
>> +echo "Nuking build directory..."
>> +rm -rf +build
>
> As Philippe points out, odd name choice.
> It might also be nice to steal the logic from configure
> that avoids blowing away the build directory if it
> wasn't created by this script in the first place.

I thought that was a bit overkill for this usage, and just used a
slightly odd name to limit the chance of doing damage.

> PS: on the subject of component regexes, they seem to have
> vanished from the Coverity website. I don't suppose you have
> a backup of them, do you ? (I have a list of what the component
> names were, but not the associated regexes.)

I did have a backup and I've now tried to update them again.
It passed, here they are:

alpha	(/qemu)?((/include)?/hw/alpha/.*|/target/alpha/.*)
arm     (/qemu)?((/include)?/hw/arm/.*|(/include)?/hw/.*/(arm|allwinner-a10|bcm28|digic|exynos|imx|omap|stellaris|pxa2xx|versatile|zynq|cadence).*|/hw/net/xgmac.c|/hw/ssi/xilinx_spips.c|/target/arm/.*)
avr     (/qemu)?((/include)?/hw/avr/.*|/target/avr/.*)
cris    (/qemu)?((/include)?/hw/cris/.*|/target/cris/.*)
hppa    (/qemu)?(/target/hppa/.*)
i386    (/qemu)?((/include)?/hw/i386/.*|/target/i386/.*|/hw/intc/[^/]*apic[^/]*\.c)
lm32    (/qemu)?((/include)?/hw/lm32/.*|/target/lm32/.*|/hw/.*/(milkymist|lm32).*)
m68k    (/qemu)?((/include)?/hw/m68k/.*|/target/m68k/.*|(/include)?/hw(/.*)?/mcf.*)
microblaze      (/qemu)?((/include)?/hw/microblaze/.*|/target/microblaze/.*)
mips    (/qemu)?((/include)?/hw/mips/.*|/target/mips/.*)
nios2   (/qemu)?((/include)?/hw/nios2/.*|/target/nios2/.*)
ppc     (/qemu)?((/include)?/hw/ppc/.*|/target/ppc/.*|/hw/pci-host/(uninorth.*|dec.*|prep.*|ppc.*)|/hw/misc/macio/.*|(/include)?/hw/.*/(xics|openpic|spapr).*)
riscv   (/qemu)?((/include)?/hw/riscv/.*|/target/riscv/.*)
rx      (/qemu)?((/include)?/hw/rx/.*|/target/rx/.*)
s390    (/qemu)?((/include)?/hw/s390x/.*|/target/s390x/.*|/hw/.*/s390_.*)
sh4     (/qemu)?((/include)?/hw/sh4/.*|/target/sh4/.*)
sparc   (/qemu)?((/include)?/hw/sparc(64)?.*|/target/sparc/.*|/hw/.*/grlib.*|/hw/display/cg3.c)
tilegx  (/qemu)?(/target/tilegx/.*)
tricore         (/qemu)?((/include)?/hw/tricore/.*|/target/tricore/.*)
unicore32       (/qemu)?((/include)?/hw/unicore32/.*|/target/unicore32/.*)
9pfs    (/qemu)?(/hw/9pfs/.*|/fsdev/.*)
audio   (/qemu)?((/include)?/(audio|hw/audio)/.*)
block   (/qemu)?(/block.*|(/include?)(/hw)?/(block|storage-daemon)/.*|(/include)?/hw/ide/.*|/qemu-(img|io).*|/util/(aio|async|thread-pool).*)
char    (/qemu)?(/qemu-char\.c|/include/sysemu/char\.h|(/include)?/hw/char/.*)
capstone        (/qemu)?(/capstone/.*)
crypto  (/qemu)?((/include)?/crypto/.*|/hw/.*/crypto.*)
disas   (/qemu)?((/include)?/disas.*)
fpu     (/qemu)?((/include)?(/fpu|/libdecnumber)/.*)
io      (/qemu)?((/include)?/io/.*)
ipmi    (/qemu)?((/include)?/hw/ipmi/.*)
libvixl         (/qemu)?(/disas/libvixl/.*)
migration       (/qemu)?((/include)?/migration/.*)
monitor         (/qemu)?(/qapi.*|/qobject/.*|/monitor\..*|/[hq]mp\..*)
nbd     (/qemu)?(/nbd/.*|/include/block/nbd.*|/qemu-nbd\.c)
net     (/qemu)?((/include)?(/hw)?/(net|rdma)/.*)
pci     (/qemu)?(/hw/pci.*|/include/hw/pci.*)
qemu-ga         (/qemu)?(/qga/.*)
scsi    (/qemu)?(/scsi/.*|/hw/scsi/.*|/include/hw/scsi/.*)
slirp   (/qemu)?(/.*slirp.*)
tcg     (/qemu)?(/accel/tcg/.*|/replay/.*|/(.*/)?softmmu.*)
trace   (/qemu)?(/.*trace.*\.[ch])
ui      (/qemu)?((/include)?(/ui|/hw/display|/hw/input)/.*)
usb     (/qemu)?(/hw/usb/.*|/include/hw/usb/.*)
user    (/qemu)?(/linux-user/.*|/bsd-user/.*|/user-exec\.c|/thunk\.c|/include/exec/user/.*)
util    (/qemu)?(/util/.*|/include/qemu/.*)
xen     (/qemu)?(.*/xen.*)
(headers)       (/qemu)?(/include/.*)
virtiofsd       (/qemu)?(/tools/virtiofsd/.*)

Adding the "Other" component fails but, this time, it didn't blow up
and delete everything else.

Paolo