Fix another bug in alpine enumeration#2241
Conversation
michaelkedar
left a comment
There was a problem hiding this comment.
LGTM, with some tiny, tiny nits
andrewpollock
left a comment
There was a problem hiding this comment.
Could you add a test for something that is currently failing that no longer fails with this change?
andrewpollock
left a comment
There was a problem hiding this comment.
I'm not sufficiently familiar with this code to offer a meaningful review of this change without spending additional time to familiarize myself with it. I'm happy to defer to Michael's LGTM. I'd suggest adding some more docstrings to help unfamiliar readers get up to speed with what this code is trying to accomplish.
|
Adding tests for this is quite tough, as the old version actually works 90% of the time with no predictable way (afaik) to trigger the issue. I'll definitely add some more comments to explain what this file is doing though. |
Turns out alpine enumeration currently works by accident, relying on git to bundle the changes to package version and package revision in the same diff hunk, which is not always the case.
This caused enumeration to break for some packages (e.g. busybox https://osv.dev/vulnerability/CVE-2023-42366 , 3.16.1 only have r11).
This change fixes this by separating out the "start" search for versions and revisions, so changes to both are displayed, not just changes to the first element found (which is usually
pkgver, therefore it was relying on changes inpkgrelto be in the same diff hunk in git to be shown).