Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 37 additions & 1 deletion src/packagesystem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,12 @@ fn parse_evr(pkg: &str) -> Module {
(nevra.name().to_string(), nevra.evr().to_string())
};

let (name, _) = name_str.split_once('-').unwrap_or((&name_str, ""));
// Only cut the packages name that we know
let (name, _) = if ["grub2", "shim"].iter().any(|p| name_str.starts_with(p)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would do something like

while true
    next = split.next()
    if next.starts_with_number() == false
        name.join("-", next)
    else
        break
    fi
endwhile

then is next epoch or version, etc.

name_str.split_once('-').unwrap_or((&name_str, ""))
} else {
(name_str.as_str(), "")
};
Comment on lines +120 to +124

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

This change correctly handles package names that are not grub2 or shim. However, the same incorrect parsing logic that was just fixed here still exists earlier in this function for packages that don't end with an architecture suffix (lines 106-112).

The logic pkg.split_once('-') on line 107 will incorrectly parse a package like test-bootupd-payload-1.0-1.

To ensure consistent and correct parsing, consider refactoring this function to apply the same corrected logic for both cases. It might be possible to unify the parsing logic for both branches.

On a related note, for better performance and code style, you could define ["grub2", "shim"] as a static or const slice at the module level to avoid re-creating it on each call.

Module {
name: name.to_string(),
rpm_evr,
Expand Down Expand Up @@ -181,6 +186,37 @@ pub(crate) fn compare_package_versions(a: &str, b: &str) -> Ordering {
mod tests {
use super::*;

#[test]
fn test_parse_evr() {
let cases = [
// Case from this PR
(
"test-bootupd-payload-1.0-1.x86_64",
"test-bootupd-payload",
"1.0-1",
),
// grub2/shim cases
(
"grub2-efi-x64-1:2.06-95.fc38.x86_64",
"grub2",
"1:2.06-95.fc38",
),
("shim-x64-15.6-2.x86_64", "shim", "15.6-2"),
// Case without arch suffix
("grub2-1:2.12-28.fc42", "grub2", "1:2.12-28.fc42"),
];

for &(input, expected_name, expected_evr) in &cases {
assert_eq!(
Module {
name: expected_name.to_string(),
rpm_evr: expected_evr.to_string(),
},
parse_evr(input)
);
}
}

#[test]
fn test_parse_rpmout() {
let testdata = "grub2-efi-x64-1:2.06-95.fc38.x86_64,1681321788 grub2-efi-x64-1:2.06-95.fc38.x86_64,1681321788 shim-x64-15.6-2.x86_64,1657222566 shim-x64-15.6-2.x86_64,1657222566 shim-x64-15.6-2.x86_64,1657222566";
Expand Down