Looks like the code has been this way since #774. Has something come up recently to highlight this?

Yes, konflux was behaving a bit differently for the given build (in test) and failing its check-noarch step. Anyway, it was a bug there (I relied only on kojihash which is not enough) and as going through the debugging I've found also this issue.

https://github.com/konflux-ci/rpmbuild-pipeline-environment-container/pull/130

Where can I read more about the actual failure? When is kojihash not enough?

ROK-1677 - there are inode changes which yields different hash but if compared via rpmdiff -S5TN, they are ignored.

It looks like there is some confusion about what fields rpmdiff is supposed to be checking and ignoring. The help text says the that ignore codes include N (inode), L (number of links). If that is true, then the mappings in __FILEIDX are incorrect and always have been.

In Koji we have:

             ['N', 6],
             ['L', 7],

These are supposed to be fileinfo offsets for inode and nlinks respectively (going by the rpmdiff help text). However, they are off by one (the other entries appear to be correct). Index 6 is file.nlink and index 7 is file.state.

These values have been this way since the code was added to Koji in 2009. That code came from rpmlint, which has the same values.

Actually, rpmlist appears to still work this way. When they updated the code for newer rpm, they actually propagated this. They now have

 ['N', 'nlink'],
 ['L', 'state'],

So, it looks like this has never worked the way the the help says it does. N means nlink not inode, and L means state not nlink.

Since __FILEIDX determines which file fields are compared at all, it looks like inode has never been checked.

I would be very interested in getting the actual files that failed the comparison

afaict, state is only relevant for installed rpms. It doesn't make sense to compare it for rpm files. The above seems like a very longstanding typo in rpmdiff that koji has duplicated.

Ah, I generated two rpms that differed only by inode and now I see the problem.

The kojihash changes cleared out the file data from the ignored list, but the since the inode field was not actually listed in FILEIDX, it cannot be ignored and is never cleared. So we get a discrepancy between kojihash comparison and the .differs() method.

Possible change here -- https://pagure.io/fork/mikem/koji/commits/pr4542updates
(works well for me, but I haven't adjust the unit tests yet).

Yep, works for me also now:

import koji.rpmdiff
print(koji.rpmdiff.Rpmdiff('s390x/golang-src-1.25.3-7.el10.noarch.rpm', 's390x/golang-src-1.25.3-7.el10.noarch.rpm', ignore='5TSN').kojihash())
print(koji.rpmdiff.Rpmdiff('x86/golang-src-1.25.3-7.el10.noarch.rpm', 'x86/golang-src-1.25.3-7.el10.noarch.rpm', ignore='5TSN').kojihash())
print(koji.rpmdiff.Rpmdiff('x86/golang-src-1.25.3-7.el10.noarch.rpm', 's390x/golang-src-1.25.3-7.el10.noarch.rpm', ignore='5TSN').kojihash())
# your code
1338b2b65fc0d33b0c2dc131295137b3b8fc92e3c80fd6baef3b163ce2164652
1338b2b65fc0d33b0c2dc131295137b3b8fc92e3c80fd6baef3b163ce2164652
1338b2b65fc0d33b0c2dc131295137b3b8fc92e3c80fd6baef3b163ce2164652
# current code (master)
6adf2d193095a7e31a418f449ec19233a0657b599c69120f93dc176311915674
e643b0fcd842c7260db4903ceb077766ffd465c7d5f1d4706d318b516391f380
e643b0fcd842c7260db4903ceb077766ffd465c7d5f1d4706d318b516391f380
# without ignore='S5TN'
# your code
abfa4f0b0fb885d9b2b58f1a43c61053674c298c31f848039442f33b0e2c3e34
b1591a28a7559818ecd310c4c7e6e2ef0ee90c683e255428f5bea0e80654fd57
b1591a28a7559818ecd310c4c7e6e2ef0ee90c683e255428f5bea0e80654fd57
# master
4f23eacbd28e4e368a1e0be8e4262b870fbd8f50d9131d1a4f0b438e02afa59a
616ce5429dba371f1d4179f93e163504f8510a0a70d0461e9ded9b4b1d3fddaa
616ce5429dba371f1d4179f93e163504f8510a0a70d0461e9ded9b4b1d3fddaa

Filed #4546 with my changes. Currently it includes this PR, but the parts should be easy to separate if needed.

Yeah, I think keeping these separate is probably the right way. I've filed #4547 to track the other fix

I don't love the way we have to parse the task path here, but I guess we already were and fixing that is more involved.

:thumbsup: