diff --git a/crates/wasm-pkg-client/src/decoded_component.rs b/crates/wasm-pkg-client/src/decoded_component.rs index 8c8767d..2142e12 100644 --- a/crates/wasm-pkg-client/src/decoded_component.rs +++ b/crates/wasm-pkg-client/src/decoded_component.rs @@ -88,34 +88,47 @@ impl DecodedComponent { (other, self) }; - let (prev_resolve, prev_world) = extract_resolve_and_world_id(&newer.decoded_wasm)?; - let (new_resolve, new_world) = extract_resolve_and_world_id(&older.decoded_wasm)?; - - // Merge resolves, remap merged resolve, check for incompatibility - let mut merged = prev_resolve.clone(); - let new_world = match merged - .merge(new_resolve.clone()) - .map(|remap| remap.map_world(new_world, wit_parser::Span::default())) - { - Ok(Ok(w)) => w, - Ok(Err(e)) => { - return Err(Error::InvalidComponent(anyhow::format_err!( - "failed to remap merged worlds: {}", - e.kind() - ))); - } - Err(e) => { - return Err(Error::InvalidComponent(e)); - } - }; + let (prev_resolve, prev_worlds) = extract_resolve_and_worlds(&newer.decoded_wasm); + let (new_resolve, new_worlds) = extract_resolve_and_worlds(&older.decoded_wasm); + + for (name, new_world) in new_worlds { + let Some(&prev_world) = prev_worlds.get(&name) else { + // World removal is considered a breaking change + return Err(Error::SemverIncompatible { + previous: older.version.clone(), + new: newer.version.clone(), + source: anyhow::anyhow!("world `{name}` was removed"), + }); + }; + + // Merge resolves, remap merged resolve, check for incompatibility + let mut merged = prev_resolve.clone(); + let new_world = match merged + .merge(new_resolve.clone()) + .map(|remap| remap.map_world(new_world, wit_parser::Span::default())) + { + Ok(Ok(w)) => w, + Ok(Err(e)) => { + return Err(Error::InvalidComponent(anyhow::format_err!( + "failed to remap merged worlds: {}", + e.kind() + ))); + } + Err(e) => { + return Err(Error::InvalidComponent(e)); + } + }; + + wit_component::semver_check(merged, prev_world, new_world).map_err(|e| { + Error::SemverIncompatible { + previous: older.version.clone(), + new: newer.version.clone(), + source: e.context(format!("world `{name}`")), + } + })?; + } - wit_component::semver_check(merged, prev_world, new_world).map_err(|e| { - Error::SemverIncompatible { - previous: older.version.clone(), - new: newer.version.clone(), - source: e, - } - }) + Ok(()) } } @@ -208,20 +221,26 @@ fn extract_package_version(decoded: &DecodedWasm) -> Result<(PackageRef, Version Ok((package, version)) } -/// Borrow the inner `wit_parser::Resolve` and resolve a concrete `WorldId`. -/// For a decoded component the world is fixed; for a WIT package we ask -/// `Resolve::select_world` to pick one — deferred until needed so a -/// multi-world WIT package can publish its first version unambiguously. -fn extract_resolve_and_world_id( +/// Borrow the inner `Resolve` and build an index of its worlds. The world +/// names are borrowed from the `Resolve`, which outlives the returned index. +fn extract_resolve_and_worlds( decoded: &DecodedWasm, -) -> Result<(&wit_parser::Resolve, wit_parser::WorldId), Error> { +) -> ( + &wit_parser::Resolve, + std::collections::HashMap<&str, wit_parser::WorldId>, +) { match decoded { - DecodedWasm::Component(resolve, world_id) => Ok((resolve, *world_id)), + DecodedWasm::Component(resolve, world_id) => { + let name = resolve.worlds[*world_id].name.as_str(); + (resolve, std::iter::once((name, *world_id)).collect()) + } DecodedWasm::WitPackage(resolve, pkg) => { - let world_id = resolve - .select_world(&[*pkg], None) - .map_err(Error::InvalidPackage)?; - Ok((resolve, world_id)) + let worlds = resolve.packages[*pkg] + .worlds + .iter() + .map(|(name, id)| (name.as_str(), *id)) + .collect(); + (resolve, worlds) } } } diff --git a/crates/wasm-pkg-client/tests/publish_semver_check.rs b/crates/wasm-pkg-client/tests/publish_semver_check.rs index 5dee7c9..897b3a7 100644 --- a/crates/wasm-pkg-client/tests/publish_semver_check.rs +++ b/crates/wasm-pkg-client/tests/publish_semver_check.rs @@ -215,3 +215,112 @@ async fn publish_semver_check( _ => panic!("expectation mismatch\n expected: {expected:?}\n actual: {result:?}",), } } + +// --------------------------------------------------------------------------- +// Multi-world packages (issue: semver_check assumed at most one world). +// +// A WIT package may declare more than one world. Semver checking must compare +// worlds pairwise by name: shared worlds must stay compatible, adding a world +// is additive (OK), and removing a world within the same compat range is +// breaking (strict policy). +// --------------------------------------------------------------------------- + +/// Which worlds a multi-world fixture declares, and the body of each. +struct MultiWorld { + /// `alpha` world body, if present. + alpha: Option, + /// `beta` world body, if present. + beta: Option, +} + +fn multiworld_wit(package: &str, version: &str, worlds: &MultiWorld) -> String { + let mut out = format!("\npackage {NAMESPACE}:{package}@{version};\n"); + if let Some(diff) = worlds.alpha { + out.push_str(&format!("\nworld alpha {{\n {diff}\n}}\n")); + } + if let Some(diff) = worlds.beta { + out.push_str(&format!("\nworld beta {{\n {diff}\n}}\n")); + } + out +} + +#[rstest] +// Both worlds compatible across a minor bump within a major -> OK. +#[case::multiworld_all_compatible( + "mw-all-compat", + ("1.2.0", MultiWorld { alpha: Some(WorldDiff::AddBase), beta: Some(WorldDiff::AddBase) }), + ("1.3.0", MultiWorld { alpha: Some(WorldDiff::AddExtra), beta: Some(WorldDiff::AddExtra) }), + Ok(()) +)] +// One shared world (beta) breaks across a minor bump -> incompatible. +#[case::multiworld_one_incompatible( + "mw-one-incompat", + ("1.2.0", MultiWorld { alpha: Some(WorldDiff::AddBase), beta: Some(WorldDiff::AddBase) }), + ("1.3.0", MultiWorld { alpha: Some(WorldDiff::AddExtra), beta: Some(WorldDiff::ChangeBase) }), + Err(semver_incompatible("1.2.0", "1.3.0")) +)] +// Adding a world across a minor bump is additive -> OK. +#[case::multiworld_added_world( + "mw-added", + ("1.2.0", MultiWorld { alpha: Some(WorldDiff::AddBase), beta: None }), + ("1.3.0", MultiWorld { alpha: Some(WorldDiff::AddExtra), beta: Some(WorldDiff::AddBase) }), + Ok(()) +)] +// Removing a world within the same major is breaking (strict policy) -> incompatible. +#[case::multiworld_removed_world_same_major( + "mw-removed", + ("1.2.0", MultiWorld { alpha: Some(WorldDiff::AddBase), beta: Some(WorldDiff::AddBase) }), + ("1.3.0", MultiWorld { alpha: Some(WorldDiff::AddExtra), beta: None }), + Err(semver_incompatible("1.2.0", "1.3.0")) +)] +// Removing a world across a major boundary is allowed: the versions fall in +// different compatibility ranges, so `semver_check` never compares them. +#[case::multiworld_removed_world_across_major( + "mw-removed-cross-major", + ("1.2.0", MultiWorld { alpha: Some(WorldDiff::AddBase), beta: Some(WorldDiff::AddBase) }), + ("2.0.0", MultiWorld { alpha: Some(WorldDiff::AddBase), beta: None }), + Ok(()) +)] +#[tokio::test] +async fn publish_semver_check_multiworld( + #[case] package: &str, + #[case] initial: (&str, MultiWorld), + #[case] candidate: (&str, MultiWorld), + #[case] expected: Result<(), Error>, +) { + let tmp = TempDir::new().unwrap(); + let client = make_client(tmp.path()); + + let (init_version, init_worlds) = initial; + publish( + &client, + wit_to_wasm(&multiworld_wit(package, init_version, &init_worlds)), + Default::default(), + ) + .await + .unwrap_or_else(|e| panic!("seeding {init_version} failed: {e:?}")); + + let (cand_version, cand_worlds) = candidate; + let result = publish( + &client, + wit_to_wasm(&multiworld_wit(package, cand_version, &cand_worlds)), + Default::default(), + ) + .await; + + match (&expected, &result) { + (Ok(()), Ok(())) => {} + ( + Err(Error::SemverIncompatible { + previous: exp_prev, + new: exp_new, + .. + }), + Err(Error::SemverIncompatible { previous, new, .. }), + ) => { + assert_eq!(previous, exp_prev, "previous version mismatch"); + assert_eq!(new, exp_new, "new version mismatch"); + } + _ => panic!("expectation mismatch\n expected: {expected:?}\n actual: {result:?}",), + } +}