#293 Add a sanity check on remotely opened RPMs
Closed by tkopecek. Opened by puiterwijk.
puiterwijk/koji check-mutilated-rpm  into  master

Download 293.patch

This should catch issues with downloading RPMs early, so as to not break
the build in unknown ways.

Fixes: #290
Signed-off-by: Patrick Uiterwijk puiterwijk@redhat.com

A reasonable goal, but this seems like the wrong place to implement. Having that special case in a generic utility function seems wrong.

Also, this will raise a non-koji error if it fails. Those tend to be more less informative to the end user. We should trap the error and raise our own.

I'd like to see this check split out into its own function and called in the one place in kojid where this matters.

I'd personally say that adding it in a general utility function might be an advantage.
At this moment, only kojid actually uses this function, but if something else would later also use this same function, they'd get sanity checking "for free".
Splitting it into a sanityCheckRpmFd() function might be a way to improve?

Regarding the error: agreed. If we agree to a general approach, I'll rework that part.

@mikem Did that explain why I put it there, or do you still want it moved?

I did a little refactor on this way back. Here it is, rebased:
https://github.com/mikem23/koji-playground/commits/check-mutilated-rpm

Still needs to trap the rpmlib error

It doesn't work for me. hdrCheck doesn't work on whole file, just on header. I've tuned it a bit (fixed this and added koji exceptions) in #1829

Merged as part of #1829

Pull-Request has been closed by tkopecek

Metadata