Skip to content

erofs: Switch default format version from V2 to V1 #330

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
cgwalters wants to merge 1 commit into
base: main
Choose a base branch
from
Open

erofs: Switch default format version from V2 to V1 #330

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 4 additions & 4 deletions crates/composefs-ctl/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ pub struct App {
pub hash: Option<HashType>,

/// 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<ErofsVersion>,

Expand Down Expand Up @@ -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<ErofsVersion>,
},
Expand Down Expand Up @@ -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(),
Expand Down
12 changes: 7 additions & 5 deletions crates/composefs-integration-tests/src/tests/cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Expand All @@ -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();

Expand Down Expand Up @@ -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(())
Expand Down Expand Up @@ -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}"
Expand Down
193 changes: 102 additions & 91 deletions crates/composefs-integration-tests/src/tests/digest_stability.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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<()> {
Expand All @@ -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,
Expand All @@ -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!(
Expand All @@ -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!(
Expand Down Expand Up @@ -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`
Expand All @@ -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()?;
Expand All @@ -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 @<config>`
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 <mountpoint>`
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(())
}

Expand Down
5 changes: 3 additions & 2 deletions crates/composefs-oci/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
Loading