Skip to content

Commit ca51e49

Browse files
cfs/status: Implement bootloader-specific sorting
Update `get_sorted_type1_boot_entries_helper` to implement sorting logic based on bootloader type - systemd-boot: Sort by sort-key (using BLSConfig::cmp which handles sort-key ascending, then version descending) - GRUB: Sort by filename in descending order (ignoring sort-key fields) Unit Tests generated by ClaudeCode (Opus) Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
1 parent 57d0036 commit ca51e49

1 file changed

Lines changed: 220 additions & 7 deletions

File tree

crates/lib/src/bootc_composefs/status.rs

Lines changed: 220 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -243,34 +243,47 @@ fn get_sorted_grub_uki_boot_entries_helper<'a>(
243243
parse_grub_menuentry_file(str)
244244
}
245245

246+
/// Get sorted boot entries
247+
/// The sort here is done in terms of what will be shown on the boot menu
248+
/// For systemd-boot, the entries are sorted by `sort-key`
249+
/// For grub, the entries are sorted by the filename in descending order
246250
pub(crate) fn get_sorted_type1_boot_entries(
247251
boot_dir: &Dir,
248252
ascending: bool,
249253
) -> Result<Vec<BLSConfig>> {
250-
get_sorted_type1_boot_entries_helper(boot_dir, ascending, false)
254+
let bootloader = get_bootloader()?;
255+
get_sorted_type1_boot_entries_helper(boot_dir, ascending, false, bootloader)
251256
}
252257

258+
/// Same as [`get_sorted_type1_boot_entries`], but returns staged entries
259+
/// See [`get_sorted_type1_boot_entries`] for more details
253260
pub(crate) fn get_sorted_staged_type1_boot_entries(
254261
boot_dir: &Dir,
255262
ascending: bool,
256263
) -> Result<Vec<BLSConfig>> {
257-
get_sorted_type1_boot_entries_helper(boot_dir, ascending, true)
264+
let bootloader = get_bootloader()?;
265+
get_sorted_type1_boot_entries_helper(boot_dir, ascending, true, bootloader)
258266
}
259267

260268
#[context("Getting sorted Type1 boot entries")]
261269
fn get_sorted_type1_boot_entries_helper(
262270
boot_dir: &Dir,
263271
ascending: bool,
264272
get_staged_entries: bool,
273+
bootloader: crate::spec::Bootloader,
265274
) -> Result<Vec<BLSConfig>> {
266-
let mut all_configs = vec![];
275+
#[derive(Debug)]
276+
struct ConfigWithFilename {
277+
config: BLSConfig,
278+
filename: String,
279+
}
267280

268281
let dir = match get_staged_entries {
269282
true => {
270283
let dir = boot_dir.open_dir_optional(TYPE1_ENT_PATH_STAGED)?;
271284

272285
let Some(dir) = dir else {
273-
return Ok(all_configs);
286+
return Ok(vec![]);
274287
};
275288

276289
dir.read_dir(".")?
@@ -279,6 +292,8 @@ fn get_sorted_type1_boot_entries_helper(
279292
false => boot_dir.read_dir(TYPE1_ENT_PATH)?,
280293
};
281294

295+
let mut configs_with_filenames = vec![];
296+
282297
for entry in dir {
283298
let entry = entry?;
284299

@@ -302,12 +317,31 @@ fn get_sorted_type1_boot_entries_helper(
302317

303318
let config = parse_bls_config(&contents).context("Parsing bls config")?;
304319

305-
all_configs.push(config);
320+
configs_with_filenames.push(ConfigWithFilename {
321+
config,
322+
filename: file_name.to_string(),
323+
});
306324
}
307325

308-
all_configs.sort_by(|a, b| if ascending { a.cmp(b) } else { b.cmp(a) });
326+
// Sort based on bootloader type
327+
configs_with_filenames.sort_by(|a, b| {
328+
let ord = match bootloader {
329+
// For systemd-boot sort by sort-key
330+
Bootloader::Systemd => a.config.cmp(&b.config),
331+
// For grub, sort by filename in descending order
332+
Bootloader::Grub => b.filename.cmp(&a.filename),
333+
Bootloader::None => {
334+
unreachable!("Bootloader checked during installation should not have been none")
335+
}
336+
};
309337

310-
Ok(all_configs)
338+
if ascending { ord } else { ord.reverse() }
339+
});
340+
341+
Ok(configs_with_filenames
342+
.into_iter()
343+
.map(|c| c.config)
344+
.collect())
311345
}
312346

313347
pub(crate) fn list_type1_entries(boot_dir: &Dir) -> Result<Vec<BootloaderEntry>> {
@@ -979,6 +1013,10 @@ async fn composefs_deployment_status_from(
9791013
mod tests {
9801014
use cap_std_ext::{cap_std, dirext::CapStdExtDirExt};
9811015

1016+
use crate::bootc_composefs::boot::{
1017+
FILENAME_PRIORITY_PRIMARY, FILENAME_PRIORITY_SECONDARY, primary_sort_key,
1018+
secondary_sort_key, type1_entry_conf_file_name,
1019+
};
9821020
use crate::parsers::{bls_config::BLSConfigType, grub_menuconfig::MenuentryBody};
9831021

9841022
use super::*;
@@ -1217,4 +1255,179 @@ mod tests {
12171255

12181256
Ok(())
12191257
}
1258+
1259+
#[test]
1260+
fn test_get_sorted_type1_boot_entries_helper_systemd() -> Result<()> {
1261+
let tempdir = cap_std_ext::cap_tempfile::tempdir(cap_std::ambient_authority())?;
1262+
1263+
// Create entries with different sort-keys for systemd-boot testing
1264+
let entry1 = format!(
1265+
r#"
1266+
title Fedora Linux (1.0.0)
1267+
version 1.0.0
1268+
sort-key {}
1269+
linux /boot/vmlinuz
1270+
initrd /boot/initramfs.img
1271+
"#,
1272+
secondary_sort_key("fedora")
1273+
);
1274+
1275+
let entry2 = format!(
1276+
r#"
1277+
title Fedora Linux (2.0.0)
1278+
version 2.0.0
1279+
sort-key {}
1280+
linux /boot/vmlinuz
1281+
initrd /boot/initramfs.img
1282+
"#,
1283+
primary_sort_key("fedora")
1284+
);
1285+
1286+
let entry3 = format!(
1287+
r#"
1288+
title Fedora Linux (1.5.0)
1289+
version 1.5.0
1290+
sort-key {}
1291+
linux /boot/vmlinuz
1292+
initrd /boot/initramfs.img
1293+
"#,
1294+
primary_sort_key("fedora")
1295+
);
1296+
1297+
tempdir.create_dir_all("loader/entries")?;
1298+
1299+
// Use realistic filenames as used in production
1300+
let filename1 = type1_entry_conf_file_name("fedora", "1.0.0", FILENAME_PRIORITY_SECONDARY);
1301+
let filename2 = type1_entry_conf_file_name("fedora", "2.0.0", FILENAME_PRIORITY_PRIMARY);
1302+
let filename3 = type1_entry_conf_file_name("fedora", "1.5.0", FILENAME_PRIORITY_PRIMARY);
1303+
1304+
tempdir.atomic_write(format!("loader/entries/{}", filename1), entry1)?;
1305+
tempdir.atomic_write(format!("loader/entries/{}", filename2), entry2)?;
1306+
tempdir.atomic_write(format!("loader/entries/{}", filename3), entry3)?;
1307+
1308+
// Test systemd-boot sorting (by sort-key, then by version in descending order)
1309+
let result = get_sorted_type1_boot_entries_helper(
1310+
&tempdir,
1311+
true,
1312+
false,
1313+
crate::spec::Bootloader::Systemd,
1314+
)?;
1315+
1316+
assert_eq!(result.len(), 3);
1317+
// With ascending=true, primary sort-key (bootc-fedora-0) should come before secondary (bootc-fedora-1)
1318+
// Within primary sort-key, version 2.0.0 should come before 1.5.0 (descending version order)
1319+
assert_eq!(
1320+
result[0].sort_key.as_ref().unwrap(),
1321+
&primary_sort_key("fedora")
1322+
);
1323+
assert_eq!(result[0].version(), "2.0.0".into()); // Entry 2 (version 2.0.0)
1324+
assert_eq!(
1325+
result[1].sort_key.as_ref().unwrap(),
1326+
&primary_sort_key("fedora")
1327+
);
1328+
assert_eq!(result[1].version(), "1.5.0".into()); // Entry 3 (version 1.5.0)
1329+
assert_eq!(
1330+
result[2].sort_key.as_ref().unwrap(),
1331+
&secondary_sort_key("fedora")
1332+
);
1333+
assert_eq!(result[2].version(), "1.0.0".into()); // Entry 1 (version 1.0.0)
1334+
1335+
// Test descending order
1336+
let result = get_sorted_type1_boot_entries_helper(
1337+
&tempdir,
1338+
false,
1339+
false,
1340+
crate::spec::Bootloader::Systemd,
1341+
)?;
1342+
1343+
assert_eq!(result.len(), 3);
1344+
// With ascending=false, secondary sort-key should come before primary
1345+
assert_eq!(
1346+
result[0].sort_key.as_ref().unwrap(),
1347+
&secondary_sort_key("fedora")
1348+
);
1349+
assert_eq!(result[0].version(), "1.0.0".into()); // Entry 1
1350+
assert_eq!(
1351+
result[1].sort_key.as_ref().unwrap(),
1352+
&primary_sort_key("fedora")
1353+
);
1354+
assert_eq!(result[1].version(), "1.5.0".into()); // Entry 3
1355+
assert_eq!(
1356+
result[2].sort_key.as_ref().unwrap(),
1357+
&primary_sort_key("fedora")
1358+
);
1359+
assert_eq!(result[2].version(), "2.0.0".into()); // Entry 2
1360+
1361+
Ok(())
1362+
}
1363+
1364+
#[test]
1365+
fn test_get_sorted_type1_boot_entries_helper_grub() -> Result<()> {
1366+
let tempdir = cap_std_ext::cap_tempfile::tempdir(cap_std::ambient_authority())?;
1367+
1368+
// Create entries with sort-keys but GRUB ignores them and sorts by filename
1369+
let entry1 = format!(
1370+
r#"
1371+
title Fedora Linux (41.20251125.0)
1372+
version 41.20251125.0
1373+
sort-key {}
1374+
linux /boot/vmlinuz
1375+
initrd /boot/initramfs.img
1376+
"#,
1377+
secondary_sort_key("fedora")
1378+
);
1379+
1380+
let entry2 = format!(
1381+
r#"
1382+
title Fedora Linux (42.20251125.0)
1383+
version 42.20251125.0
1384+
sort-key {}
1385+
linux /boot/vmlinuz
1386+
initrd /boot/initramfs.img
1387+
"#,
1388+
primary_sort_key("fedora")
1389+
);
1390+
1391+
tempdir.create_dir_all("loader/entries")?;
1392+
1393+
// Use realistic filenames - GRUB will sort by these, not sort-key
1394+
let filename1 =
1395+
type1_entry_conf_file_name("fedora", "41.20251125.0", FILENAME_PRIORITY_SECONDARY);
1396+
let filename2 =
1397+
type1_entry_conf_file_name("fedora", "42.20251125.0", FILENAME_PRIORITY_PRIMARY);
1398+
1399+
tempdir.atomic_write(format!("loader/entries/{}", filename1), entry1)?;
1400+
tempdir.atomic_write(format!("loader/entries/{}", filename2), entry2)?;
1401+
1402+
// Test GRUB sorting (by filename - note that GRUB uses b.filename.cmp(&a.filename) for descending by default)
1403+
let result = get_sorted_type1_boot_entries_helper(
1404+
&tempdir,
1405+
true,
1406+
false,
1407+
crate::spec::Bootloader::Grub,
1408+
)?;
1409+
1410+
assert_eq!(result.len(), 2);
1411+
// With ascending=true for GRUB, we reverse the default descending filename order
1412+
// Filenames: bootc_fedora-40.20251125.0-1.conf, bootc_fedora-41.20251125.0-0.conf, bootc_fedora-42.20251125.0-1.conf
1413+
// Ascending filename order should be: 42-1, 41-0
1414+
assert_eq!(result[0].version(), "42.20251125.0".into());
1415+
assert_eq!(result[1].version(), "41.20251125.0".into());
1416+
1417+
// Test descending order (GRUB's default filename sorting)
1418+
let result = get_sorted_type1_boot_entries_helper(
1419+
&tempdir,
1420+
false,
1421+
false,
1422+
crate::spec::Bootloader::Grub,
1423+
)?;
1424+
1425+
assert_eq!(result.len(), 2);
1426+
// With ascending=false for GRUB, filenames should be sorted in descending order
1427+
// Descending filename order should be: 42-1, 41-0, 40-1
1428+
assert_eq!(result[0].version(), "41.20251125.0".into());
1429+
assert_eq!(result[1].version(), "42.20251125.0".into());
1430+
1431+
Ok(())
1432+
}
12201433
}

0 commit comments

Comments
 (0)