fix: systemd Checker

[BreadGenie]

#1227 (comment)

systemd checker wasn't working at #1155 and was fixed in #1156 but we don't need that fix anymore due to the changes in #1227.
Also enabled the commented out test for systemd v246.

[codecov-commenter]

Codecov Report

Merging #1289 (e53a72c) into main (2d69f84) will increase coverage by 1.35%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1289      +/-   ##
==========================================
+ Coverage   78.96%   80.31%   +1.35%     
==========================================
  Files         262      264       +2     
  Lines        4801     4801              
  Branches      578      575       -3     
==========================================
+ Hits         3791     3856      +65     
+ Misses        856      797      -59     
+ Partials      154      148       -6     

Flag	Coverage Δ
longtests	80.31% <100.00%> (+1.35%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
test/test_data/systemd.py	100.00% <ø> (ø)
cve_bin_tool/checkers/systemd.py	100.00% <100.00%> (ø)
cve_bin_tool/async_utils.py	91.66% <0.00%> (-2.09%)	⬇️
cve_bin_tool/checkers/__init__.py	95.45% <0.00%> (ø)
cve_bin_tool/checkers/pigz.py	100.00% <0.00%> (ø)
test/test_data/pigz.py	100.00% <0.00%> (ø)
cve_bin_tool/cvedb.py	83.03% <0.00%> (+0.60%)	⬆️
cve_bin_tool/version_scanner.py	77.30% <0.00%> (+1.41%)	⬆️
cve_bin_tool/cli.py	78.31% <0.00%> (+3.01%)	⬆️
cve_bin_tool/checkers/glibc.py	100.00% <0.00%> (+4.16%)	⬆️
... and 4 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2d69f84...e53a72c. Read the comment docs.

[terriko]

Looking good. Let's try to help out our future selves with pre-documenting the slightly unusual strings for systemd.

[terriko]

Let's put a note right in the checker somewhere (maybe here at the bottom) describing the way systemd's version patterns occur, preferably showing the list of strings like was done in #1155. This is just to help us out in the future in case further refactoring breaks this again.

Equally, we could do something more clever with getting all possible results and sorting them explicitly to make this more robust against changes. The original cve-bin-tool code actually had something like that in it.

[BreadGenie]

Wrote some explanation on what's happening.

Also it seems like the changes in #1227 didn't actually detect the versions in the package from #1155 (but it did work on packages of same version but different releases from Fedora 33). So I made changes in the first regex pattern.
I didn't want to bring back what we did in the older fix(reversing the list), since it means we can't use any other utility functions and would have to split, sort and detect versions in the strings in here (due to the changes in #1227). If there was no other way or if systemd suddenly decides to change the way they present their version strings then we might have to do it that way.

But fortunately the changes I made in the 1st pattern detects the correct version string.

[BreadGenie]

I have also added that certain package as a condensed download.

[terriko]

I don't think we'd actually have to split, you'd just have to use re.findall for systemd and then sort the results. (You'd do this in an overridden get_version() function so we weren't wasting time doing findall on other checkers.) But I think what you've got right now works and the comment is clear about the multiple version strings, so let's go with the cleaner solution first and save over-riding idea only if they change the sorting.

[BreadGenie]

Aah. That could've been more readable than the current implementation.
Using findall didn't pass through my mind.

[BreadGenie]

I wrote the commit messages with "docs" as prefix since I'm not sure what to write instead. (Comments maybe? 😅 )

[Molkree]

I wrote the commit messages with "docs" as prefix

Offtopic
I guess you discussed prefixes internally, is it just to make commit history more readable? Is it a convention I can read about somewhere? Is it from conventionalcommits.org? Do you want to add something like release-drafter if commits are structured?

[BreadGenie]

I guess you discussed prefixes internally, is it just to make commit history more readable?

Yep, we were trying out conventional commits to make the commit history more readable. We made the decision during a weekly meet and decided we'll try it out. And I think it's time we add it to the docs about conventional commits in the README.

Is it a convention I can read about somewhere? Is it from conventionalcommits.org?

Yep.

Do you want to add something like release-drafter if commits are structured?

Not sure if we'll be adding this but looks really helpful for @terriko .

[terriko]

I don't think we'd actually have to split, you'd just have to use re.findall for systemd and then sort the results. (You'd do this in an overridden get_version() function so we weren't wasting time doing findall on other checkers.) But I think what you've got right now works and the comment is clear about the multiple version strings, so let's go with the cleaner solution first and save over-riding idea only if they change the sorting.