Unexpected /patches VERIFY result from rpminspect

[Florian Weimer]

I've got an rpminspect failure I don't understand.  Looking at

https://osci-jenkins-1.ci.fedoraproject.org/job/fedora-ci/job/rpminspect-...

the relevant result seems to be this:

|25) glibc-upstream-2.34-18.patch touches 26 files and as many as 35 lines
|
|Result: VERIFY
|Waiver Authorization: Anyone
|
|Details:
| aarch64/arch-syscall.h           |    2 ++
| alpha/arch-syscall.h             |    1 +
| arc/arch-syscall.h               |    1 +
| arm/arch-syscall.h               |    1 +
| csky/arch-syscall.h              |    1 +
| hppa/arch-syscall.h              |    1 +
| i386/arch-syscall.h              |    2 ++
| ia64/arch-syscall.h              |    1 +
| m68k/arch-syscall.h              |    1 +
| microblaze/arch-syscall.h        |    1 +
| mips/mips32/arch-syscall.h       |    1 +
| mips/mips64/n32/arch-syscall.h   |    1 +
| mips/mips64/n64/arch-syscall.h   |    1 +
| nios2/arch-syscall.h             |    1 +
| powerpc/powerpc32/arch-syscall.h |    1 +
| powerpc/powerpc64/arch-syscall.h |    1 +
| riscv/rv32/arch-syscall.h        |    1 +
| riscv/rv64/arch-syscall.h        |    1 +
| s390/s390-32/arch-syscall.h      |    1 +
| s390/s390-64/arch-syscall.h      |    1 +
| sh/arch-syscall.h                |    1 +
| sparc/sparc32/arch-syscall.h     |    1 +
| sparc/sparc64/arch-syscall.h     |    1 +
| syscall-names.list               |    6 ++++--
| x86_64/64/arch-syscall.h         |    2 ++
| x86_64/x32/arch-syscall.h        |    2 ++
| 26 files changed, 33 insertions(+), 2 deletions(-)

Why is this VERIFY?  The patch was generated as if by “git show”, and I
do not see anything wrong with it.

Thanks,
Florian

[Aleksei Bavshin]

On 11/4/21 09:17, Florian Weimer wrote:
>  Why is this VERIFY?  The patch was generated as if by “git show”, and
> I
>  do not see anything wrong with it. 
rpminspect thinks that the patch is suspiciously large and asks you to 
confirm that it is intentional.

There's a test description in the beginning of the log:

Inspects all patches defined in the spec file and reports changes 
between builds.  At the INFO level, rpminspect reports diffstat(1) and 
patch size changes.  If thresholds are reached regarding a change in the 
patch size or the number of files the patch touches, rpminspect reports 
the change at the VERIFY level unless the comparison is for a rebase. 
The configuration file can also list patch names that rpminspect should 
ignore during the inspection.

-- 
Best regards,
Aleksei Bavshin

[Florian Weimer]

* Aleksei Bavshin:

>  On 11/4/21 09:17, Florian Weimer wrote:
> > Why is this VERIFY?  The patch was generated as if by “git show”, and I
> > do not see anything wrong with it.
>
>  rpminspect thinks that the patch is suspiciously large and asks you to
>  confirm that it is intentional.
>
>  There's a test description in the beginning of the log:
>
>  Inspects all patches defined in the spec file and reports changes
>  between builds.  At the INFO level, rpminspect reports diffstat(1) and 
>  patch size changes.  If thresholds are reached regarding a change in
>  the patch size or the number of files the patch touches, rpminspect
>  reports the change at the VERIFY level unless the comparison is for a
>  rebase. The configuration file can also list patch names that
>  rpminspect should ignore during the inspection. 
Is this a new check?  It was flagged as a regression in Jenkins.

Thanks,
Florian

[David Cantrell]

On Thu, Nov 04, 2021 at 06:51:23PM +0100, Florian Weimer wrote:
> * Aleksei Bavshin:
>
> > On 11/4/21 09:17, Florian Weimer wrote:
> >> Why is this VERIFY?  The patch was generated as if by “git show”, and I
> >> do not see anything wrong with it.
> >
> > rpminspect thinks that the patch is suspiciously large and asks you to
> > confirm that it is intentional.
> >
> > There's a test description in the beginning of the log:
> >
> > Inspects all patches defined in the spec file and reports changes
> > between builds.  At the INFO level, rpminspect reports diffstat(1) and
> > patch size changes.  If thresholds are reached regarding a change in
> > the patch size or the number of files the patch touches, rpminspect
> > reports the change at the VERIFY level unless the comparison is for a
> > rebase. The configuration file can also list patch names that
> > rpminspect should ignore during the inspection.
>
> Is this a new check?  It was flagged as a regression in Jenkins. 
It´s not new, the patches inspection has been present for a while.

Thanks,

-- 
David Cantrell <dcantrell(a)redhat.com&gt;
Red Hat, Inc. | Boston, MA | EST5EDT

[David Cantrell]

On Thu, Nov 04, 2021 at 06:51:23PM +0100, Florian Weimer wrote:
> * Aleksei Bavshin:
>
> > On 11/4/21 09:17, Florian Weimer wrote:
> >> Why is this VERIFY?  The patch was generated as if by “git show”, and I
> >> do not see anything wrong with it.
> >
> > rpminspect thinks that the patch is suspiciously large and asks you to
> > confirm that it is intentional.
> >
> > There's a test description in the beginning of the log:
> >
> > Inspects all patches defined in the spec file and reports changes
> > between builds.  At the INFO level, rpminspect reports diffstat(1) and
> > patch size changes.  If thresholds are reached regarding a change in
> > the patch size or the number of files the patch touches, rpminspect
> > reports the change at the VERIFY level unless the comparison is for a
> > rebase. The configuration file can also list patch names that
> > rpminspect should ignore during the inspection.
>
> Is this a new check?  It was flagged as a regression in Jenkins. 
I should also point out that rpminspect is configurable with regard to
what it considers a "large" patch.  The intent of the inspection is to
provide a guard for packages in maintenance mode.  For example, if a
maintainer wants to stay on a particular version even though a newer
release is available and then someone submits a PR that patches the
source and the patch is effectively the new version, something like
this patches inspection could provide the maintainer with a warning
that this is a large patch and that _may_ be something that needs
closer inspection.

It is entirely subjective and some maintainers don't really care for
it.  I think in the case of maintenance builds, it can be helpful to
catch large patches that may introduce an unwanted change and provide
another guard for the maintainer to doublecheck the work.

If rpminspect is comparing builds and determines the comparison to be
a rebase (that is, the version numbers are different), it will report
all findings in the patches inspection at the INFO level.  But if it
determines the builds compared are the same version but the release
numbers differ (that is, a maintenance update), it will report
findings in the patches inspection at the VERIFY level.

You can control the patches inspection with an rpminspect.yaml file in
your dist-git directory.  First, you can exclude specific patches from
the inspection.  You can also set the file_count_threshold (default is
20) which is the number of files a single patch modifies and you can
also set the line_count_threshold (default 5000) which is the number
of lines a single patch modifies.

Not all packages are the same, so these settings are configurable on a
per-package basis so the rpminspect results are more useful to package
maintainers.  See this section in the upstream project for
documentation on what the patches section in rpminspect.yaml can look
like:

     https://github.com/rpminspect/rpminspect/blob/master/data/generic.yaml#L646

Thanks,

-- 
David Cantrell <dcantrell(a)redhat.com&gt;
Red Hat, Inc. | Boston, MA | EST5EDT