diff mbox series

coverity_scan: switch to vpath build

Message ID 20200922130806.1506324-1-pbonzini@redhat.com
State Superseded
Headers show
Series coverity_scan: switch to vpath build | expand

Commit Message

Paolo Bonzini Sept. 22, 2020, 1:08 p.m. UTC
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(-)

Comments

Philippe Mathieu-Daudé Sept. 22, 2020, 1:12 p.m. UTC | #1
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 \
>
Peter Maydell Sept. 22, 2020, 1:18 p.m. UTC | #2
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
Philippe Mathieu-Daudé Sept. 22, 2020, 1:27 p.m. UTC | #3
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

>
Peter Maydell Sept. 22, 2020, 1:31 p.m. UTC | #4
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
Paolo Bonzini Sept. 22, 2020, 4:35 p.m. UTC | #5
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
diff mbox series

Patch

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 \