From 6e07006a209b94db194e24aaf776fd90156515b9 Mon Sep 17 00:00:00 2001 From: Aditya Salunkhe Date: Sat, 15 Aug 2026 18:46:27 +0530 Subject: [PATCH 1/3] feat(core,wkg): resolve multiple versions of the same package Key dependencies by package plus version requirement rather than by package alone, so a wkg.toml can declare a separate override per version and a world naming two versions of the same package resolves both instead of dropping one --- crates/wasm-pkg-core/src/manifest.rs | 113 +++++++++- crates/wasm-pkg-core/src/resolver.rs | 151 ++++++++++--- crates/wasm-pkg-core/src/wit.rs | 75 ++++++- crates/wasm-pkg-core/tests/fetch.rs | 203 ++++++++++++++++++ .../local-dep-0.1.0/wit/world.wit | 5 + .../local-dep-0.2.0/wit/world.wit | 6 + .../multi-version-local/project/wit/world.wit | 7 + .../multi-version-registry/wit/world.wit | 7 + crates/wkg/src/wit.rs | 6 +- docs/manifest.md | 19 ++ 10 files changed, 552 insertions(+), 40 deletions(-) create mode 100644 crates/wasm-pkg-core/tests/fixtures/multi-version-local/local-dep-0.1.0/wit/world.wit create mode 100644 crates/wasm-pkg-core/tests/fixtures/multi-version-local/local-dep-0.2.0/wit/world.wit create mode 100644 crates/wasm-pkg-core/tests/fixtures/multi-version-local/project/wit/world.wit create mode 100644 crates/wasm-pkg-core/tests/fixtures/multi-version-registry/wit/world.wit diff --git a/crates/wasm-pkg-core/src/manifest.rs b/crates/wasm-pkg-core/src/manifest.rs index 78dd347..76ac69f 100644 --- a/crates/wasm-pkg-core/src/manifest.rs +++ b/crates/wasm-pkg-core/src/manifest.rs @@ -1,13 +1,14 @@ //! Type definitions and functions for working with `wkg.toml` files. use std::{ - collections::HashMap, + collections::{HashMap, HashSet}, path::{Path, PathBuf}, }; use anyhow::{Context, Result}; use semver::VersionReq; use serde::{Deserialize, Serialize}; +use wasm_pkg_common::package::{PackageRef, PackageSpec}; mod paths; pub mod workspace; @@ -79,10 +80,56 @@ impl Manifest { // `Manifest` validations, mirrors cargo's `Workspace::validate` fn validate(&self) -> Result<()> { self.validate_workspace_exclusivity()?; + self.validate_override_keys()?; // Add new validation rules with `self.validate_*()?;` Ok(()) } + /// Checks that override keys parse and that no package is covered by both a bare and a + /// versioned key. + /// + /// Runs when a `wkg.toml` is loaded (see [`validate`](Self::validate)), and again when + /// resolving, since a `Manifest` built directly in Rust code skips the load step. + pub(crate) fn validate_override_keys(&self) -> Result<()> { + let Some(overrides) = self.overrides.as_ref() else { + return Ok(()); + }; + // `overrides` is a map, so walk it in a stable order + let mut sorted_keys: Vec<&String> = overrides.keys().collect(); + sorted_keys.sort_unstable(); + + let mut bare: HashSet = HashSet::new(); + let mut versioned: HashMap> = HashMap::new(); + for key in sorted_keys { + let spec: PackageSpec = key + .parse() + .with_context(|| format!("invalid override key `{key}`"))?; + match spec.version { + Some(_) => versioned.entry(spec.package).or_default().push(key), + None => { + bare.insert(spec.package); + } + } + } + + let mut conflicts: Vec = versioned + .iter() + .filter(|(package, _)| bare.contains(*package)) + .map(|(package, keys)| { + format!( + "override `{package}` applies to every version of the package, so it \ + conflicts with the versioned override(s) `{}`", + keys.join("`, `") + ) + }) + .collect(); + if conflicts.is_empty() { + return Ok(()); + } + conflicts.sort_unstable(); + anyhow::bail!("{} - remove one or the other", conflicts.join("; ")); + } + // no overrides or top-level metadata when workspace is present fn validate_workspace_exclusivity(&self) -> Result<()> { if self.workspace.is_none() { @@ -241,4 +288,68 @@ mod tests { "manifest loaded from file does not match original manifest" ); } + + #[test] + fn override_keys_may_carry_a_version() { + let manifest = Manifest::from_toml( + r#" +[overrides] +"foo:bar@0.1.0" = { path = "bar-0.1.0" } +"foo:bar@0.2.0" = { path = "bar-0.2.0" } +"foo:baz" = { path = "baz" } +"#, + ) + .expect("versioned override keys should be accepted"); + assert_eq!(manifest.overrides.unwrap().len(), 3); + } + + #[test] + fn override_keys_conflict_when_bare_and_versioned() { + let err = Manifest::from_toml( + r#" +[overrides] +"foo:bar" = { path = "bar" } +"foo:bar@0.1.0" = { path = "bar-0.1.0" } +"#, + ) + .expect_err("a bare key alongside a versioned one is ambiguous"); + let err = format!("{err:#}"); + assert!(err.contains("foo:bar@0.1.0"), "unexpected error: {err}"); + } + + #[test] + fn override_key_conflicts_are_all_reported_in_a_stable_order() { + // Two conflicting packages: both must appear, and always in the same order, rather than + // whichever the underlying map happened to yield first. + let err = Manifest::from_toml( + r#" +[overrides] +"zzz:two" = { path = "z" } +"zzz:two@0.2.0" = { path = "z2" } +"aaa:one" = { path = "a" } +"aaa:one@0.1.0" = { path = "a1" } +"#, + ) + .expect_err("both packages conflict"); + let err = format!("{err:#}"); + let aaa = err.find("aaa:one").expect("aaa:one should be reported"); + let zzz = err.find("zzz:two").expect("zzz:two should be reported"); + assert!(aaa < zzz, "conflicts should be sorted: {err}"); + } + + #[test] + fn override_keys_must_parse() { + let err = Manifest::from_toml( + r#" +[overrides] +"not a package ref" = { path = "bar" } +"#, + ) + .expect_err("an unparseable override key should be rejected"); + let err = format!("{err:#}"); + assert!( + err.contains("invalid override key"), + "unexpected error: {err}" + ); + } } diff --git a/crates/wasm-pkg-core/src/resolver.rs b/crates/wasm-pkg-core/src/resolver.rs index 08bba5c..df3d9af 100644 --- a/crates/wasm-pkg-core/src/resolver.rs +++ b/crates/wasm-pkg-core/src/resolver.rs @@ -104,6 +104,67 @@ impl FromStr for RegistryPackage { } } +/// The key identifying a dependency in the resolver. +/// +/// A single world can name more than one version of the same package — a component exporting both +/// `ns:pkg/interface@0.2.0` and `@0.3.0`, for example — so a [`PackageRef`] on its own +/// does not uniquely identify a dependency. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct DependencyKey { + /// The package the dependency refers to. + pub package: PackageRef, + /// The version this dependency was requested at. + /// + /// `None` means "every version". This only happens for a bare override key (no `@version`), + /// which applies to every version the WIT names. + pub version: Option, +} + +impl DependencyKey { + pub fn new(package: PackageRef, version: VersionReq) -> Self { + Self { + package, + version: Some(version), + } + } + + /// Creates a key that applies to every version of the given package. + pub fn any_version(package: PackageRef) -> Self { + Self { + package, + version: None, + } + } + + /// Returns whether this key applies to every version of its package. + pub fn is_any_version(&self) -> bool { + self.version.is_none() + } + + /// Returns whether resolving this key also satisfies `other`, making `other` redundant. + /// + /// `foo:bar` has no version, so it satisfies every key for `foo:bar`. `foo:bar@0.1.0` + /// satisfies only itself. + pub fn supersedes(&self, other: &DependencyKey) -> bool { + self.package == other.package && (self.is_any_version() || self.version == other.version) + } +} + +impl From for DependencyKey { + fn from(package: PackageRef) -> Self { + Self::any_version(package) + } +} + +impl std::fmt::Display for DependencyKey { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match &self.version { + Some(version) => write!(f, "{}@{version}", self.package), + None => write!(f, "{}", self.package), + } + } +} + /// Represents information about a resolution of a registry package. #[derive(Clone)] pub struct RegistryResolution { @@ -348,7 +409,8 @@ pub struct DependencyResolver<'a> { client: CachingClient, lock_file: Option<&'a LockFile>, packages: HashMap>, - dependencies: HashMap, + dependencies: HashMap, + any_version_overrides: HashSet, resolutions: DependencyResolutionMap, } @@ -371,6 +433,7 @@ impl<'a> DependencyResolver<'a> { resolutions: Default::default(), packages: Default::default(), dependencies: Default::default(), + any_version_overrides: Default::default(), }) } @@ -390,6 +453,7 @@ impl<'a> DependencyResolver<'a> { resolutions: Default::default(), packages: Default::default(), dependencies: Default::default(), + any_version_overrides: Default::default(), }) } @@ -397,28 +461,58 @@ impl<'a> DependencyResolver<'a> { /// To override an existing dependency, use [`override_dependency`](Self::override_dependency). pub async fn add_dependency( &mut self, - name: &PackageRef, + key: &DependencyKey, dependency: &Dependency, ) -> Result<()> { - self.add_dependency_internal(name, dependency, false).await + self.add_dependency_internal(key, dependency, false).await } /// Add a dependency to the resolver. If the dependency already exists, then it will be /// overridden. pub async fn override_dependency( &mut self, - name: &PackageRef, + key: &DependencyKey, dependency: &Dependency, ) -> Result<()> { - self.add_dependency_internal(name, dependency, true).await + self.add_dependency_internal(key, dependency, true).await + } + + /// Returns whether this key has already been handled: it was added before, or a bare + /// override for the whole package already covers it. + fn is_already_superseded(&self, key: &DependencyKey) -> bool { + // A bare override covers every version of a package, even ones we haven't added a + // dependency for yet, so we keep track of it separately from the maps below. + if !key.is_any_version() && self.any_version_overrides.contains(&key.package) { + return true; + } + self.resolutions.contains_key(key) || self.dependencies.contains_key(key) + } + + /// Removes any registry dependency that this key replaces. Returns true if something was + /// removed. + fn take_superseded_dependencies(&mut self, key: &DependencyKey) -> bool { + let before = self.dependencies.len(); + self.dependencies.retain(|k, _| !key.supersedes(k)); + self.dependencies.len() != before + } + + /// Returns whether a resolution that this key replaces has already been recorded. + fn has_superseded_resolution(&self, key: &DependencyKey) -> bool { + self.resolutions.keys().any(|k| key.supersedes(k)) } async fn add_dependency_internal( &mut self, - name: &PackageRef, + key: &DependencyKey, dependency: &Dependency, force_override: bool, ) -> Result<()> { + let name = &key.package; + // Record this first: a versioned request for the same package later in this resolution + // will see it and be skipped. + if key.is_any_version() { + self.any_version_overrides.insert(name.clone()); + } match dependency { Dependency::Package(package) => { // Dependency comes from a registry, add a dependency to the resolver @@ -445,14 +539,12 @@ impl<'a> DependencyResolver<'a> { // So if it wasn't already fetched first? then we'll try and resolve it later, and the override // is not present there for some reason - if !force_override - && (self.resolutions.contains_key(name) || self.dependencies.contains_key(name)) - { - tracing::debug!(%name, %dependency, "dependency already exists and override is not set, ignoring"); + if !force_override && self.is_already_superseded(key) { + tracing::debug!(%key, %dependency, "dependency already exists and override is not set, ignoring"); return Ok(()); } self.dependencies.insert( - name.to_owned(), + key.to_owned(), RegistryDependency { package: package_name, version: package.version.clone(), @@ -471,19 +563,14 @@ impl<'a> DependencyResolver<'a> { // deps as registry deps. So if we're handling a local path and the dependencies // have a registry package already, override it. Otherwise follow normal overrides. // We should definitely fix this and change where we resolve these things - let should_insert = force_override - || self.dependencies.contains_key(name) - || !self.resolutions.contains_key(name); + let superseded = self.take_superseded_dependencies(key); + let should_insert = + force_override || superseded || !self.has_superseded_resolution(key); if !should_insert { - tracing::debug!(%name, "dependency already exists and registry override is not set, ignoring"); + tracing::debug!(%key, "dependency already exists and registry override is not set, ignoring"); return Ok(()); } - // Because we got here, we should remove anything from dependencies that is the same - // package because we're overriding with the local package. Technically we could be - // clever and just do this in the boolean above, but I'm paranoid - self.dependencies.remove(name); - // Now that we check we haven't already inserted this dep, get the packages from the // local dependency and add those to the resolver before adding the dependency let (_, packages) = get_packages(p) @@ -492,7 +579,7 @@ impl<'a> DependencyResolver<'a> { .await .context("Error adding packages to resolver for local dependency")?; - let prev = self.resolutions.insert(name.clone(), res); + let prev = self.resolutions.insert(key.clone(), res); assert!(prev.is_none()); } } @@ -508,9 +595,9 @@ impl<'a> DependencyResolver<'a> { ) -> Result<()> { for (package, req) in packages { self.add_dependency( - &package, + &DependencyKey::new(package.clone(), req.clone()), &Dependency::Package(RegistryPackage { - name: Some(package.clone()), + name: Some(package), version: req, registry: None, }), @@ -527,7 +614,8 @@ impl<'a> DependencyResolver<'a> { /// Returns the dependency resolution map. pub async fn resolve(mut self) -> Result { let mut resolutions = self.resolutions; - for (name, dependency) in self.dependencies.into_iter() { + for (key, dependency) in self.dependencies.into_iter() { + let name = &key.package; // We need to clone a handle to the client because we mutably borrow self below. Might // be worth replacing the mutable borrow with a RwLock down the line. let client = self.client.clone(); @@ -604,7 +692,7 @@ impl<'a> DependencyResolver<'a> { registry: self.client.client().ok().and_then(|client| { client .config() - .resolve_registry(&name) + .resolve_registry(name) .map(ToString::to_string) }), requirement: dependency.version.clone(), @@ -612,7 +700,7 @@ impl<'a> DependencyResolver<'a> { digest: release.content_digest.clone(), client: self.client.clone(), }; - resolutions.insert(name, DependencyResolution::Registry(resolution)); + resolutions.insert(key, DependencyResolution::Registry(resolution)); } Ok(resolutions) @@ -660,18 +748,19 @@ fn find_latest_release<'a>( /// Represents a map of dependency resolutions. /// -/// The key to the map is the package name of the dependency. +/// Each key is a dependency's package plus the version it was requested at. This lets a world +/// naming several versions of the same package resolve all of them, not just one. #[derive(Debug, Clone, Default)] -pub struct DependencyResolutionMap(HashMap); +pub struct DependencyResolutionMap(HashMap); -impl AsRef> for DependencyResolutionMap { - fn as_ref(&self) -> &HashMap { +impl AsRef> for DependencyResolutionMap { + fn as_ref(&self) -> &HashMap { &self.0 } } impl Deref for DependencyResolutionMap { - type Target = HashMap; + type Target = HashMap; fn deref(&self) -> &Self::Target { &self.0 diff --git a/crates/wasm-pkg-core/src/wit.rs b/crates/wasm-pkg-core/src/wit.rs index 5fcf934..1bb0eb2 100644 --- a/crates/wasm-pkg-core/src/wit.rs +++ b/crates/wasm-pkg-core/src/wit.rs @@ -23,7 +23,7 @@ use crate::{ lock::LockFile, manifest::Manifest, resolver::{ - DecodedDependency, Dependency, DependencyGraph, DependencyResolution, + DecodedDependency, Dependency, DependencyGraph, DependencyKey, DependencyResolution, DependencyResolutionMap, DependencyResolver, LocalPackageIndex, LocalResolution, RegistryPackage, }, @@ -279,16 +279,25 @@ pub async fn resolve_dependencies( lock_file: Option<&LockFile>, client: CachingClient, ) -> Result { + manifest.validate_override_keys()?; + let mut resolver = DependencyResolver::new_with_client(client, lock_file)?; // add deps from manifest first in case they're local deps and then add deps from the directory if let Some(overrides) = manifest.overrides.as_ref() { tracing::debug!("detected manifest overrides"); for (pkg, ovr) in overrides.iter() { - let pkg: PackageRef = pkg.parse().context("Unable to parse as a package ref")?; + let key = + parse_override_key(pkg).with_context(|| format!("invalid override key `{pkg}`"))?; let dep = match (ovr.path.as_ref(), ovr.version.as_ref()) { (Some(path), v) => { if v.is_some() { - tracing::warn!("Ignoring version override for local package"); + tracing::warn!( + %key, + "Ignoring `version` field for local override; to scope an override to \ + a single version, put the version in the key (e.g. \ + `\"{}@1.2.3\"`)", + key.package, + ); } let path = tokio::fs::canonicalize(path).await.with_context(|| { format!("resolving local dependency {}", path.display()) @@ -296,7 +305,7 @@ pub async fn resolve_dependencies( Dependency::Local(path) } (None, Some(version)) => Dependency::Package(RegistryPackage { - name: Some(pkg.clone()), + name: Some(key.package.clone()), version: version.to_owned(), registry: None, }), @@ -308,7 +317,7 @@ pub async fn resolve_dependencies( tracing::debug!(dependency = %dep); resolver - .add_dependency(&pkg, &dep) + .add_dependency(&key, &dep) .await .with_context(|| format!("unable to add dependency {dep}"))?; } @@ -417,6 +426,26 @@ async fn write_wasm_deps( Ok(()) } +/// Parses a key from the manifest's `[overrides]` table. +/// +/// A bare package name (`"ns:pkg"`) applies to every version of that package, same as before. +/// Adding an exact version (`"ns:pkg@1.2.3"`) limits the override to just that version, so a world +/// with two versions of one package can point each version somewhere different. +fn parse_override_key(key: &str) -> Result { + let spec: PackageSpec = key.parse().context("Unable to parse as a package ref")?; + Ok(match spec.version { + // Built the same way as `packages_from_foreign_deps` does, so this matches the requirement + // the WIT's own `@version` produces. + Some(version) => DependencyKey::new( + spec.package, + format!("={version}") + .parse() + .expect("an exact version is always a valid requirement"), + ), + None => DependencyKey::any_version(spec.package), + }) +} + fn packages_from_foreign_deps( deps: impl IntoIterator, ) -> impl Iterator { @@ -551,3 +580,39 @@ fn name_from_package_name(package_name: &PackageName) -> String { let package_name_str = package_name.to_string(); package_name_str.replace([':', '@'], "-") } + +#[cfg(test)] +mod tests { + use super::*; + + /// A versioned override key only matches its dependency if it builds the same requirement the + /// WIT's `@version` does — these two need to always agree. + #[test] + fn override_key_requirement_matches_foreign_dep() { + for version in ["0.1.0", "1.2.3", "0.2.0-draft", "0.2.0-alpha.1"] { + let from_wit = packages_from_foreign_deps([PackageName { + namespace: "foo".to_string(), + name: "bar".to_string(), + version: Some(version.parse().unwrap()), + }]) + .next() + .expect("foreign dep should yield a package"); + + let from_key = parse_override_key(&format!("foo:bar@{version}")).unwrap(); + + assert_eq!(from_key.package, from_wit.0, "package for {version}"); + assert_eq!( + from_key.version, + Some(from_wit.1), + "requirement for {version}" + ); + } + } + + #[test] + fn malformed_override_keys_are_rejected() { + for key in ["foo:bar@", "foo:bar@not-a-version", "not a package ref"] { + assert!(parse_override_key(key).is_err(), "`{key}` should not parse"); + } + } +} diff --git a/crates/wasm-pkg-core/tests/fetch.rs b/crates/wasm-pkg-core/tests/fetch.rs index a56685c..4650fe9 100644 --- a/crates/wasm-pkg-core/tests/fetch.rs +++ b/crates/wasm-pkg-core/tests/fetch.rs @@ -158,6 +158,209 @@ async fn test_transitive_local(#[values(OutputType::Wasm, OutputType::Wit)] outp ); } +/// A world can name several versions of the same package, so an override key may carry an exact +/// version to scope it to just one of them. +#[rstest] +#[tokio::test] +async fn test_multi_version_local(#[values(OutputType::Wasm, OutputType::Wit)] output: OutputType) { + let (_temp, fixture_path) = common::load_fixture("multi-version-local").await.unwrap(); + let project_path = fixture_path.join("project"); + let lock_file = project_path.join("wkg.lock"); + let mut lock = LockFile::new_with_path([], &lock_file) + .await + .expect("Should be able to create a new lock file"); + // ```toml + // [overrides] + // "my:local@0.1.0" = { "path" = "../local-dep-0.1.0/wit" } + // "my:local@0.2.0" = { "path" = "../local-dep-0.2.0/wit" } + // ``` + let manifest = Manifest { + overrides: Some(HashMap::from([ + ( + "my:local@0.1.0".to_string(), + Override { + path: Some(fixture_path.join("local-dep-0.1.0/wit")), + version: None, + }, + ), + ( + "my:local@0.2.0".to_string(), + Override { + path: Some(fixture_path.join("local-dep-0.2.0/wit")), + version: None, + }, + ), + ])), + ..Default::default() + }; + let (_temp_cache, client) = common::get_client().await.unwrap(); + + wit::fetch_dependencies( + &manifest, + project_path.join("wit"), + &mut lock, + client, + output, + ) + .await + .unwrap_or_else(|e| panic!("Should be able to fetch the dependencies: {e:#}")); + + // Both versions must be written out. Before per-version override keys, the two entries + // collided on the package name and only one of them (nondeterministically) survived. + let mut deps_dir = tokio::fs::read_dir(project_path.join("wit/deps")) + .await + .expect("Should be able to read the deps directory"); + let mut deps = Vec::new(); + while let Ok(Some(entry)) = deps_dir.next_entry().await { + deps.push(entry.file_name().to_string_lossy().to_string()); + } + // Local directory deps are written out as directories under both output types + assert_eq!(deps.len(), 2, "expected both versions, got {deps:?}"); + assert!( + deps.contains(&"my-local-0.1.0".to_string()), + "missing my-local-0.1.0 in {deps:?}" + ); + assert!( + deps.contains(&"my-local-0.2.0".to_string()), + "missing my-local-0.2.0 in {deps:?}" + ); + + // each directory must hold its own version's WIT, not two copies of whichever one won + let v1 = read_dep_wit(&project_path.join("wit/deps/my-local-0.1.0")).await; + let v2 = read_dep_wit(&project_path.join("wit/deps/my-local-0.2.0")).await; + assert!(v1.contains("foo: func() -> string"), "0.1.0 body was: {v1}"); + assert!( + v2.contains("foo: func() -> result"), + "0.2.0 body was: {v2}" + ); + + // All dependencies are local, so the lock file should be empty + assert_eq!( + lock.packages.len(), + 0, + "Should have the correct number of packages in the lock file" + ); +} + +/// The same applies to packages coming from a registry rather than from an override: both +/// versions must be fetched and both must be recorded against the one package in the lock file. +#[rstest] +#[tokio::test] +async fn test_multi_version_registry( + #[values(OutputType::Wasm, OutputType::Wit)] output: OutputType, +) { + let (_temp, fixture_path) = common::load_fixture("multi-version-registry") + .await + .unwrap(); + let lock_file = fixture_path.join("wkg.lock"); + let mut lock = LockFile::new_with_path([], &lock_file) + .await + .expect("Should be able to create a new lock file"); + let (_temp_cache, client) = common::get_client().await.unwrap(); + + wit::fetch_dependencies( + &Manifest::default(), + fixture_path.join("wit"), + &mut lock, + client, + output, + ) + .await + .unwrap_or_else(|e| panic!("Should be able to fetch the dependencies: {e:#}")); + + let mut deps_dir = tokio::fs::read_dir(fixture_path.join("wit/deps")) + .await + .expect("Should be able to read the deps directory"); + let mut deps = Vec::new(); + while let Ok(Some(entry)) = deps_dir.next_entry().await { + deps.push(entry.file_name().to_string_lossy().to_string()); + } + let suffix = match output { + OutputType::Wit => "", + OutputType::Wasm => ".wasm", + }; + for version in ["0.2.0", "0.2.1"] { + let expected = format!("wasi-clocks-{version}{suffix}"); + assert!(deps.contains(&expected), "missing {expected} in {deps:?}"); + } + + // Both requirements belong to the one `wasi:clocks` package, so the lock file records a single + // package holding two locked versions. + let clocks = lock + .packages + .iter() + .find(|p| p.name.to_string() == "wasi:clocks") + .expect("wasi:clocks should be in the lock file"); + let mut locked: Vec = clocks + .versions + .iter() + .map(|v| v.version.to_string()) + .collect(); + locked.sort(); + assert_eq!(locked, ["0.2.0", "0.2.1"], "lock file versions: {locked:?}"); +} + +/// A bare key already covers every version, so pairing it with a versioned key for the same +/// package is ambiguous. Manifests built through the library API skip the load-time check, so +/// resolving has to reject the combination too rather than pick a winner by map ordering. +#[tokio::test] +async fn test_conflicting_override_keys_rejected() { + let (_temp, fixture_path) = common::load_fixture("multi-version-local").await.unwrap(); + let project_path = fixture_path.join("project"); + let mut lock = LockFile::new_with_path([], project_path.join("wkg.lock")) + .await + .expect("Should be able to create a new lock file"); + // Built directly rather than parsed from TOML, so `Manifest::validate` never ran + let manifest = Manifest { + overrides: Some(HashMap::from([ + ( + "my:local".to_string(), + Override { + path: Some(fixture_path.join("local-dep-0.1.0/wit")), + version: None, + }, + ), + ( + "my:local@0.2.0".to_string(), + Override { + path: Some(fixture_path.join("local-dep-0.2.0/wit")), + version: None, + }, + ), + ])), + ..Default::default() + }; + let (_temp_cache, client) = common::get_client().await.unwrap(); + + let err = wit::fetch_dependencies( + &manifest, + project_path.join("wit"), + &mut lock, + client, + OutputType::Wit, + ) + .await + .expect_err("conflicting override keys should be rejected"); + + let err = format!("{err:#}"); + assert!( + err.contains("my:local@0.2.0") && err.contains("conflicts"), + "unexpected error: {err}" + ); +} + +/// Concatenates every WIT file in a dep directory, whose name differs by output type. +async fn read_dep_wit(dir: &Path) -> String { + let mut entries = tokio::fs::read_dir(dir) + .await + .unwrap_or_else(|e| panic!("Should be able to read {}: {e}", dir.display())); + let mut contents = String::new(); + while let Ok(Some(entry)) = entries.next_entry().await { + contents.push_str(&tokio::fs::read_to_string(entry.path()).await.unwrap()); + } + contents +} + async fn build_component(fixture_path: &Path) { let output = Command::new(env!("CARGO")) .current_dir(fixture_path) diff --git a/crates/wasm-pkg-core/tests/fixtures/multi-version-local/local-dep-0.1.0/wit/world.wit b/crates/wasm-pkg-core/tests/fixtures/multi-version-local/local-dep-0.1.0/wit/world.wit new file mode 100644 index 0000000..6c0518a --- /dev/null +++ b/crates/wasm-pkg-core/tests/fixtures/multi-version-local/local-dep-0.1.0/wit/world.wit @@ -0,0 +1,5 @@ +package my:local@0.1.0; + +interface foo { + foo: func() -> string; +} diff --git a/crates/wasm-pkg-core/tests/fixtures/multi-version-local/local-dep-0.2.0/wit/world.wit b/crates/wasm-pkg-core/tests/fixtures/multi-version-local/local-dep-0.2.0/wit/world.wit new file mode 100644 index 0000000..8736692 --- /dev/null +++ b/crates/wasm-pkg-core/tests/fixtures/multi-version-local/local-dep-0.2.0/wit/world.wit @@ -0,0 +1,6 @@ +package my:local@0.2.0; + +interface foo { + // Differs from 0.1.0 so a test can tell which version was written out. + foo: func() -> result; +} diff --git a/crates/wasm-pkg-core/tests/fixtures/multi-version-local/project/wit/world.wit b/crates/wasm-pkg-core/tests/fixtures/multi-version-local/project/wit/world.wit new file mode 100644 index 0000000..a3f1a19 --- /dev/null +++ b/crates/wasm-pkg-core/tests/fixtures/multi-version-local/project/wit/world.wit @@ -0,0 +1,7 @@ +package my:component; + +// Exports two versions of the same package, which is only expressible with per-version overrides. +world component { + export my:local/foo@0.1.0; + export my:local/foo@0.2.0; +} diff --git a/crates/wasm-pkg-core/tests/fixtures/multi-version-registry/wit/world.wit b/crates/wasm-pkg-core/tests/fixtures/multi-version-registry/wit/world.wit new file mode 100644 index 0000000..1ba826a --- /dev/null +++ b/crates/wasm-pkg-core/tests/fixtures/multi-version-registry/wit/world.wit @@ -0,0 +1,7 @@ +package test:multiversion; + +// Imports two versions of the same registry package, which must both be fetched. +world multiversion { + import wasi:clocks/monotonic-clock@0.2.0; + import wasi:clocks/monotonic-clock@0.2.1; +} diff --git a/crates/wkg/src/wit.rs b/crates/wkg/src/wit.rs index 76cb811..2782699 100644 --- a/crates/wkg/src/wit.rs +++ b/crates/wkg/src/wit.rs @@ -238,11 +238,11 @@ impl FetchArgs { .with_context(|| { format!("failed to resolve dependencies for {}", dir.display()) })?; - for (pkg, resolution) in resolved.as_ref() { - if verifier.packages.contains(pkg) { + for (key, resolution) in resolved.as_ref() { + if verifier.packages.contains(&key.package) { continue; } - merged.insert(pkg.clone(), resolution.clone()); + merged.insert(key.clone(), resolution.clone()); } } diff --git a/docs/manifest.md b/docs/manifest.md index bd6b2c9..de2feb2 100644 --- a/docs/manifest.md +++ b/docs/manifest.md @@ -54,6 +54,25 @@ developing two components together. "my:local-dep" = { path = "../local-dep/wit" } ``` +A bare package name applies to every version of that package the WIT names. To +scope an override to one version, suffix the key with that exact version. This +is what lets a world name more than one version of the same package, since each +version can then point somewhere different: + +```toml +[overrides] +"my:local-dep@0.1.0" = { path = "../local-dep-0.1.0/wit" } +"my:local-dep@0.2.0" = { path = "../local-dep-0.2.0/wit" } +``` + +A package cannot have both a bare and a versioned key, since the bare key +already covers every version; such a manifest is rejected rather than resolved +in an unspecified order. + +Note that the `version` field is a *registry* version requirement and is +ignored when `path` is set — use a versioned key to select which version an +override applies to. + ### `workspace.members` - Type: list of strings (paths; gitignore-style globs allowed) From 5fa186f53bcfd2a2c1319d9f641d8e771b633405 Mon Sep 17 00:00:00 2001 From: Aditya Salunkhe Date: Sat, 22 Aug 2026 02:18:08 +0530 Subject: [PATCH 2/3] refactor(core): key dependencies by PackageSpec instead of DependencyKey --- crates/wasm-pkg-common/src/package.rs | 2 +- crates/wasm-pkg-core/src/manifest.rs | 5 +- crates/wasm-pkg-core/src/resolver.rs | 129 +++++++++----------------- crates/wasm-pkg-core/src/wit.rs | 95 ++++++++----------- 4 files changed, 90 insertions(+), 141 deletions(-) diff --git a/crates/wasm-pkg-common/src/package.rs b/crates/wasm-pkg-common/src/package.rs index 5df67d2..9329919 100644 --- a/crates/wasm-pkg-common/src/package.rs +++ b/crates/wasm-pkg-common/src/package.rs @@ -79,7 +79,7 @@ impl std::fmt::Display for PackageSpec { } /// A package spec combines a [`PackageRef`] with an optional version. -#[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord)] +#[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Hash)] pub struct PackageSpec { pub package: PackageRef, pub version: Option, diff --git a/crates/wasm-pkg-core/src/manifest.rs b/crates/wasm-pkg-core/src/manifest.rs index 76ac69f..d4651b3 100644 --- a/crates/wasm-pkg-core/src/manifest.rs +++ b/crates/wasm-pkg-core/src/manifest.rs @@ -1,7 +1,7 @@ //! Type definitions and functions for working with `wkg.toml` files. use std::{ - collections::{HashMap, HashSet}, + collections::{BTreeSet, HashMap, HashSet}, path::{Path, PathBuf}, }; @@ -95,8 +95,7 @@ impl Manifest { return Ok(()); }; // `overrides` is a map, so walk it in a stable order - let mut sorted_keys: Vec<&String> = overrides.keys().collect(); - sorted_keys.sort_unstable(); + let sorted_keys: BTreeSet<&str> = overrides.keys().map(String::as_str).collect(); let mut bare: HashSet = HashSet::new(); let mut versioned: HashMap> = HashMap::new(); diff --git a/crates/wasm-pkg-core/src/resolver.rs b/crates/wasm-pkg-core/src/resolver.rs index df3d9af..684fd25 100644 --- a/crates/wasm-pkg-core/src/resolver.rs +++ b/crates/wasm-pkg-core/src/resolver.rs @@ -104,64 +104,19 @@ impl FromStr for RegistryPackage { } } -/// The key identifying a dependency in the resolver. -/// -/// A single world can name more than one version of the same package — a component exporting both -/// `ns:pkg/interface@0.2.0` and `@0.3.0`, for example — so a [`PackageRef`] on its own -/// does not uniquely identify a dependency. -#[derive(Debug, Clone, PartialEq, Eq, Hash)] -pub struct DependencyKey { - /// The package the dependency refers to. - pub package: PackageRef, - /// The version this dependency was requested at. - /// - /// `None` means "every version". This only happens for a bare override key (no `@version`), - /// which applies to every version the WIT names. - pub version: Option, +/// Returns whether resolving `key` also satisfies `other`. A key without a version (`foo:bar`) +/// covers every version of its package; `foo:bar@0.1.0` covers only itself. +fn key_supersedes(key: &PackageSpec, other: &PackageSpec) -> bool { + key.package == other.package && (key.version.is_none() || key.version == other.version) } -impl DependencyKey { - pub fn new(package: PackageRef, version: VersionReq) -> Self { - Self { - package, - version: Some(version), - } - } - - /// Creates a key that applies to every version of the given package. - pub fn any_version(package: PackageRef) -> Self { - Self { - package, - version: None, - } - } - - /// Returns whether this key applies to every version of its package. - pub fn is_any_version(&self) -> bool { - self.version.is_none() - } - - /// Returns whether resolving this key also satisfies `other`, making `other` redundant. - /// - /// `foo:bar` has no version, so it satisfies every key for `foo:bar`. `foo:bar@0.1.0` - /// satisfies only itself. - pub fn supersedes(&self, other: &DependencyKey) -> bool { - self.package == other.package && (self.is_any_version() || self.version == other.version) - } -} - -impl From for DependencyKey { - fn from(package: PackageRef) -> Self { - Self::any_version(package) - } -} - -impl std::fmt::Display for DependencyKey { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match &self.version { - Some(version) => write!(f, "{}@{version}", self.package), - None => write!(f, "{}", self.package), - } +/// The version requirement a dependency key asks for. +pub(crate) fn key_requirement(key: &PackageSpec) -> VersionReq { + match &key.version { + Some(version) => format!("={version}") + .parse() + .expect("an exact version is always a valid requirement"), + None => VersionReq::STAR, } } @@ -409,7 +364,7 @@ pub struct DependencyResolver<'a> { client: CachingClient, lock_file: Option<&'a LockFile>, packages: HashMap>, - dependencies: HashMap, + dependencies: HashMap, any_version_overrides: HashSet, resolutions: DependencyResolutionMap, } @@ -459,11 +414,14 @@ impl<'a> DependencyResolver<'a> { /// Add a dependency to the resolver. If the dependency already exists, then it will be ignored. /// To override an existing dependency, use [`override_dependency`](Self::override_dependency). + /// + /// A key without a version covers every version of the package. pub async fn add_dependency( &mut self, - key: &DependencyKey, + key: &PackageSpec, dependency: &Dependency, ) -> Result<()> { + self.record_any_version_override(key); self.add_dependency_internal(key, dependency, false).await } @@ -471,18 +429,28 @@ impl<'a> DependencyResolver<'a> { /// overridden. pub async fn override_dependency( &mut self, - key: &DependencyKey, + key: &PackageSpec, dependency: &Dependency, ) -> Result<()> { + self.record_any_version_override(key); self.add_dependency_internal(key, dependency, true).await } + /// Recorded before the dependency itself is added, so that versioned requests for the same + /// package arriving later are skipped. Only overrides get here: an unversioned WIT import is + /// a single dependency with no version, not an override covering every version. + fn record_any_version_override(&mut self, key: &PackageSpec) { + if key.version.is_none() { + self.any_version_overrides.insert(key.package.clone()); + } + } + /// Returns whether this key has already been handled: it was added before, or a bare /// override for the whole package already covers it. - fn is_already_superseded(&self, key: &DependencyKey) -> bool { + fn is_already_superseded(&self, key: &PackageSpec) -> bool { // A bare override covers every version of a package, even ones we haven't added a // dependency for yet, so we keep track of it separately from the maps below. - if !key.is_any_version() && self.any_version_overrides.contains(&key.package) { + if key.version.is_some() && self.any_version_overrides.contains(&key.package) { return true; } self.resolutions.contains_key(key) || self.dependencies.contains_key(key) @@ -490,29 +458,24 @@ impl<'a> DependencyResolver<'a> { /// Removes any registry dependency that this key replaces. Returns true if something was /// removed. - fn take_superseded_dependencies(&mut self, key: &DependencyKey) -> bool { + fn take_superseded_dependencies(&mut self, key: &PackageSpec) -> bool { let before = self.dependencies.len(); - self.dependencies.retain(|k, _| !key.supersedes(k)); + self.dependencies.retain(|k, _| !key_supersedes(key, k)); self.dependencies.len() != before } /// Returns whether a resolution that this key replaces has already been recorded. - fn has_superseded_resolution(&self, key: &DependencyKey) -> bool { - self.resolutions.keys().any(|k| key.supersedes(k)) + fn has_superseded_resolution(&self, key: &PackageSpec) -> bool { + self.resolutions.keys().any(|k| key_supersedes(key, k)) } async fn add_dependency_internal( &mut self, - key: &DependencyKey, + key: &PackageSpec, dependency: &Dependency, force_override: bool, ) -> Result<()> { let name = &key.package; - // Record this first: a versioned request for the same package later in this resolution - // will see it and be skipped. - if key.is_any_version() { - self.any_version_overrides.insert(name.clone()); - } match dependency { Dependency::Package(package) => { // Dependency comes from a registry, add a dependency to the resolver @@ -587,20 +550,20 @@ impl<'a> DependencyResolver<'a> { Ok(()) } - /// A helper function for adding an iterator of package refs and their associated version - /// requirements to the resolver + /// A helper function for adding an iterator of packages named by a WIT file to the resolver. pub async fn add_packages( &mut self, - packages: impl IntoIterator, + packages: impl IntoIterator, ) -> Result<()> { - for (package, req) in packages { - self.add_dependency( - &DependencyKey::new(package.clone(), req.clone()), + for spec in packages { + self.add_dependency_internal( + &spec, &Dependency::Package(RegistryPackage { - name: Some(package), - version: req, + name: Some(spec.package.clone()), + version: key_requirement(&spec), registry: None, }), + false, ) .await?; } @@ -751,16 +714,16 @@ fn find_latest_release<'a>( /// Each key is a dependency's package plus the version it was requested at. This lets a world /// naming several versions of the same package resolve all of them, not just one. #[derive(Debug, Clone, Default)] -pub struct DependencyResolutionMap(HashMap); +pub struct DependencyResolutionMap(HashMap); -impl AsRef> for DependencyResolutionMap { - fn as_ref(&self) -> &HashMap { +impl AsRef> for DependencyResolutionMap { + fn as_ref(&self) -> &HashMap { &self.0 } } impl Deref for DependencyResolutionMap { - type Target = HashMap; + type Target = HashMap; fn deref(&self) -> &Self::Target { &self.0 diff --git a/crates/wasm-pkg-core/src/wit.rs b/crates/wasm-pkg-core/src/wit.rs index 1bb0eb2..773acbd 100644 --- a/crates/wasm-pkg-core/src/wit.rs +++ b/crates/wasm-pkg-core/src/wit.rs @@ -9,7 +9,7 @@ use std::{ use anyhow::{Context as _, Result, bail}; use indexmap::IndexMap; use petgraph::{Direction, data::Build}; -use semver::{Version, VersionReq}; +use semver::Version; use wasm_metadata::{AddMetadata, AddMetadataField}; use wasm_pkg_client::{ PackageRef, @@ -23,7 +23,7 @@ use crate::{ lock::LockFile, manifest::Manifest, resolver::{ - DecodedDependency, Dependency, DependencyGraph, DependencyKey, DependencyResolution, + DecodedDependency, Dependency, DependencyGraph, DependencyResolution, DependencyResolutionMap, DependencyResolver, LocalPackageIndex, LocalResolution, RegistryPackage, }, @@ -146,14 +146,12 @@ pub async fn fetch_dependencies( populate_dependencies(wit_dir, &dependencies, output).await } -/// Generate the list of all packages and their version requirement from the given path (a directory -/// or file). +/// Generate the list of all packages named by the given path (a directory or file), each with the +/// version the WIT asks for, if any. /// /// This is a lower level function exposed for convenience that is used by higher level functions /// for resolving dependencies. -pub fn get_packages( - path: impl AsRef, -) -> Result<(PackageSpec, HashSet<(PackageRef, VersionReq)>)> { +pub fn get_packages(path: impl AsRef) -> Result<(PackageSpec, HashSet)> { let path = path.as_ref(); // Build a package group out of a single file or a directory @@ -201,7 +199,7 @@ pub fn get_packages( }; // Get all package refs from the main package and then from any nested packages - let packages: HashSet<(PackageRef, VersionReq)> = + let packages: HashSet = packages_from_foreign_deps(group.main.foreign_deps.into_keys()) .chain( group @@ -241,7 +239,7 @@ pub(crate) fn get_local_dependencies( } for ((spec, _), deps) in pkg_trees { // TODO handle version matching for dependencies - for (dep, _version) in deps { + for PackageSpec { package: dep, .. } in deps { if let Some(&(dep, _)) = indices.get(&dep) { let pkg = &spec.package; let (id, _) = indices[pkg]; @@ -286,8 +284,10 @@ pub async fn resolve_dependencies( if let Some(overrides) = manifest.overrides.as_ref() { tracing::debug!("detected manifest overrides"); for (pkg, ovr) in overrides.iter() { - let key = - parse_override_key(pkg).with_context(|| format!("invalid override key `{pkg}`"))?; + // `"ns:pkg"` overrides every version of the package, `"ns:pkg@1.2.3"` just that one + let key: PackageSpec = pkg + .parse() + .with_context(|| format!("invalid override key `{pkg}`"))?; let dep = match (ovr.path.as_ref(), ovr.version.as_ref()) { (Some(path), v) => { if v.is_some() { @@ -426,41 +426,15 @@ async fn write_wasm_deps( Ok(()) } -/// Parses a key from the manifest's `[overrides]` table. -/// -/// A bare package name (`"ns:pkg"`) applies to every version of that package, same as before. -/// Adding an exact version (`"ns:pkg@1.2.3"`) limits the override to just that version, so a world -/// with two versions of one package can point each version somewhere different. -fn parse_override_key(key: &str) -> Result { - let spec: PackageSpec = key.parse().context("Unable to parse as a package ref")?; - Ok(match spec.version { - // Built the same way as `packages_from_foreign_deps` does, so this matches the requirement - // the WIT's own `@version` produces. - Some(version) => DependencyKey::new( - spec.package, - format!("={version}") - .parse() - .expect("an exact version is always a valid requirement"), - ), - None => DependencyKey::any_version(spec.package), - }) -} - fn packages_from_foreign_deps( deps: impl IntoIterator, -) -> impl Iterator { +) -> impl Iterator { deps.into_iter().filter_map(|dep| { - let name = PackageRef::new(dep.namespace.parse().ok()?, dep.name.parse().ok()?); - let version = match dep.version { - Some(v) => format!("={v}"), - None => "*".to_string(), - }; - Some(( - name, - version - .parse() - .expect("Unable to parse into version request, this is programmer error"), - )) + let package = PackageRef::new(dep.namespace.parse().ok()?, dep.name.parse().ok()?); + Some(PackageSpec { + package, + version: dep.version, + }) }) } @@ -585,26 +559,36 @@ fn name_from_package_name(package_name: &PackageName) -> String { mod tests { use super::*; - /// A versioned override key only matches its dependency if it builds the same requirement the - /// WIT's `@version` does — these two need to always agree. + /// An override key and the WIT import it targets must produce the same key, versioned or not. #[test] - fn override_key_requirement_matches_foreign_dep() { - for version in ["0.1.0", "1.2.3", "0.2.0-draft", "0.2.0-alpha.1"] { + fn override_key_matches_foreign_dep() { + for version in [ + None, + Some("0.1.0"), + Some("1.2.3"), + Some("0.2.0-draft"), + Some("0.2.0-alpha.1"), + ] { let from_wit = packages_from_foreign_deps([PackageName { namespace: "foo".to_string(), name: "bar".to_string(), - version: Some(version.parse().unwrap()), + version: version.map(|v| v.parse().unwrap()), }]) .next() .expect("foreign dep should yield a package"); - let from_key = parse_override_key(&format!("foo:bar@{version}")).unwrap(); + let key = match version { + Some(version) => format!("foo:bar@{version}"), + None => "foo:bar".to_string(), + }; + let from_key: PackageSpec = key.parse().unwrap(); - assert_eq!(from_key.package, from_wit.0, "package for {version}"); + assert_eq!(from_key, from_wit, "key for {version:?}"); + let req = crate::resolver::key_requirement(&from_key); assert_eq!( - from_key.version, - Some(from_wit.1), - "requirement for {version}" + req.to_string(), + version.map(|v| format!("={v}")).unwrap_or("*".to_string()), + "requirement for {version:?}" ); } } @@ -612,7 +596,10 @@ mod tests { #[test] fn malformed_override_keys_are_rejected() { for key in ["foo:bar@", "foo:bar@not-a-version", "not a package ref"] { - assert!(parse_override_key(key).is_err(), "`{key}` should not parse"); + assert!( + key.parse::().is_err(), + "`{key}` should not parse" + ); } } } From e0eb6e1a912ab8d3e4fa8af8a7d38b82d1c867f6 Mon Sep 17 00:00:00 2001 From: Aditya Salunkhe Date: Fri, 28 Aug 2026 01:42:34 +0530 Subject: [PATCH 3/3] fix: minor doc comment --- crates/wasm-pkg-core/src/manifest.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/wasm-pkg-core/src/manifest.rs b/crates/wasm-pkg-core/src/manifest.rs index d4651b3..cd2ac0b 100644 --- a/crates/wasm-pkg-core/src/manifest.rs +++ b/crates/wasm-pkg-core/src/manifest.rs @@ -87,9 +87,7 @@ impl Manifest { /// Checks that override keys parse and that no package is covered by both a bare and a /// versioned key. - /// - /// Runs when a `wkg.toml` is loaded (see [`validate`](Self::validate)), and again when - /// resolving, since a `Manifest` built directly in Rust code skips the load step. + pub(crate) fn validate_override_keys(&self) -> Result<()> { let Some(overrides) = self.overrides.as_ref() else { return Ok(());