It seems strange to keep __fileIteratorToDict when we're not dealing with file iterators. This private function is only used in this one spot. I suggest we replace it with a new function that accepts the rpm header and returns the dict that we expect.

Well hmm.

I mocked up my idea here. This keeps the special case isolated in one spot. Sort of. The trouble is that we still have polymorphism in the files dict values: a list of file data vs an rpm.file object.

It looks like this code is going to fail when it tries to compare two file objects, as the FILEIDX values are no longer relevant.

I think the proper fix is going to be a little more involved. Most likely rewrite all the compare code around the new rpm.file interface and write some backwards compatible code to simulate that interface.

I was curious what rpmlint had done about this and I see they haven't updated these calls.

How pressing is this?

On F37 it throws additional warnings but I've not found if rpm is planning to drop the support soon. (Moved to next release at least)

As I alluded above, the rpm.file interface is not compatible with the compare code we have in rpmdiff.py. If I try to use this class to compare an rpm with itself, I get the following traceback.

$ PYTHONPATH=~/Devel/koji/koji python3 koji-rpmdiff.py /home/mikem/kubevirt-virtctl-4.13.0-918.el9.x86_64.rpm /home/mikem/kubevirt-virtctl-4.13.0-918.el9.x86_64.rpm
Traceback (most recent call last):
  File "/home/mikem/Devel/scripts/koji-rpmdiff.py", line 7, in <module>
    d = koji.rpmdiff.Rpmdiff(sys.argv[1], sys.argv[2], ignore='S5TN')
  File "/home/mikem/Devel/koji/koji/koji/rpmdiff.py", line 144, in __init__
    old_file[entry[1]] = None
TypeError: 'rpm.file' object does not support item assignment

Trivial test script:

$ cat koji-rpmdiff.py
#!/usr/bin/python3
import sys
import koji.rpmdiff
d = koji.rpmdiff.Rpmdiff(sys.argv[1], sys.argv[2], ignore='S5TN')
print(f'Differs: {d.differs()}')

If we are embracing the new way, I think it makes the most sense to rewrite the checks around the rpm.file interface and provide a backwards compatible layer for older rpm libs.

Also, looks like rpm has already dropped this upstream (last April)

commit 742be88cd97d95bae356a93f17ec0595b610cee7
Author: Panu Matilainen <pmatilai@redhat.com>
Date:   Fri Apr 8 12:18:33 2022 +0300
    Drop the klunky and ugly rpm.fi python binding finally
    Update our lone test-case to use rpm.files instead

If we are embracing the new way, I think it makes the most sense to rewrite the checks around the rpm.file interface and provide a backwards compatible layer for older rpm libs.

Hmm we may want to preserve the list interface so that kojihash will continue to work correctly.

Also, it would be good to add a unit test for this code

Possible update on same branch

I see a couple unit test failures, but neither seems to be related to this PR

:thumbsup:

Commit 0034e9eb fixes this pull-request