From 3a3fac21f3dabcb0876fd6b866789c2818307490 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 25 Jun 2026 21:04:06 -0400 Subject: [PATCH] erofs: Switch default format version from V2 to V1 This is one of the goals in getting to composefs-rs 1.0. V1 is compatible with both C mkcomposefs 1.0.8 and composefs-rs, making it the better default for interoperability. V2 remains available for users who need it. For backward compatibility, old repositories that lack an explicit erofs_formats field in meta.json continue to use V2 via a separate serde default (legacy_erofs_default), ensuring they are not silently reinterpreted as V1. Tests that validate specific V2 behavior (directory entry assertions without whiteout stubs, V2-only OCI image refs, digest stability pinning) now use V2 explicitly. Closes: https://github.com/composefs/composefs-rs/issues/327 Assisted-by: opencode (Claude claude-opus-4-6) Signed-off-by: Colin Walters --- crates/composefs-ctl/src/lib.rs | 8 +- .../src/tests/cli.rs | 12 +- .../src/tests/digest_stability.rs | 193 +++++++++--------- crates/composefs-oci/src/lib.rs | 5 +- crates/composefs/src/erofs/format.rs | 13 +- crates/composefs/src/erofs/reader.rs | 55 +++-- crates/composefs/src/repository.rs | 45 ++-- crates/composefs/tests/mkfs.rs | 129 ++++++------ .../tests/snapshots/mkfs__empty.snap | 3 +- .../tests/snapshots/mkfs__simple.snap | 3 +- 10 files changed, 253 insertions(+), 213 deletions(-) diff --git a/crates/composefs-ctl/src/lib.rs b/crates/composefs-ctl/src/lib.rs index 9a8c592e..c86d7494 100644 --- a/crates/composefs-ctl/src/lib.rs +++ b/crates/composefs-ctl/src/lib.rs @@ -182,7 +182,7 @@ pub struct App { pub hash: Option, /// The EROFS format version to use when generating images. - /// If omitted, the library default (V2) is used. + /// If omitted, the library default (V1) is used. #[clap(long, value_enum)] pub erofs_version: Option, @@ -612,7 +612,7 @@ enum Command { reset_metadata: bool, /// Default EROFS format version for images in this repository. /// V1 is compatible with C `mkcomposefs` 1.0.8; V2 is the native format. - /// If omitted, falls back to the global `--erofs-version` flag, then defaults to V2. + /// If omitted, falls back to the global `--erofs-version` flag, then defaults to V1. #[clap(long)] erofs_version: Option, }, @@ -967,11 +967,11 @@ pub async fn run_app(args: App) -> Result<()> { erofs_version: ref init_erofs_version, } = args.cmd { - // Prefer the subcommand-level --erofs-version; fall back to global flag; default V2. + // Prefer the subcommand-level --erofs-version; fall back to global flag; default V1. let erofs_version = init_erofs_version .or(args.erofs_version) .map(composefs::erofs::format::FormatVersion::from) - .unwrap_or(composefs::erofs::format::FormatVersion::V2); + .unwrap_or(composefs::erofs::format::FormatVersion::V1); return run_init( algorithm, path.as_deref(), diff --git a/crates/composefs-integration-tests/src/tests/cli.rs b/crates/composefs-integration-tests/src/tests/cli.rs index 33260c9a..5a8dc1df 100644 --- a/crates/composefs-integration-tests/src/tests/cli.rs +++ b/crates/composefs-integration-tests/src/tests/cli.rs @@ -1902,7 +1902,7 @@ fn test_erofs_versions() -> Result<()> { ) .read()?; - // Default digest (should be V2) + // Default digest (should be V1) let id_default = cmd!( sh, "{cfsctl} --no-repo compute-id --no-propagate-usr-to-root {rootfs}" @@ -1914,9 +1914,10 @@ fn test_erofs_versions() -> Result<()> { id2.trim(), "V1 and V2 should produce different digests" ); - assert_eq!(id2.trim(), id_default.trim(), "Default should be V2"); + assert_eq!(id1.trim(), id_default.trim(), "Default should be V1"); - // Also verify via create-image in a real repo + // Also verify via create-image in a repo initialized with explicit V2 + // (the repo's stored format takes precedence over the global default) let repo_dir = init_insecure_repo(&sh, &cfsctl)?; let repo = repo_dir.path(); @@ -1944,7 +1945,7 @@ fn test_erofs_versions() -> Result<()> { assert_eq!( img_v2.trim(), img_default.trim(), - "create-image: default should match V2" + "create-image: V2 repo default should match explicit V2" ); Ok(()) @@ -2094,7 +2095,8 @@ fn test_oci_pull_v1_digest_stability() -> Result<()> { "V1 oci compute-id must be idempotent" ); - // V2 (default) must differ from V1 + // V2 (repo default, since init_insecure_repo uses --erofs-version 2) + // must differ from V1 let v2_id = cmd!( sh, "{cfsctl} --insecure --repo {repo} oci compute-id {at_config_digest}" diff --git a/crates/composefs-integration-tests/src/tests/digest_stability.rs b/crates/composefs-integration-tests/src/tests/digest_stability.rs index 26691e49..0a111fda 100644 --- a/crates/composefs-integration-tests/src/tests/digest_stability.rs +++ b/crates/composefs-integration-tests/src/tests/digest_stability.rs @@ -21,9 +21,9 @@ struct ContainerImage { /// unavailable (e.g. a PR that adds a new mirror entry before it has been /// pushed). Should be pinned by digest for reproducibility. upstream_ref: &'static str, - /// Expected composefs image ID without `--bootable` (V2/default EROFS). + /// Expected composefs image ID without `--bootable` (V2 EROFS). expected_id: &'static str, - /// Expected composefs image ID with `--bootable` (V2/default EROFS), or + /// Expected composefs image ID with `--bootable` (V2 EROFS), or /// `None` if the image lacks /sysroot and doesn't support bootable /// transformation. expected_bootable_id: Option<&'static str>, @@ -178,7 +178,7 @@ fn try_pull_image( bail!("could not find config digest in pull output:\n{output}") } -/// Compute the composefs image ID for a pulled OCI image (default V2 EROFS). +/// Compute the composefs image ID for a pulled OCI image (V2 EROFS, via repo default). /// /// The `config_digest` should be a bare OCI digest (e.g. `sha256:abc...`); /// this function adds the `@` prefix required by the CLI. @@ -236,8 +236,8 @@ fn compute_id_v1( /// Table-driven OCI container digest stability test. /// /// Pulls each pinned container image from a registry, computes the composefs -/// image ID for both plain and `--bootable` transforms using both the default -/// (V2) and V1 EROFS writers, and asserts they match the expected values. +/// image ID for both plain and `--bootable` transforms using both V1 and V2 +/// EROFS writers explicitly, and asserts they match the expected values. /// /// Skipped when `COMPOSEFS_SKIP_NETWORK=1` is set. fn test_oci_container_digest_stability() -> Result<()> { @@ -253,7 +253,7 @@ fn test_oci_container_digest_stability() -> Result<()> { eprintln!("--- {} ---", image.label); let repo_dir = tempfile::tempdir()?; let repo = repo_dir.path(); - // Use V2 explicitly: compute_id() tests V2 (default) hashes; V1 is + // Use V2 explicitly: compute_id() tests V2 hashes; V1 is // tested separately via compute_id_v1() with --erofs-version 1. cmd!( sh, @@ -264,7 +264,7 @@ fn test_oci_container_digest_stability() -> Result<()> { eprintln!("Pulling {} (this may take a while)...", image.label); let config = pull_image(&sh, &cfsctl, repo, image, image.label)?; - // V2 (default): plain image ID + // V2: plain image ID let plain_id = compute_id(&sh, &cfsctl, repo, &config, false)?; eprintln!("{} composefs V2 image ID: {plain_id}", image.label); assert_eq!( @@ -274,7 +274,7 @@ fn test_oci_container_digest_stability() -> Result<()> { image.label, ); - // V2 (default): bootable image ID (only for images that support it) + // V2: bootable image ID (only for images that support it) if let Some(expected_bootable) = image.expected_bootable_id { let bootable_id = compute_id(&sh, &cfsctl, repo, &config, true)?; eprintln!( @@ -359,7 +359,7 @@ fn try_expand_var(sh: &Shell) { } /// Verify that the bootable EROFS digest of a pinned image is identical -/// across all three computation paths: +/// across all three computation paths, for both V1 and V2 formats: /// /// 1. **OCI registry** — verified by `test_oci_container_digest_stability`, /// which pins the expected value in each image's `expected_bootable_id` @@ -372,9 +372,12 @@ fn try_expand_var(sh: &Shell) { /// On digest mismatch, captures dumpfiles from both paths and emits a /// unified diff to help identify the divergent entries. fn check_digest_equivalence(image: &ContainerImage) -> Result<()> { - let expected = image + let expected_v2 = image .expected_bootable_id .expect("image must have expected_bootable_id for equivalence test"); + let expected_v1 = image + .expected_v1_bootable_id + .expect("image must have expected_v1_bootable_id for equivalence test"); let sh = Shell::new()?; let cfsctl = cfsctl()?; @@ -386,109 +389,117 @@ fn check_digest_equivalence(image: &ContainerImage) -> Result<()> { eprintln!("Pulling {} into podman...", image.label); cmd!(sh, "podman pull {image_ref}").run()?; - // --- containers-storage path --- - let repo_dir = tempfile::tempdir()?; - let repo = repo_dir.path(); - cmd!(sh, "{cfsctl} --insecure --repo {repo} init").read()?; - - let cstor_ref = format!("containers-storage:{bare_ref}"); - eprintln!("Importing via containers-storage..."); - let pull_out = cmd!( - sh, - "{cfsctl} --insecure --repo {repo} oci pull --local-fetch auto {cstor_ref}" - ) - .read()?; - - let config = pull_out - .lines() - .find_map(|l| l.strip_prefix("config").map(|r| r.trim().to_string())) - .ok_or_else(|| { - anyhow::anyhow!("{}: no config in cstor output:\n{pull_out}", image.label) - })?; - - let cstor_digest = compute_id(&sh, &cfsctl, repo, &config, true)?; - eprintln!(" containers-storage: {cstor_digest}"); - - // --- on-disk filesystem path --- + // Mount the container for the on-disk filesystem path. let cid = cmd!(sh, "podman create {image_ref} /bin/true").read()?; let cid = cid.trim(); let mountpoint = cmd!(sh, "podman mount {cid}").read()?; let mountpoint = mountpoint.trim(); - let fs_digest = cmd!( - sh, - "{cfsctl} --insecure --repo {repo} compute-id --bootable {mountpoint}" - ) - .read() - .map(|s| s.trim().to_string()); - - // If digests mismatch, capture dumpfiles *before* unmounting. - let mismatch = match &fs_digest { - Ok(d) => d != &cstor_digest, - Err(_) => true, - }; + // Test both V1 and V2 explicitly. + for (version, expected) in [("1", expected_v1), ("2", expected_v2)] { + eprintln!(" --- V{version} ---"); - let diff_output = if mismatch { - eprintln!(" MISMATCH detected — capturing dumpfiles for diff..."); + let repo_dir = tempfile::tempdir()?; + let repo = repo_dir.path(); + cmd!( + sh, + "{cfsctl} --insecure --repo {repo} init --erofs-version {version}" + ) + .read()?; - // cstor dumpfile: `cfsctl oci dump --bootable @` - let at_config = format!("@{config}"); - let cstor_dump = cmd!( + let cstor_ref = format!("containers-storage:{bare_ref}"); + eprintln!(" Importing via containers-storage (V{version})..."); + let pull_out = cmd!( sh, - "{cfsctl} --insecure --repo {repo} oci dump --bootable {at_config}" + "{cfsctl} --insecure --repo {repo} oci pull --local-fetch auto {cstor_ref}" ) - .read() - .unwrap_or_else(|e| format!("(failed to dump cstor: {e})")); + .read()?; + + let config = pull_out + .lines() + .find_map(|l| l.strip_prefix("config").map(|r| r.trim().to_string())) + .ok_or_else(|| { + anyhow::anyhow!("{}: no config in cstor output:\n{pull_out}", image.label) + })?; - // on-disk dumpfile: `cfsctl create-dumpfile --bootable ` - let fs_dump = cmd!( + let cstor_digest = compute_id(&sh, &cfsctl, repo, &config, true)?; + eprintln!(" containers-storage V{version}: {cstor_digest}"); + + let fs_digest = cmd!( sh, - "{cfsctl} --no-repo create-dumpfile --bootable {mountpoint}" + "{cfsctl} --insecure --erofs-version {version} --repo {repo} compute-id --bootable {mountpoint}" ) .read() - .unwrap_or_else(|e| format!("(failed to dump on-disk: {e})")); - - // Write to temp files and diff - let cstor_path = std::env::temp_dir().join("cstor.dumpfile"); - let fs_path = std::env::temp_dir().join("fs.dumpfile"); - std::fs::write(&cstor_path, &cstor_dump)?; - std::fs::write(&fs_path, &fs_dump)?; + .map(|s| s.trim().to_string()); + + // If digests mismatch, capture dumpfiles *before* unmounting. + let mismatch = match &fs_digest { + Ok(d) => d != &cstor_digest, + Err(_) => true, + }; + + let diff_output = if mismatch { + eprintln!(" MISMATCH detected — capturing dumpfiles for diff..."); + + let at_config = format!("@{config}"); + let cstor_dump = cmd!( + sh, + "{cfsctl} --insecure --repo {repo} oci dump --bootable {at_config}" + ) + .read() + .unwrap_or_else(|e| format!("(failed to dump cstor: {e})")); - let diff = cmd!(sh, "diff -u {cstor_path} {fs_path}") - .ignore_status() + let fs_dump = cmd!( + sh, + "{cfsctl} --no-repo create-dumpfile --bootable {mountpoint}" + ) .read() - .unwrap_or_else(|e| format!("(diff failed: {e})")); + .unwrap_or_else(|e| format!("(failed to dump on-disk: {e})")); - Some(diff) - } else { - None - }; + let cstor_path = std::env::temp_dir().join("cstor.dumpfile"); + let fs_path = std::env::temp_dir().join("fs.dumpfile"); + std::fs::write(&cstor_path, &cstor_dump)?; + std::fs::write(&fs_path, &fs_dump)?; - // Clean up container before asserting. - cmd!(sh, "podman umount {cid}").ignore_status().run()?; - cmd!(sh, "podman rm -f {cid}").ignore_status().run()?; + let diff = cmd!(sh, "diff -u {cstor_path} {fs_path}") + .ignore_status() + .read() + .unwrap_or_else(|e| format!("(diff failed: {e})")); - let fs_digest = fs_digest?; - eprintln!(" on-disk filesystem: {fs_digest}"); + Some(diff) + } else { + None + }; - if let Some(ref diff) = diff_output { - eprintln!( - "\n=== dumpfile diff (containers-storage vs on-disk) ===\n{diff}\n=== end diff ===\n" - ); - } + let fs_digest = fs_digest?; + eprintln!(" on-disk filesystem V{version}: {fs_digest}"); - // Assert both paths match the pinned expected value. - if cstor_digest != expected || fs_digest != expected { - bail!( - "{label}: digest mismatch!\n\ - \x20 expected (pinned): {expected}\n\ - \x20 containers-storage: {cstor_digest}\n\ - \x20 on-disk filesystem: {fs_digest}", - label = image.label, - ); + if let Some(ref diff) = diff_output { + eprintln!( + "\n=== V{version} dumpfile diff (containers-storage vs on-disk) ===\n\ + {diff}\n=== end diff ===\n" + ); + } + + if cstor_digest != expected || fs_digest != expected { + // Clean up before bailing. + cmd!(sh, "podman umount {cid}").ignore_status().run()?; + cmd!(sh, "podman rm -f {cid}").ignore_status().run()?; + bail!( + "{label} V{version}: digest mismatch!\n\ + \x20 expected (pinned): {expected}\n\ + \x20 containers-storage: {cstor_digest}\n\ + \x20 on-disk filesystem: {fs_digest}", + label = image.label, + ); + } + + eprintln!(" OK V{version}: all three paths match: {expected}"); } - eprintln!(" OK: all three paths match: {expected}"); + cmd!(sh, "podman umount {cid}").ignore_status().run()?; + cmd!(sh, "podman rm -f {cid}").ignore_status().run()?; + Ok(()) } diff --git a/crates/composefs-oci/src/lib.rs b/crates/composefs-oci/src/lib.rs index 807ae966..9e1cc056 100644 --- a/crates/composefs-oci/src/lib.rs +++ b/crates/composefs-oci/src/lib.rs @@ -1291,12 +1291,13 @@ mod test { "expected no V1 image ref for a V2-only config" ); - // Also verify via the convenience function + // Also verify via the convenience function (V2 lookup, matching the + // V2-only ref stored above) let img_ref = composefs_erofs_for_config( &repo, &config_digest, Some(&config_verity), - repo.erofs_version(), + FormatVersion::V2, ) .unwrap(); assert_eq!(img_ref, Some(fake_erofs_id)); diff --git a/crates/composefs/src/erofs/format.rs b/crates/composefs/src/erofs/format.rs index 4616c761..87bf261a 100644 --- a/crates/composefs/src/erofs/format.rs +++ b/crates/composefs/src/erofs/format.rs @@ -314,14 +314,15 @@ pub enum FormatVersion { /// Format V1: same EROFS layout as V0 (compact inodes, BFS, whiteout table), /// but `composefs_version` is always `1` in the header. /// - /// Equivalent to C `mkcomposefs --min-version=1`. Recommended default for - /// new repositories: unconditionally signals support for data-only layers. + /// Equivalent to C `mkcomposefs --min-version=1`. Default format for new + /// repositories: unconditionally signals support for data-only layers and + /// is compatible with both C `mkcomposefs` 1.0.8 and composefs-rs. + #[default] V1 = 1, /// Format V2: extended inodes, DFS ordering, no whiteout table, /// `composefs_version=2`. /// - /// This is the composefs-rs native format, used by older bootc deployments. - #[default] + /// composefs-rs native format, used by older bootc deployments. V2 = 2, } @@ -383,9 +384,9 @@ pub struct FormatConfig { } impl Default for FormatConfig { - /// Returns a single-V2 config, matching the default for newly created repositories. + /// Returns a single-V1 config, matching the default for newly created repositories. fn default() -> Self { - Self::single(FormatVersion::V2) + Self::single(FormatVersion::V1) } } diff --git a/crates/composefs/src/erofs/reader.rs b/crates/composefs/src/erofs/reader.rs index ddd63e91..5dd31b6c 100644 --- a/crates/composefs/src/erofs/reader.rs +++ b/crates/composefs/src/erofs/reader.rs @@ -2055,7 +2055,10 @@ mod tests { use super::*; use crate::{ dumpfile::{dumpfile_to_filesystem, write_dumpfile}, - erofs::writer::{ValidatedFileSystem, mkfs_erofs}, + erofs::{ + format::FormatVersion, + writer::{ValidatedFileSystem, mkfs_erofs, mkfs_erofs_versioned}, + }, fsverity::Sha256HashValue, }; use std::collections::HashMap; @@ -2130,13 +2133,16 @@ mod tests { #[test] fn test_empty_directory() { - // Create filesystem with empty directory + // Create filesystem with empty directory (V2: no whiteout stubs) let dumpfile = r#"/ 0 40755 2 0 0 0 1000.0 - - - /empty_dir 0 40755 2 0 0 0 1000.0 - - - "#; let fs = dumpfile_to_filesystem::(dumpfile).unwrap(); - let image = mkfs_erofs(&mut ValidatedFileSystem::new(fs).unwrap()); + let image = mkfs_erofs_versioned( + &mut ValidatedFileSystem::new(fs).unwrap(), + FormatVersion::V2, + ); let img = Image::open(&image).unwrap(); // Root should have . and .. and empty_dir @@ -2267,7 +2273,7 @@ mod tests { #[test] fn test_nested_directories() { - // Test deeply nested directory structure + // Test deeply nested directory structure (V2: no whiteout stubs) let dumpfile = r#"/ 0 40755 2 0 0 0 1000.0 - - - /a 0 40755 2 0 0 0 1000.0 - - - /a/b 0 40755 2 0 0 0 1000.0 - - - @@ -2276,7 +2282,10 @@ mod tests { "#; let fs = dumpfile_to_filesystem::(dumpfile).unwrap(); - let image = mkfs_erofs(&mut ValidatedFileSystem::new(fs).unwrap()); + let image = mkfs_erofs_versioned( + &mut ValidatedFileSystem::new(fs).unwrap(), + FormatVersion::V2, + ); let img = Image::open(&image).unwrap(); // Navigate through the structure @@ -2427,7 +2436,7 @@ mod tests { #[test] fn test_round_trip_basic() { - // Full round-trip: dumpfile -> tree -> erofs -> read back -> validate + // Full round-trip: dumpfile -> tree -> erofs -> read back -> validate (V2) let dumpfile = r#"/ 0 40755 2 0 0 0 1000.0 - - - /file1 5 100644 1 0 0 0 1000.0 - hello - /file2 6 100644 1 0 0 0 1000.0 - world! - @@ -2436,7 +2445,10 @@ mod tests { "#; let fs = dumpfile_to_filesystem::(dumpfile).unwrap(); - let image = mkfs_erofs(&mut ValidatedFileSystem::new(fs).unwrap()); + let image = mkfs_erofs_versioned( + &mut ValidatedFileSystem::new(fs).unwrap(), + FormatVersion::V2, + ); let img = Image::open(&image).unwrap(); // Verify root entries @@ -2475,7 +2487,7 @@ mod tests { assert_eq!(inline_data, Some(b"hello".as_slice())); } - /// Helper: round-trip a dumpfile through erofs and compare the result. + /// Helper: round-trip a dumpfile through V2 erofs and compare the result. fn round_trip_dumpfile(input: &str) -> (String, String) { let fs_orig = dumpfile_to_filesystem::(input).unwrap(); @@ -2483,7 +2495,10 @@ mod tests { write_dumpfile(&mut orig_output, &fs_orig).unwrap(); let orig_str = String::from_utf8(orig_output).unwrap(); - let image = mkfs_erofs(&mut ValidatedFileSystem::new(fs_orig).unwrap()); + let image = mkfs_erofs_versioned( + &mut ValidatedFileSystem::new(fs_orig).unwrap(), + FormatVersion::V2, + ); let fs_rt = erofs_to_filesystem::(&image).unwrap(); let mut rt_output = Vec::new(); @@ -3038,7 +3053,10 @@ mod tests { let mut expected_output = Vec::new(); write_dumpfile(&mut expected_output, &fs_expected).unwrap(); - let image = mkfs_erofs(&mut ValidatedFileSystem::new(fs_write).unwrap()); + let image = mkfs_erofs_versioned( + &mut ValidatedFileSystem::new(fs_write).unwrap(), + FormatVersion::V2, + ); let fs_rt = erofs_to_filesystem::(&image).unwrap(); let mut rt_output = Vec::new(); @@ -3578,13 +3596,16 @@ mod tests { /// first in the BTreeMap, leaving the first leaf unreferenced). #[test] fn test_duplicate_dirent_rejected() { - // Build a valid image with two files + // Build a valid V2 image with two files (V2 for predictable byte layout) let dumpfile = r#"/ 0 40755 2 0 0 0 1000.0 - - - /aaa 5 100644 1 0 0 0 1000.0 - hello - /bbb 5 100644 1 0 0 0 1000.0 - world - "#; let fs = dumpfile_to_filesystem::(dumpfile).unwrap(); - let image = mkfs_erofs(&mut ValidatedFileSystem::new(fs).unwrap()); + let image = mkfs_erofs_versioned( + &mut ValidatedFileSystem::new(fs).unwrap(), + FormatVersion::V2, + ); // Sanity: the unmodified image round-trips fine erofs_to_filesystem::(&image).unwrap(); @@ -4023,9 +4044,15 @@ mod tests { /// Helper: build a simple filesystem from a dumpfile and write it as an /// Epoch2 (V2) EROFS image. fn build_epoch2_image(dumpfile: &str) -> Box<[u8]> { - use crate::erofs::writer::{ValidatedFileSystem, mkfs_erofs}; + use crate::erofs::{ + format::FormatVersion, + writer::{ValidatedFileSystem, mkfs_erofs_versioned}, + }; let fs = crate::dumpfile::dumpfile_to_filesystem::(dumpfile).unwrap(); - mkfs_erofs(&mut ValidatedFileSystem::new(fs).unwrap()) + mkfs_erofs_versioned( + &mut ValidatedFileSystem::new(fs).unwrap(), + FormatVersion::V2, + ) } /// Helper: build a filesystem from a dumpfile and write it as an Epoch1 (V1) diff --git a/crates/composefs/src/repository.rs b/crates/composefs/src/repository.rs index be6d7f42..8a2dbaab 100644 --- a/crates/composefs/src/repository.rs +++ b/crates/composefs/src/repository.rs @@ -286,6 +286,12 @@ impl FeatureFlags { } } +/// Serde default for `RepoMetadata::erofs_formats` — returns `single(V2)` so +/// that old repos (created before the field existed) deserialize correctly. +fn legacy_erofs_default() -> FormatConfig { + FormatConfig::single(FormatVersion::V2) +} + /// Repository metadata stored in `meta.json` at the repository root. /// /// This file records the repository's format version, digest algorithm, @@ -322,10 +328,10 @@ pub struct RepoMetadata { /// /// Persisted directly so that dual-format repositories (e.g. V1+V2) can /// round-trip through `meta.json`. For repositories created before this - /// field existed, the field is absent from JSON and defaults to - /// `FormatConfig::single(FormatVersion::V2)`; the `"v1_erofs"` flag is then - /// checked for seamless migration of old V1 repos. - #[serde(default)] + /// field existed, the field is absent from JSON and deserializes as + /// `FormatConfig::single(FormatVersion::V2)` (the legacy default); the + /// `"v1_erofs"` flag is then checked for seamless migration of old V1 repos. + #[serde(default = "legacy_erofs_default")] pub erofs_formats: FormatConfig, } @@ -344,13 +350,14 @@ impl RepoMetadata { /// Return the effective [`FormatConfig`] for this repository. /// - /// If `erofs_formats` was explicitly set in `meta.json` (i.e. it is not the - /// serde default `single(V2)`), it is returned as-is. Otherwise the config - /// is derived from the legacy `"v1_erofs"` ro_compat flag for backward - /// compatibility with repos created before the `erofs_formats` field existed. + /// If `erofs_formats` was explicitly set in `meta.json` (i.e. it differs + /// from the legacy serde default of `single(V2)`), it is returned as-is. + /// Otherwise the config is derived from the legacy `"v1_erofs"` ro_compat + /// flag for backward compatibility with repos created before the + /// `erofs_formats` field existed. pub fn format_config(&self) -> FormatConfig { - let default_config = FormatConfig::default(); - if self.erofs_formats != default_config { + let legacy_default = legacy_erofs_default(); + if self.erofs_formats != legacy_default { // Field was explicitly stored — use it directly. self.erofs_formats.clone() } else if self @@ -362,8 +369,8 @@ impl RepoMetadata { // Legacy V1 repo: flag present but new field absent/default. FormatConfig::single(FormatVersion::V1) } else { - // V2 default (new repos or old repos without the flag). - default_config + // Old V2 repo (field absent, no v1_erofs flag). + legacy_default } } } @@ -5282,9 +5289,17 @@ mod tests { let parsed: serde_json::Value = serde_json::from_slice(&json).unwrap(); assert_eq!(parsed["features"]["compatible"][0], "some-compat"); - assert_eq!( - parsed["features"]["read-only-compatible"][0], - "some-rocompat" + // V1 default adds "v1_erofs" as the first ro_compat flag + let ro_compat = parsed["features"]["read-only-compatible"] + .as_array() + .unwrap(); + assert!( + ro_compat.iter().any(|v| v == "v1_erofs"), + "expected v1_erofs flag" + ); + assert!( + ro_compat.iter().any(|v| v == "some-rocompat"), + "expected some-rocompat flag" ); // Roundtrip diff --git a/crates/composefs/tests/mkfs.rs b/crates/composefs/tests/mkfs.rs index 9efd5936..15ca95bc 100644 --- a/crates/composefs/tests/mkfs.rs +++ b/crates/composefs/tests/mkfs.rs @@ -15,7 +15,7 @@ use composefs::{ erofs::{ debug::debug_img, format::FormatVersion, - writer::{ValidatedFileSystem, mkfs_erofs, mkfs_erofs_versioned}, + writer::{ValidatedFileSystem, mkfs_erofs_versioned}, }, fsverity::{FsVerityHashValue, Sha256HashValue}, tree::{FileSystem, Inode, LeafContent, RegularFile, Stat}, @@ -32,8 +32,11 @@ fn default_stat() -> Stat { } } -fn debug_fs(fs: FileSystem) -> String { - let image = mkfs_erofs(&mut ValidatedFileSystem::new(fs).unwrap()); +fn debug_fs_v2(fs: FileSystem) -> String { + let image = mkfs_erofs_versioned( + &mut ValidatedFileSystem::new(fs).unwrap(), + FormatVersion::V2, + ); let mut output = vec![]; debug_img(&mut output, &image).unwrap(); String::from_utf8(output).unwrap() @@ -45,7 +48,7 @@ fn empty(_fs: &mut FileSystem) {} fn test_empty() { let mut fs = FileSystem::::new(default_stat()); empty(&mut fs); - insta::assert_snapshot!(debug_fs(fs)); + insta::assert_snapshot!(debug_fs_v2(fs)); } fn add_leaf( @@ -97,40 +100,24 @@ fn simple(fs: &mut FileSystem) { fn test_simple() { let mut fs = FileSystem::::new(default_stat()); simple(&mut fs); - insta::assert_snapshot!(debug_fs(fs)); -} - -fn foreach_case(f: fn(FileSystem)) { - for case in [empty, simple] { - let mut fs = FileSystem::new(default_stat()); - case(&mut fs); - f(fs); - } + insta::assert_snapshot!(debug_fs_v2(fs)); } #[test_with::executable(fsck.erofs)] fn test_fsck() { - foreach_case(|fs| { - // V2 (default) - let mut tmp = NamedTempFile::new().unwrap(); - tmp.write_all(&mkfs_erofs(&mut ValidatedFileSystem::new(fs).unwrap())) - .unwrap(); - let mut fsck = Command::new("fsck.erofs").arg(tmp.path()).spawn().unwrap(); - assert!(fsck.wait().unwrap().success()); - }); - - // V1 — needs its own filesystem instances (whiteout stubs are added by the writer) - for case in [empty, simple] { - let mut fs = FileSystem::::new(default_stat()); - case(&mut fs); - let image = mkfs_erofs_versioned( - &mut ValidatedFileSystem::new(fs).unwrap(), - FormatVersion::V1, - ); - let mut tmp = NamedTempFile::new().unwrap(); - tmp.write_all(&image).unwrap(); - let mut fsck = Command::new("fsck.erofs").arg(tmp.path()).spawn().unwrap(); - assert!(fsck.wait().unwrap().success()); + for version in [FormatVersion::V0, FormatVersion::V1, FormatVersion::V2] { + for case in [empty as fn(&mut _), simple] { + let mut fs = FileSystem::::new(default_stat()); + case(&mut fs); + let image = mkfs_erofs_versioned(&mut ValidatedFileSystem::new(fs).unwrap(), version); + let mut tmp = NamedTempFile::new().unwrap(); + tmp.write_all(&image).unwrap(); + let mut fsck = Command::new("fsck.erofs").arg(tmp.path()).spawn().unwrap(); + assert!( + fsck.wait().unwrap().success(), + "fsck failed for {version:?}" + ); + } } } @@ -185,66 +172,64 @@ fn dump_image(img: &[u8]) -> String { #[test] fn test_erofs_digest_stability() { - // Pin digests for each test case — any change to the EROFS writer that - // alters byte-level output will break these, which is the point: composefs - // image digest stability is critical for the bootc sealed UKI trust chain. - let cases: &[(&str, fn(&mut FileSystem), &str)] = &[ + // Pin digests for each format version × test case. Any change to the EROFS + // writer that alters byte-level output will break these, which is the point: + // composefs image digest stability is critical for the bootc sealed UKI trust + // chain. V0 output must also be byte-stable since it needs to match C + // mkcomposefs. + let cases: &[( + FormatVersion, + &str, + fn(&mut FileSystem), + &str, + )] = &[ ( - "empty", + FormatVersion::V0, + "empty_v0", empty, - "086b702a519b57d6ef5aea6f8b3f2be24355cd1fb835cd80fb4e3d388b24d5a5", + "8f589e8f57ecb88823736b0d857ddca1e1068a23e264fad164b28f7038eb3682", ), ( - "simple", + FormatVersion::V0, + "simple_v0", simple, - "a8fcd41f8b313bede69f462f2af0a38d64b99a6333f5df884ea9ab4037fac722", + "9f3f5620ee0c54708516467d0d58741e7963047c7106b245d94c298259d0fa01", ), - ]; - - for (name, case, expected_digest) in cases { - let mut fs = FileSystem::::new(default_stat()); - case(&mut fs); - let image = mkfs_erofs(&mut ValidatedFileSystem::new(fs).unwrap()); - let digest = composefs::fsverity::compute_verity::(&image); - let hex = digest.to_hex(); - assert_eq!( - &hex, expected_digest, - "{name}: EROFS digest changed — if this is intentional, update the pinned value" - ); - } -} - -#[test] -fn test_erofs_v1_digest_stability() { - // Same as test_erofs_digest_stability but for V0 (C-compatible) format. - // V0 uses the same layout as C mkcomposefs default: composefs_version is 0 - // normally and auto-bumped to 1 when user whiteouts are present. - // Output must be byte-stable since it needs to match C mkcomposefs. - let cases: &[(&str, fn(&mut FileSystem), &str)] = &[ ( + FormatVersion::V1, "empty_v1", empty, - "8f589e8f57ecb88823736b0d857ddca1e1068a23e264fad164b28f7038eb3682", + "14a26c957c84f6eb774b91205476adc13196c7c33b9dd97d08d43725ecb90b63", ), ( + FormatVersion::V1, "simple_v1", simple, - "9f3f5620ee0c54708516467d0d58741e7963047c7106b245d94c298259d0fa01", + "ad6db6427568a1e4fdc772cbab7b6063f5eb4b33cb62982a740ba9213e5962b5", + ), + ( + FormatVersion::V2, + "empty_v2", + empty, + "086b702a519b57d6ef5aea6f8b3f2be24355cd1fb835cd80fb4e3d388b24d5a5", + ), + ( + FormatVersion::V2, + "simple_v2", + simple, + "a8fcd41f8b313bede69f462f2af0a38d64b99a6333f5df884ea9ab4037fac722", ), ]; - for (name, case, expected_digest) in cases { + for (version, name, case, expected_digest) in cases { let mut fs = FileSystem::::new(default_stat()); case(&mut fs); - let image = mkfs_erofs_versioned( - &mut ValidatedFileSystem::new(fs).unwrap(), - FormatVersion::V0, - ); + let image = mkfs_erofs_versioned(&mut ValidatedFileSystem::new(fs).unwrap(), *version); let digest = composefs::fsverity::compute_verity::(&image); let hex = digest.to_hex(); assert_eq!( &hex, expected_digest, - "{name}: V0 EROFS digest changed — if this is intentional, update the pinned value" + "{name}: EROFS digest changed — if this is intentional, update the pinned value" ); } } diff --git a/crates/composefs/tests/snapshots/mkfs__empty.snap b/crates/composefs/tests/snapshots/mkfs__empty.snap index b73e7006..042c0b3f 100644 --- a/crates/composefs/tests/snapshots/mkfs__empty.snap +++ b/crates/composefs/tests/snapshots/mkfs__empty.snap @@ -1,7 +1,6 @@ --- source: crates/composefs/tests/mkfs.rs -assertion_line: 47 -expression: debug_fs(fs) +expression: debug_fs_v2(fs) --- 00000000 ComposefsHeader +0 magic: U32(3497550490) diff --git a/crates/composefs/tests/snapshots/mkfs__simple.snap b/crates/composefs/tests/snapshots/mkfs__simple.snap index bc042c29..44829625 100644 --- a/crates/composefs/tests/snapshots/mkfs__simple.snap +++ b/crates/composefs/tests/snapshots/mkfs__simple.snap @@ -1,7 +1,6 @@ --- source: crates/composefs/tests/mkfs.rs -assertion_line: 100 -expression: debug_fs(fs) +expression: debug_fs_v2(fs) --- 00000000 ComposefsHeader +0 magic: U32(3497550490)