diff --git a/.gitignore b/.gitignore index ceecc9ef..27e97649 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,4 @@ target .DS_Store .vscode .envrc +.worktrees/ diff --git a/Cargo.lock b/Cargo.lock index 283a1fa8..f071b89e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -113,17 +113,6 @@ dependencies = [ "tempfile", ] -[[package]] -name = "async-recursion" -version = "1.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3b43422f69d8ff38f95f1b2bb76517c91589a924d1559a0e935d7c8ce0274c11" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.103", -] - [[package]] name = "async-trait" version = "0.1.89" @@ -149,6 +138,7 @@ checksum = "021e862c184ae977658b36c4500f7feac3221ca5da43e3f25bd04ab6c79a29b5" dependencies = [ "axum-core", "bytes", + "form_urlencoded", "futures-util", "http", "http-body", @@ -163,6 +153,8 @@ dependencies = [ "pin-project-lite", "rustversion", "serde", + "serde_path_to_error", + "serde_urlencoded", "sync_wrapper", "tokio", "tower", @@ -243,7 +235,6 @@ dependencies = [ "anyhow", "assert_cmd", "assert_fs", - "async-recursion", "async-trait", "axum", "base64", @@ -1740,6 +1731,17 @@ dependencies = [ "serde", ] +[[package]] +name = "serde_path_to_error" +version = "0.1.20" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "10a9ff822e371bb5403e391ecd83e182e0e77ba7f6fe0160b795797109d1b457" +dependencies = [ + "itoa", + "serde", + "serde_core", +] + [[package]] name = "serde_spanned" version = "0.6.9" diff --git a/Cargo.toml b/Cargo.toml index 5036a5b7..5f8639d0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -43,7 +43,6 @@ validation = [ git = [] [dependencies] -async-recursion = "1.0.5" async-trait = "0.1.89" anyhow = { version = "1.0", optional = true } base64 = "0.22.1" @@ -89,6 +88,7 @@ assert_fs = "1.0" axum = { version = "0.8", default-features = false, features = [ "tokio", "http1", + "query", ] } hex = "0.4.3" pretty_assertions = "1.4" diff --git a/docs/src/commands/buffrs-add.md b/docs/src/commands/buffrs-add.md index c4a117cf..ff6911ba 100644 --- a/docs/src/commands/buffrs-add.md +++ b/docs/src/commands/buffrs-add.md @@ -26,13 +26,12 @@ it will default to the latest version of this artifact in the registry. The repository name should adhere to lower-kebab case (e.g. `my-buffrs-repo`). The package name has its own set of constraints as detailed in [Package Name -Specification](../reference/pkgid-spec.md). When specified, the version must -adhere to the [Semantic Version convention](https://semver.org/) (e.g. `1.2.3`) --- see [SemVer compatibility](../reference/semver.md) for more information. - -Currently there is no support for resolving version operators but the specific -version has to be provided. This means `^1.0.0`, `<2.3.0`, `~2.0.0`, etc. can't -be installed, but `=1.2.3` has to be provided. +Specification](../reference/pkgid-spec.md). When specified, the version must be a valid +[SemVer requirement](../reference/semver.md). Both exact pins (`=1.2.3`) and +range operators (`^1.0.0`, `~2.1.0`, `>=1.5.0`, etc.) are supported. During +`buffrs install`, the resolver queries the registry and selects the highest +available version that satisfies the requirement, then records the concrete +version in the lockfile for reproducibility. #### Lockfile interaction diff --git a/docs/src/guide/consuming-packages.md b/docs/src/guide/consuming-packages.md index 300099e5..75634494 100644 --- a/docs/src/guide/consuming-packages.md +++ b/docs/src/guide/consuming-packages.md @@ -35,7 +35,7 @@ type = "lib" version = "1.0.0" [dependencies] -google = { version = "=1.0.0", registry = "", repository = " } +google = { version = "^1.0.0", registry = "", repository = "" } ``` Running `buffrs install` yields you with the following filesystem: @@ -65,7 +65,7 @@ major difference is the lack of the `[package]` section in your manifest. ``` [dependencies] -logging = { version = "=1.0.0", registry = "", repository = " } +logging = { version = "^1.0.0", registry = "", repository = "" } ``` Running a `buffrs install` yields you the very same as above, except for the diff --git a/docs/src/reference/resolver.md b/docs/src/reference/resolver.md index 8cc7fce9..d8a5eed1 100644 --- a/docs/src/reference/resolver.md +++ b/docs/src/reference/resolver.md @@ -1 +1,81 @@ # Dependency Resolution + +When you run `buffrs install`, the resolver builds a complete dependency graph +for your project — including all transitive dependencies — and determines the +concrete version to install for each package. + +## Resolution algorithm + +For each dependency (direct or transitive), the resolver follows this priority +order: + +1. **Lockfile hit** — if `Proto.lock` records a version of the package that + satisfies *every* requirement gathered for it so far, that version is used + immediately without contacting the registry. This makes repeated installs + fast and reproducible. A pin that does not satisfy them all is simply not a + usable answer, and resolution falls through to the registry. + +2. **Registry resolution** — if no locked version qualifies, the resolver + queries the registry for all available versions of the package, then selects + the **highest** version that satisfies **all** of the requirements at once. + +3. **Download and cache** — the resolved version is downloaded, stored in the + local cache, and its digest is recorded in the lockfile for future installs. + +Transitive dependencies are discovered by reading the `Proto.toml` bundled +inside each downloaded package archive, then resolved using the same steps +above. Because a package's full set of requirements is only known once every +path to it has been walked, choosing a lower version can retract dependencies +that a previously-chosen higher version had pulled in; those become unreachable +and are dropped from the graph. + +## Version conflict detection + +If the same package is required by more than one path in the dependency tree, +the resolver merges every requirement and picks the highest version satisfying +all of them together. Requirements are never evaluated pairwise or in +encounter order, so a later, tighter requirement can lower an earlier pick +rather than conflicting with it. + +The install fails only when the intersection is empty — no published version +satisfies every requirement: + +``` +no version of leaf-lib satisfies all requirements: [^1.0.0, ^2.0.0]; +available versions: [2.0.0, 1.0.0] +``` + +To fix a conflict, update the requiring packages so their version requirements +overlap, or introduce a package that bridges the incompatible requirements. + +Note that this conflict detection operates **within a single package's +dependency graph**. In a workspace, different members may independently resolve +different versions of the same package — the workspace lockfile records them +separately using a `(name, version)` composite key. + +## Workspace resolution + +In a workspace, each member package's dependency graph is resolved +independently. The workspace lockfile (`Proto.lock` at the workspace root) +accumulates all resolved packages across all members. Because the workspace +lockfile allows multiple versions of the same package, two members that require +incompatible versions of a shared library can co-exist. + +If a subsequent install finds a workspace lockfile, it reuses those locked +versions (subject to satisfying each member's requirements) to avoid redundant +registry queries. + +## Topological ordering + +After the full graph is built, packages are sorted topologically so that each +dependency is installed before its dependants. This guarantees that vendored +proto sources are available in the correct order during compilation. + +## Determinism and the lockfile + +The resolver always picks the **highest** version satisfying every requirement +when multiple candidates exist. This is deterministic given the same set of available +registry versions. Once a version is recorded in `Proto.lock`, it is used +as-is on all subsequent installs, regardless of newer versions that may have +been published since. Run `buffrs install` after deleting or modifying +`Proto.lock` to re-resolve against the current registry state. diff --git a/docs/src/reference/semver.md b/docs/src/reference/semver.md index b8bff592..0a27ddf0 100644 --- a/docs/src/reference/semver.md +++ b/docs/src/reference/semver.md @@ -1 +1,88 @@ # SemVer Compatibility + +buffrs uses [Semantic Versioning](https://semver.org/) for all packages. +Version requirements in `Proto.toml` follow the same syntax as +[Cargo](https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html). + +## Version requirement syntax + +A version requirement is placed in the `version` field of a dependency: + +```toml +[dependencies.my-lib] +version = "^1.2.0" +registry = "https://my-registry.example.com" +repository = "my-repo" +``` + +The following operators are supported: + +### Caret (`^`) — default for ranges + +Allows minor and patch updates within the same major version. +This is the recommended operator for most dependencies. + +| Requirement | Resolves versions | +|-------------|-------------------| +| `^1.2.3` | `>=1.2.3, <2.0.0` | +| `^1.2` | `>=1.2.0, <2.0.0` | +| `^1` | `>=1.0.0, <2.0.0` | +| `^0.2.3` | `>=0.2.3, <0.3.0` | +| `^0.0.3` | `>=0.0.3, <0.0.4` | + +Note that `0.x` versions are treated as unstable: `^0.2` only allows `0.2.x`, +not `0.3.x`, since breaking changes are expected in pre-1.0 packages. + +### Tilde (`~`) — patch-level updates only + +Allows patch updates within the same minor version. + +| Requirement | Resolves versions | +|-------------|-------------------| +| `~1.2.3` | `>=1.2.3, <1.3.0` | +| `~1.2` | `>=1.2.0, <1.3.0` | +| `~1` | `>=1.0.0, <2.0.0` | + +### Exact (`=`) — pin to a specific version + +Resolves to exactly the stated version, with no flexibility. + +```toml +version = "=1.2.3" +``` + +Use exact pins when you need bit-for-bit reproducibility in the manifest +itself, or when you are distributing a library whose consumers should be +in full control of the version. + +### Comparison operators + +For more control, the standard comparison operators are available: + +| Requirement | Meaning | +|-----------------|----------------------------------| +| `>=1.2.0` | Any version at or above 1.2.0 | +| `>1.2.0` | Any version strictly above 1.2.0 | +| `<2.0.0` | Any version strictly below 2.0.0 | +| `<=2.0.0` | Any version at or below 2.0.0 | +| `>=1.0.0, <2.0.0` | Intersection (multiple constraints) | + +## How the resolver picks a version + +When a requirement matches more than one available version, buffrs always +selects the **highest** satisfying version. The resolved concrete version is +written to `Proto.lock` to ensure reproducible installs — re-running +`buffrs install` will use the locked version rather than querying the registry +again. + +See [Dependency Resolution](./resolver.md) for a full description of the +resolution algorithm. + +## Choosing between pinning and ranges + +| Situation | Recommended style | +|-----------|-------------------| +| Public library — let consumers decide | `^1.0.0` | +| Internal service — stable dependency set | `^1.0.0` or `~1.2.0` | +| Security patch must be applied exactly | `=1.2.5` | +| Compatibility ceiling known | `>=1.0.0, <3.0.0` | diff --git a/docs/src/reference/specifying-dependencies.md b/docs/src/reference/specifying-dependencies.md index 7f6bbb72..af0ce599 100644 --- a/docs/src/reference/specifying-dependencies.md +++ b/docs/src/reference/specifying-dependencies.md @@ -1,34 +1,61 @@ # Specifying Dependencies -Dependencies are declared in the `[dependencies]` section of the `Proto.toml` -manifest. Each entry maps a dependency package name to a dependency -specification object. +Dependencies are declared in the `[dependencies]` section of `Proto.toml`. +Each entry names the package and provides a version requirement, registry URL, +and repository name. -## Remote Dependencies - -Remote dependencies are downloaded from an Artifactory registry during -[`buffrs install`](../commands/buffrs-install.md). +## Inline table syntax ```toml [dependencies] -my-package = { registry = "https://your.registry/artifactory", repository = "my-repo", version = "1.2.3" } +my-package = { registry = "https://your.registry/artifactory", repository = "my-repo", version = "^1.0.0" } ``` -The `version` field must be an exact semantic version (e.g. `"1.2.3"`). -Version ranges or operators (`^`, `~`, `<`, `>`) are not currently supported. +The three required fields for a remote dependency are: + +| Field | Description | +|-------|-------------| +| `version` | A SemVer requirement — see [SemVer Compatibility](./semver.md) | +| `registry` | Base URL of the Artifactory registry | +| `repository` | Repository name within that registry | + +## Adding dependencies via the CLI -Use [`buffrs add`](../commands/buffrs-add.md) to add a remote dependency from -the command line: +The `buffrs add` command writes the manifest entry for you: +```bash +# Caret range (recommended): resolves to the highest 1.x.y +buffrs add --registry https://my-registry.example.com my-repo/my-lib@^1.0.0 + +# Exact pin: resolves to exactly 1.2.3 +buffrs add --registry https://my-registry.example.com my-repo/my-lib@=1.2.3 + +# Latest: omitting the version resolves to the latest available +buffrs add --registry https://my-registry.example.com my-repo/my-lib ``` -buffrs add --registry https://your.registry/artifactory my-repo/my-package@1.2.3 + +After adding a dependency, run `buffrs install` to resolve and download it. + +## Version requirements + +buffrs supports the full range of SemVer requirement operators: + +```toml +version = "^1.0.0" # >=1.0.0, <2.0.0 (recommended for most deps) +version = "~1.2.0" # >=1.2.0, <1.3.0 (patch updates only) +version = ">=1.5.0" # any version at or above 1.5.0 +version = "=1.2.3" # exactly 1.2.3 +version = ">=1.0, <2.0" # explicit intersection ``` -## Local Dependencies +The resolver queries the registry and selects the **highest** available version +satisfying the requirement. See [SemVer Compatibility](./semver.md) for the +full operator reference. + +## Local dependencies -Local dependencies are resolved from the local filesystem relative to the -manifest. They are useful for multi-package repositories where packages depend -on each other without going through a remote registry. +You can depend on a package in a local directory (useful in monorepos or during +development): ```toml [dependencies] @@ -38,15 +65,20 @@ my-lib = { path = "../my-lib" } The `path` field is a relative path from the manifest file to the dependency's root directory (the directory containing the dependency's `Proto.toml`). +Local dependencies do not have a version requirement — the package at that +path is used as-is. They cannot be mixed with a remote entry for the same +package name. + See [Local Dependencies](../guide/local-dependencies.md) for more information. -## Lockfile +## The lockfile -After adding or modifying dependencies in the manifest, run -[`buffrs install`](../commands/buffrs-install.md) to resolve and lock them. -The lockfile (`Proto.lock`) records the exact resolved versions and checksums -and should be committed to version control. +Once resolved, the concrete version is recorded in `Proto.lock`. Subsequent +installs use the locked version without re-querying the registry, ensuring +reproducible builds across machines and CI environments. -See [Manifest vs Lockfile](../guide/manifest-vs-lockfile.md) for more -information. +Commit `Proto.lock` to version control for applications and services. For +libraries intended to be consumed by others, committing the lockfile is +optional — consumers will resolve their own versions. +See [Manifest vs Lockfile](../guide/manifest-vs-lockfile.md) for more detail. diff --git a/src/lock.rs b/src/lock.rs index a6b23195..09e26863 100644 --- a/src/lock.rs +++ b/src/lock.rs @@ -18,7 +18,7 @@ use std::{ }; use miette::{Context, IntoDiagnostic, ensure}; -use semver::Version; +use semver::{Version, VersionReq}; use serde::{Deserialize, Serialize}; use thiserror::Error; use tokio::fs; @@ -110,13 +110,20 @@ impl<'de> Deserialize<'de> for LockedDependency { let s = String::deserialize(deserializer)?; let parts: Vec<&str> = s.split_whitespace().collect(); - if parts.len() != 2 { + if parts.len() == 1 { let name = PackageName::new(parts[0]) .map_err(|e| serde::de::Error::custom(format!("invalid package name: {}", e)))?; return Ok(Self::Named { name }); } + if parts.len() != 2 { + return Err(serde::de::Error::custom(format!( + "invalid locked dependency format: expected 'name' or 'name version', got '{}'", + s + ))); + } + let name = PackageName::new(parts[0]) .map_err(|e| serde::de::Error::custom(format!("invalid package name: {}", e)))?; @@ -251,6 +258,22 @@ impl PackageLockfile { pub fn packages(&self) -> impl Iterator { self.packages.values() } + + /// Finds the locked package for `name` if its version satisfies `req` + pub fn find_satisfying(&self, name: &PackageName, req: &VersionReq) -> Option<&LockedPackage> { + self.packages.get(name).filter(|p| req.matches(&p.version)) + } + + /// Finds the locked package for `name` if its version satisfies every requirement + pub fn find_satisfying_all( + &self, + name: &PackageName, + reqs: &[VersionReq], + ) -> Option<&LockedPackage> { + self.packages + .get(name) + .filter(|p| reqs.iter().all(|r| r.matches(&p.version))) + } } #[async_trait::async_trait] @@ -371,6 +394,31 @@ impl WorkspaceLockfile { self.packages.get(&(name.clone(), version.clone())) } + /// Finds the highest locked version of `name` that satisfies `req` + pub fn find_satisfying(&self, name: &PackageName, req: &VersionReq) -> Option<&LockedPackage> { + self.packages + .values() + .filter(|p| p.name == *name && req.matches(&p.version)) + .max_by_key(|p| &p.version) + } + + /// Finds the highest locked version of `name` that satisfies every requirement + /// + /// The workspace lockfile is a pool shared by independently-resolved members, + /// so entries that satisfy nothing here may legitimately belong to another + /// member. They are skipped, not treated as errors. + pub fn find_satisfying_all( + &self, + name: &PackageName, + reqs: &[VersionReq], + ) -> Option<&LockedPackage> { + self.packages + .values() + .filter(|p| p.name == *name) + .filter(|p| reqs.iter().all(|r| r.matches(&p.version))) + .max_by_key(|p| &p.version) + } + /// Returns all packages in the lockfile pub fn packages(&self) -> impl Iterator { self.packages.values() @@ -477,7 +525,7 @@ pub enum Lockfile { } impl Lockfile { - /// Locates a package by name and version + /// Locates a package by name and exact version pub fn get(&self, name: &PackageName, version: &Version) -> Option { match self { Self::Package(lock) => lock @@ -569,6 +617,44 @@ impl Lockfile { )), } } + + /// Finds the highest locked version of `name` that satisfies `req`, returning its concrete + /// version and file requirement + pub fn find_satisfying( + &self, + name: &PackageName, + req: &VersionReq, + ) -> Option<(Version, FileRequirement)> { + match self { + Self::Package(lock) => lock + .find_satisfying(name, req) + .map(|p| (p.version.clone(), FileRequirement::from(p))), + Self::Workspace(lock) => lock + .find_satisfying(name, req) + .map(|p| (p.version.clone(), FileRequirement::from(p))), + } + } + + /// Finds the highest locked version of `name` satisfying every requirement, + /// returning its concrete version and file requirement + /// + /// A pin that satisfies none of the requirements is simply absent from the + /// answer rather than an error: in a workspace pool it may belong to another, + /// independently-resolved member. + pub fn find_satisfying_all( + &self, + name: &PackageName, + reqs: &[VersionReq], + ) -> Option<(Version, FileRequirement)> { + match self { + Self::Package(lock) => lock + .find_satisfying_all(name, reqs) + .map(|p| (p.version.clone(), FileRequirement::from(p))), + Self::Workspace(lock) => lock + .find_satisfying_all(name, reqs) + .map(|p| (p.version.clone(), FileRequirement::from(p))), + } + } } #[async_trait::async_trait] @@ -755,8 +841,8 @@ mod tests { use crate::{io::File, package::PackageName, registry::RegistryUri}; use super::{ - Digest, DigestAlgorithm, FileRequirement, LockedDependency, LockedPackage, PackageLockfile, - WorkspaceLockfile, + Digest, DigestAlgorithm, FileRequirement, LockedDependency, LockedPackage, Lockfile, + PackageLockfile, WorkspaceLockfile, }; fn simple_lockfile() -> PackageLockfile { @@ -1172,4 +1258,85 @@ mod tests { let result = resolved.get(&PackageName::unchecked("ws-pkg"), &Version::new(2, 0, 0)); assert!(result.is_none()); } + + fn locked(name: &str, version: Version) -> LockedPackage { + LockedPackage { + name: PackageName::new(name).unwrap(), + digest: Digest::from_parts( + DigestAlgorithm::SHA256, + "c109c6b120c525e6ea7b2db98335d39a3272f572ac86ba7b2d65c765c353c122", + ) + .unwrap(), + registry: RegistryUri::from_str("http://my-registry.com").unwrap(), + repository: "my-repo".to_owned(), + version, + dependencies: Default::default(), + dependants: 1, + } + } + + fn reqs(specs: &[&str]) -> Vec { + specs.iter().map(|s| s.parse().unwrap()).collect() + } + + #[test] + fn find_satisfying_all_accepts_version_satisfying_every_req() { + let lock = Lockfile::Package(PackageLockfile::from_iter(vec![locked( + "lib-a", + Version::new(1, 2, 0), + )])); + + let got = lock + .find_satisfying_all( + &PackageName::new("lib-a").unwrap(), + &reqs(&["^1.0.0", "<=1.2.0"]), + ) + .expect("1.2.0 satisfies both"); + assert_eq!(got.0, Version::new(1, 2, 0)); + } + + #[test] + fn find_satisfying_all_skips_pin_that_satisfies_only_some_reqs() { + let lock = Lockfile::Package(PackageLockfile::from_iter(vec![locked( + "lib-a", + Version::new(1, 5, 0), + )])); + + assert!( + lock.find_satisfying_all(&PackageName::new("lib-a").unwrap(), &reqs(&["<=1.2.0"])) + .is_none(), + "a non-satisfying pin is absent, not an error" + ); + } + + #[test] + fn find_satisfying_all_picks_highest_satisfying_entry_from_the_workspace_pool() { + // A workspace pool legitimately holds several versions of one package, + // contributed by independently-resolved members. + let lock = Lockfile::Workspace(WorkspaceLockfile::from_iter(vec![ + locked("lib-a", Version::new(1, 2, 0)), + locked("lib-a", Version::new(1, 5, 0)), + ])); + + let got = lock + .find_satisfying_all( + &PackageName::new("lib-a").unwrap(), + &reqs(&["^1.0.0", "<=1.2.0"]), + ) + .expect("1.2.0 is in the pool"); + assert_eq!( + got.0, + Version::new(1, 2, 0), + "must not be misled by the higher pooled entry" + ); + } + + #[test] + fn find_satisfying_all_returns_none_when_package_absent() { + let lock = Lockfile::Package(PackageLockfile::from_iter(Vec::new())); + assert!( + lock.find_satisfying_all(&PackageName::new("missing").unwrap(), &reqs(&["^1.0.0"])) + .is_none() + ); + } } diff --git a/src/manifest/raw.rs b/src/manifest/raw.rs index cc8e25b5..1031c289 100644 --- a/src/manifest/raw.rs +++ b/src/manifest/raw.rs @@ -79,12 +79,15 @@ impl RawManifest { pub(crate) fn dependencies_as_vec(&self) -> Option> { self.dependencies().map(|deps| { - deps.iter() + let mut out: Vec<_> = deps + .iter() .map(|(package, manifest)| Dependency { package: package.to_owned(), manifest: manifest.to_owned(), }) - .collect() + .collect(); + out.sort_by(|a, b| a.package.cmp(&b.package)); + out }) } @@ -458,6 +461,50 @@ mod tests { ); } + #[test] + fn dependencies_as_vec_is_sorted_by_name() { + let mut deps = HashMap::new(); + deps.insert( + "zeta".parse::().unwrap(), + DependencyManifest::Remote(RemoteDependencyManifest { + registry: "http://example.test/artifactory".parse().unwrap(), + repository: "test".to_string(), + version: VersionReq::STAR, + }), + ); + deps.insert( + "alpha".parse::().unwrap(), + DependencyManifest::Remote(RemoteDependencyManifest { + registry: "http://example.test/artifactory".parse().unwrap(), + repository: "test".to_string(), + version: VersionReq::STAR, + }), + ); + deps.insert( + "mu".parse::().unwrap(), + DependencyManifest::Remote(RemoteDependencyManifest { + registry: "http://example.test/artifactory".parse().unwrap(), + repository: "test".to_string(), + version: VersionReq::STAR, + }), + ); + + let raw = RawManifest::Canary { + package: None, + dependencies: Some(deps), + workspace: None, + }; + + let names: Vec = raw + .dependencies_as_vec() + .unwrap() + .into_iter() + .map(|d| d.package.to_string()) + .collect(); + + assert_eq!(names, vec!["alpha", "mu", "zeta"]); + } + #[test] fn raw_manifest_from_str_valid() { let toml = r#" diff --git a/src/operations/install.rs b/src/operations/install.rs index bec8e7a2..e7514bca 100644 --- a/src/operations/install.rs +++ b/src/operations/install.rs @@ -250,18 +250,20 @@ impl Install for WorkspaceManifest { let mut locked = vec![]; for package in packages { - let manifest = Manifest::require_package_manifest(&package).await?; + let member_path = ctx.cwd.join(&package); - if PackageLockfile::exists_at(&package).await? { + let manifest = Manifest::require_package_manifest(&member_path).await?; + + if PackageLockfile::exists_at(&member_path).await? { tracing::warn!( "[warn] package lockfile found at {}. Consider removing it - workspace installs use workspace-level lockfile", - PackageLockfile::resolve(&package)?.display() + PackageLockfile::resolve(&member_path)?.display() ); } - tracing::info!("running install for package: {}", package.display()); + tracing::info!("running install for package: {}", member_path.display()); - let member_ctx = ctx.child(ctx.cwd.join(&package)).await?; + let member_ctx = ctx.child(member_path).await?; let new = manifest.install(&member_ctx).await?; locked.extend_from_slice(&new); @@ -329,7 +331,7 @@ mod utils { if ctx.network_mode == NetworkMode::Offline { bail!(DependencyError::Offline { name: package_name.clone(), - version: version.clone(), + requirements: vec![version.clone()], }); } @@ -337,6 +339,16 @@ mod utils { let artifactory = Artifactory::new(registry.clone(), &ctx.credentials) .wrap_err_with(|| format!("failed to initialize registry {}", registry))?; + let resolved_version = artifactory + .resolve_version(repository.to_string(), package_name.clone(), version) + .await + .wrap_err_with(|| { + format!( + "could not resolve {}@{} from registry {}", + package_name, version, registry + ) + })?; + let dependency = Dependency { package: package_name.clone(), manifest: DependencyManifest::Remote(RemoteDependencyManifest { @@ -346,7 +358,7 @@ mod utils { }), }; - let downloaded_package = artifactory.download(dependency).await?; + let downloaded_package = artifactory.download(dependency, &resolved_version).await?; // 2. Cache the downloaded package for future installs let cache_key = CacheEntry::from(&downloaded_package); @@ -675,4 +687,74 @@ mod tests { assert_eq!(v2.version, Version::new(2, 0, 0)); assert_eq!(v2.dependants, 1); } + + #[tokio::test] + async fn test_workspace_install_uses_ctx_cwd_not_process_cwd() { + use std::collections::HashMap; + use std::fs; + use tempfile::TempDir; + + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + + fs::write( + root.join("Proto.toml"), + "edition = \"0.13\"\n\n[workspace]\nmembers = [\"pkg-a\", \"pkg-b\"]\n", + ) + .unwrap(); + + for name in ["pkg-a", "pkg-b"] { + fs::create_dir(root.join(name)).unwrap(); + fs::write( + root.join(name).join("Proto.toml"), + format!( + "edition = \"0.13\"\n\n[package]\nname = \"{name}\"\ntype = \"lib\"\nversion = \"0.1.0\"\n" + ), + ) + .unwrap(); + } + + // The process CWD (crate root during cargo test) must differ from the + // temp dir — this is what makes the test meaningful. With the old buggy + // code, relative member paths would be resolved against the process CWD + // instead of ctx.cwd, causing a file-not-found failure. + assert_ne!( + std::env::current_dir().unwrap(), + root, + "process CWD must differ from ctx.cwd for this test to be meaningful" + ); + + let manifest = Manifest::load_from(root) + .await + .expect("should parse workspace manifest"); + let workspace_manifest = match manifest { + Manifest::Workspace(ws) => ws, + _ => panic!("expected workspace manifest"), + }; + + let ctx = InstallationContext { + cwd: root.to_path_buf(), + credentials: Credentials { + registry_tokens: HashMap::new(), + }, + cache: Cache::open().await.unwrap(), + store: PackageStore::open(root).await.unwrap(), + lock: Lockfile::Workspace(WorkspaceLockfile::default()), + preserve_mtime: false, + network_mode: NetworkMode::Offline, + }; + + let result = workspace_manifest.install(&ctx).await; + assert!( + result.is_ok(), + "workspace install failed — member paths were likely resolved against \ + process CWD instead of ctx.cwd: {:?}", + result.unwrap_err() + ); + assert_eq!( + result.unwrap(), + vec![], + "no-dep workspace should produce no locked packages" + ); + } } diff --git a/src/registry/artifactory.rs b/src/registry/artifactory.rs index 634c78f8..16ea43ec 100644 --- a/src/registry/artifactory.rs +++ b/src/registry/artifactory.rs @@ -21,7 +21,7 @@ use crate::{ }; use miette::{Context, IntoDiagnostic, ensure, miette}; use reqwest::{Body, Method, Response}; -use semver::Version; +use semver::{Version, VersionReq}; use serde::Deserialize; use url::Url; @@ -82,13 +82,13 @@ impl Artifactory { .map(|_| ()) } - /// Retrieves the latest version of a package by querying artifactory. Returns an error if no artifact could be found - pub async fn get_latest_version( + /// Lists all available versions of a package from artifactory, sorted descending + pub async fn list_versions( &self, repository: String, name: PackageName, - ) -> miette::Result { - tracing::debug!("Artifactory::get_latest_version() called"); + ) -> miette::Result> { + tracing::debug!("Artifactory::list_versions() called"); tracing::debug!(" package name: {}", name); tracing::debug!(" repository: {}", repository); tracing::debug!(" registry: {}", self.registry); @@ -139,14 +139,13 @@ impl Artifactory { ))?; tracing::debug!( - "found {} artifacts matching the name: {:?}", - parsed_response.results.len(), - parsed_response + "found {} artifacts matching the name", + parsed_response.results.len() ); - // Then from all package names retrieved from artifactory, extract the highest version number + // From the package names retrieved from artifactory, list the versions tracing::debug!("extracting version numbers from artifact URIs"); - let highest_version = parsed_response + let mut versions: Vec = parsed_response .results .iter() .filter_map(|artifact_search_result| { @@ -169,7 +168,7 @@ impl Artifactory { Version::parse(version_str).ok() }); - // we double check that the artifact name matches exactly + // Double-check that the artifact name matches exactly let expected_artifact_name = artifact_version.clone().map(|av| format!("{name}-{av}")); if full_artifact_name.is_some_and(|actual| { @@ -184,18 +183,83 @@ impl Artifactory { None } }) - .max(); + .collect(); - tracing::debug!("highest version for artifact: {:?}", highest_version); + versions.sort_unstable_by(|a, b| b.cmp(a)); // descending + tracing::debug!("found {} valid versions", versions.len()); + Ok(versions) + } - highest_version.ok_or_else(|| { - tracing::error!("no version could be found for package {} in repository {}", name, repository); - miette!("no version could be found on artifactory for this artifact name. Does it exist in this registry and repository?") - }) + /// Resolves the highest available version of a package satisfying a requirement + pub async fn resolve_version( + &self, + repository: String, + name: PackageName, + req: &VersionReq, + ) -> miette::Result { + let versions = self.list_versions(repository, name.clone()).await?; + versions + .into_iter() + .find(|v| req.matches(v)) + .ok_or_else(|| { + miette!( + "no version of {} satisfies requirement {} in this registry", + name, + req + ) + }) + } + + /// Like [`Artifactory::resolve_version`], but requires *all* of `reqs` to match. + /// + /// Returns the chosen version alongside the full descending version list, so the + /// caller can attach `available_versions` to a `NoCompatibleVersion` diagnostic. + /// A registry failure is an `Err`; "nothing satisfies the constraints" is + /// `Ok((None, versions))`. + pub async fn resolve_version_for_all( + &self, + repository: String, + name: PackageName, + reqs: &[VersionReq], + ) -> miette::Result<(Option, Vec)> { + let versions = self.list_versions(repository, name).await?; + + // `list_versions` is sorted descending, so the first match is the highest. + let chosen = versions + .iter() + .find(|v| reqs.iter().all(|r| r.matches(v))) + .cloned(); + + Ok((chosen, versions)) } - /// Downloads a package from artifactory - pub async fn download(&self, dependency: Dependency) -> miette::Result { + /// Retrieves the latest version of a package by querying artifactory + pub async fn get_latest_version( + &self, + repository: String, + name: PackageName, + ) -> miette::Result { + tracing::debug!("Artifactory::get_latest_version() called"); + self.list_versions(repository.clone(), name.clone()) + .await? + .into_iter() + .next() // list_versions is already sorted descending + .ok_or_else(|| { + tracing::error!( + "no version could be found for package {} in repository {}", + name, + repository + ); + miette!("no version could be found on artifactory for this artifact name. Does it exist in this registry and repository?") + }) + } + + /// Downloads a specific version of a package from artifactory + pub async fn download( + &self, + dependency: Dependency, + version: &Version, + ) -> miette::Result { tracing::debug!("Artifactory::download() called"); tracing::debug!(" package name: {}", dependency.package); @@ -212,12 +276,9 @@ impl Artifactory { tracing::debug!(" registry: {}", manifest.registry); tracing::debug!(" repository: {}", manifest.repository); - tracing::debug!(" version requirement: {}", manifest.version); + tracing::debug!(" resolved version: {}", version); let artifact_url = { - let version = super::dependency_version_string(&dependency)?; - tracing::debug!(" resolved version: {}", version); - let path = manifest.registry.path().to_owned(); let mut url = manifest.registry.clone(); diff --git a/src/registry/cache.rs b/src/registry/cache.rs index fbdc18e4..25d16de3 100644 --- a/src/registry/cache.rs +++ b/src/registry/cache.rs @@ -37,7 +37,11 @@ impl LocalRegistry { } /// "Downloads" a package from the local filesystem - pub async fn download(&self, dependency: Dependency) -> miette::Result { + pub async fn download( + &self, + dependency: Dependency, + version: &semver::Version, + ) -> miette::Result { let DependencyManifest::Remote(ref manifest) = dependency.manifest else { return Err(miette!( "unable to serialize version of local dependency ({})", @@ -45,8 +49,6 @@ impl LocalRegistry { )); }; - let version = super::dependency_version_string(&dependency)?; - let path = self.base_dir.join(PathBuf::from(format!( "{}/{}/{}-{}.tgz", manifest.repository, dependency.package, dependency.package, version @@ -155,13 +157,17 @@ mod tests { // Download package from local registry and assert the tgz bytes and the metadata match what we // had published. + let version = semver::Version::new(0, 1, 0); let fetched = registry - .download(Dependency::new( - registry_uri, - "test-repo".into(), - "test-api".parse().unwrap(), - "=0.1.0".parse().unwrap(), - )) + .download( + Dependency::new( + registry_uri, + "test-repo".into(), + "test-api".parse().unwrap(), + "=0.1.0".parse().unwrap(), + ), + &version, + ) .await .unwrap(); diff --git a/src/registry/mod.rs b/src/registry/mod.rs index 0db92bb2..6d275ae0 100644 --- a/src/registry/mod.rs +++ b/src/registry/mod.rs @@ -24,13 +24,9 @@ mod cache; pub use artifactory::Artifactory; use miette::{Context, IntoDiagnostic, ensure, miette}; -use semver::VersionReq; use serde::{Deserialize, Serialize}; -use thiserror::Error; use url::Url; -use crate::manifest::{Dependency, DependencyManifest}; - /// A representation of a registry URI #[derive(Debug, Clone, Hash, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)] pub struct RegistryUri(Url); @@ -93,116 +89,3 @@ fn sanity_check_url(url: &Url) -> miette::Result<()> { Err(miette!("the URI must contain a host component: {url}")) } } - -#[derive(Error, Debug)] -#[error("{0} is not a supported version requirement")] -struct UnsupportedVersionRequirement(VersionReq); - -#[derive(Error, Debug)] -#[error( - "{0} is not supported yet. Pin the exact version you want to use with '='. For example: '=1.0.4' instead of '^1.0.0'" -)] -struct VersionNotPinned(VersionReq); - -fn dependency_version_string(dependency: &Dependency) -> miette::Result { - let DependencyManifest::Remote(ref manifest) = dependency.manifest else { - return Err(miette!( - "unable to serialize version of local dependency ({})", - dependency.package - )); - }; - - let version = manifest - .version - .comparators - .first() - .ok_or_else(|| UnsupportedVersionRequirement(manifest.version.clone())) - .into_diagnostic()?; - - ensure!( - version.op == semver::Op::Exact, - VersionNotPinned(manifest.version.clone()) - ); - - let minor_version = version - .minor - .ok_or_else(|| miette!("version missing minor number"))?; - - let patch_version = version - .patch - .ok_or_else(|| miette!("version missing patch number"))?; - - Ok(format!( - "{}.{}.{}{}", - version.major, - minor_version, - patch_version, - if version.pre.is_empty() { - "".to_owned() - } else { - format!("-{}", version.pre) - } - )) -} - -#[cfg(test)] -mod tests { - use std::str::FromStr; - - use semver::VersionReq; - - use crate::{ - manifest::Dependency, - package::PackageName, - registry::{VersionNotPinned, dependency_version_string}, - }; - - use super::RegistryUri; - - fn get_dependency(version: &str) -> Dependency { - let registry = RegistryUri::from_str("https://my-registry.com").unwrap(); - let repository = String::from("my-repo"); - let package = PackageName::from_str("package").unwrap(); - let version = VersionReq::from_str(version).unwrap(); - Dependency::new(registry, repository, package, version) - } - - #[test] - fn valid_version() { - let dependency = get_dependency("=0.0.1"); - assert!(dependency_version_string(&dependency).is_ok_and(|version| version == "0.0.1")); - - let dependency = get_dependency("=0.0.1-23"); - assert!(dependency_version_string(&dependency).is_ok_and(|version| version == "0.0.1-23")); - - let dependency = get_dependency("=0.0.1-ab"); - assert!(dependency_version_string(&dependency).is_ok_and(|version| version == "0.0.1-ab")); - } - - #[test] - fn unsupported_version_operator() { - let dependency = get_dependency("^0.0.1"); - assert!( - dependency_version_string(&dependency).is_err_and(|err| err.is::()) - ); - - let dependency = get_dependency("~0.0.1"); - assert!( - dependency_version_string(&dependency).is_err_and(|err| err.is::()) - ); - - let dependency = get_dependency("<=0.0.1"); - assert!( - dependency_version_string(&dependency).is_err_and(|err| err.is::()) - ); - } - - #[test] - fn incomplete_version() { - let dependency = get_dependency("=1.0"); - assert!(dependency_version_string(&dependency).is_err()); - - let dependency = get_dependency("=1"); - assert!(dependency_version_string(&dependency).is_err()); - } -} diff --git a/src/resolver.rs b/src/resolver.rs index f470bd79..ccc79c69 100644 --- a/src/resolver.rs +++ b/src/resolver.rs @@ -1,22 +1,24 @@ // (c) Copyright 2025 Helsing GmbH. All rights reserved. +// thiserror v2 generates unused_assignments in Display impls when free-function +// positional format args reference fields via .field syntax. +#![allow(unused_assignments)] use std::{ - collections::{HashMap, HashSet, VecDeque}, + collections::{HashMap, VecDeque}, path::{Path, PathBuf}, }; -use async_recursion::async_recursion; use miette::{Context as _, Diagnostic, bail}; -use semver::VersionReq; +use semver::{Version, VersionReq}; use thiserror::Error; use crate::{ cache::Cache, credentials::Credentials, - lock::Lockfile, + lock::{FileRequirement, Lockfile}, manifest::{ - Dependency, DependencyManifest, LocalDependencyManifest, MANIFEST_FILE, Manifest, - PackagesManifest, + Dependency, DependencyManifest, MANIFEST_FILE, Manifest, PackagesManifest, + RemoteDependencyManifest, }, operations::install::NetworkMode, package::{PackageName, PackageType}, @@ -51,8 +53,13 @@ pub struct DependencyNode { pub source: DependencySource, /// Packages that this package depends on pub dependencies: Vec, - /// Version requirement + /// Exact version requirement (`=`) for remote deps, `*` for locals + /// + /// Consumed by the installer to fetch the package, so it pins the version the + /// resolver chose rather than the requirement that produced it. pub version: VersionReq, + /// The concrete version that was resolved and downloaded (None for local deps) + pub resolved_version: Option, } /// Maps a package name to metadata describing the package @@ -90,33 +97,45 @@ impl DependencyGraph { lockfile: Option, network_mode: NetworkMode, ) -> miette::Result { - let mut builder = - GraphBuilder::new(base_path.to_path_buf(), credentials, lockfile, network_mode); - - // Get the parent package type from the manifest - let parent_package_type = manifest.package.as_ref().map(|p| p.kind); + let root_package_type = manifest.package.as_ref().map(|p| p.kind); + let root_name = manifest + .package + .as_ref() + .map(|p| p.name.to_string()) + .unwrap_or_else(|| "root".to_string()); + + let mut builder = GraphBuilder::new( + base_path.to_path_buf(), + root_package_type, + credentials, + lockfile, + network_mode, + ); - // Add root dependencies + // Phase 1: register every root edge before resolving anything, so a package + // required by several roots is picked against the merged requirement set. for dependency in manifest.dependencies.iter().flatten() { builder - .add_dependency(dependency, parent_package_type) - .await - .wrap_err_with(|| { - format!( - "while resolving dependencies of {}", - manifest - .package - .as_ref() - .map(|p| p.name.to_string()) - .unwrap_or_else(|| "root".to_string()) - ) - })?; + .add_edge(None, dependency) + .wrap_err_with(|| format!("while resolving dependencies of {}", root_name))?; } - Ok(Self { + // Phase 2: drain the worklist. + builder + .drive() + .await + .wrap_err_with(|| format!("while resolving dependencies of {}", root_name))?; + + let graph = Self { nodes: builder.nodes, network_mode, - }) + }; + + // The worklist is BFS, so a cycle produces a cyclic graph rather than + // unbounded recursion. Detect it on the finished graph. + graph.topological_sort()?; + + Ok(graph) } /// Returns dependencies in topological order (dependencies before dependents) @@ -214,12 +233,27 @@ impl DependencyGraph { } } +/// A single edge contributing a `VersionReq` to a package's accumulated set. +#[derive(Debug, Clone)] +struct RequirementEdge { + /// The contributing parent package, or `None` for a root manifest edge. + from: Option, + /// The version requirement carried by this edge. `VersionReq::STAR` for locals. + req: VersionReq, +} + /// Internal builder for constructing the dependency graph struct GraphBuilder<'a> { nodes: HashMap, + /// Per-package accumulated requirement edges. + requirements: HashMap>, + /// First-seen source for each package, used to detect local/remote conflicts. + sources: HashMap, + /// FIFO of package names whose resolution must be (re-)attempted. + worklist: VecDeque, base_path: PathBuf, - /// Track which packages we're currently visiting to detect cycles during construction - visiting: HashSet, + /// Package type of the root manifest, used as the parent type for root edges. + root_package_type: Option, credentials: &'a Credentials, lockfile: Option, registry_clients: HashMap, @@ -229,14 +263,18 @@ struct GraphBuilder<'a> { impl<'a> GraphBuilder<'a> { fn new( base_path: PathBuf, + root_package_type: Option, credentials: &'a Credentials, lockfile: Option, network_mode: NetworkMode, ) -> Self { Self { nodes: HashMap::new(), + requirements: HashMap::new(), + sources: HashMap::new(), + worklist: VecDeque::new(), base_path, - visiting: HashSet::new(), + root_package_type, credentials, lockfile, registry_clients: HashMap::new(), @@ -244,249 +282,373 @@ impl<'a> GraphBuilder<'a> { } } - #[async_recursion] - async fn add_dependency( + /// Records a requirement edge from `parent` (None = root manifest) to + /// `dependency`, scheduling the dependency if it needs (re-)resolution. + fn add_edge( &mut self, + parent: Option, dependency: &Dependency, - parent_type: Option, ) -> miette::Result<()> { - let package_name = &dependency.package; - - // Check for cycle during traversal - if self.visiting.contains(package_name) { - bail!(DependencyError::CircularDependency(format!( - "detected while processing {}", - package_name - ))); + let name = dependency.package.clone(); + + let new_source = match &dependency.manifest { + DependencyManifest::Local(local) => DependencySource::Local { + path: self.base_path.join(&local.path), + }, + DependencyManifest::Remote(remote) => DependencySource::Remote { + registry: remote.registry.clone(), + repository: remote.repository.clone(), + }, + }; + + match self.sources.get(&name) { + Some(existing) => Self::ensure_source_kinds_match(&name, existing, &new_source)?, + None => { + self.sources.insert(name.clone(), new_source); + } } - // If already processed, just validate compatibility - if let Some(existing) = self.nodes.get(package_name) { - self.validate_compatibility(dependency, existing) - .wrap_err_with(|| format!("conflicting dependency on {}", package_name))?; - return Ok(()); + let req = match &dependency.manifest { + DependencyManifest::Local(_) => VersionReq::STAR, + DependencyManifest::Remote(remote) => remote.version.clone(), + }; + + self.requirements + .entry(name.clone()) + .or_default() + .push(RequirementEdge { from: parent, req }); + + if self.needs_visit(&name) && !self.worklist.contains(&name) { + self.worklist.push_back(name); } - // Mark as visiting - self.visiting.insert(package_name.clone()); + Ok(()) + } - match &dependency.manifest { - DependencyManifest::Local(local) => { - self.add_local_dependency(dependency, local, parent_type) - .await?; - } - DependencyManifest::Remote(remote) => { - self.add_remote_dependency(dependency, remote, parent_type) - .await?; - } + /// True when `name` has no node yet, or its resolved version no longer + /// satisfies the (possibly just-extended) requirement set. + /// + /// The conditional scheduling this drives is what makes the traversal + /// terminate on cyclic graphs: a back-edge into an already-inserted node + /// schedules no further work. + fn needs_visit(&self, name: &PackageName) -> bool { + match self.nodes.get(name) { + None => true, + Some(node) => match &node.resolved_version { + // Locals carry no version; once resolved they never need revisiting. + None => false, + Some(version) => !self + .requirements + .get(name) + .is_some_and(|edges| edges.iter().all(|e| e.req.matches(version))), + }, } + } + + fn ensure_source_kinds_match( + name: &PackageName, + existing: &DependencySource, + new_kind: &DependencySource, + ) -> miette::Result<()> { + let mismatch = matches!( + (existing, new_kind), + ( + DependencySource::Local { .. }, + DependencySource::Remote { .. } + ) | ( + DependencySource::Remote { .. }, + DependencySource::Local { .. } + ) + ); - // Unmark as visiting - self.visiting.remove(package_name); + if mismatch { + bail!(DependencyError::LocalRemoteConflict { + package: name.clone() + }); + } Ok(()) } - async fn add_local_dependency( + /// Drains the worklist, resolving each package against its accumulated + /// requirement set and walking its transitives. + /// + /// Terminates because a package's merged requirement set only grows while its + /// parent set is stable (which can only hold or lower the chosen version), + /// retraction never raises a pick, and `fetch_and_walk` short-circuits when the + /// pick is unchanged — so each package is walked at most once per distinct + /// version, over a finite, non-increasing sequence. + async fn drive(&mut self) -> miette::Result<()> { + while let Some(name) = self.worklist.pop_front() { + // The package may have been retracted out of existence, or satisfied by + // an earlier pop, since it was scheduled. + if !self.requirements.contains_key(&name) || !self.needs_visit(&name) { + continue; + } + + self.resolve_one(&name) + .await + .wrap_err_with(|| format!("while resolving {}", name))?; + } + + Ok(()) + } + + async fn resolve_one(&mut self, name: &PackageName) -> miette::Result<()> { + let source = self + .sources + .get(name) + .cloned() + .expect("source recorded by add_edge"); + + match source { + DependencySource::Local { path } => self.resolve_local(name, &path).await, + DependencySource::Remote { + registry, + repository, + } => self.resolve_remote(name, ®istry, &repository).await, + } + } + + async fn resolve_local( &mut self, - dependency: &Dependency, - local_manifest: &LocalDependencyManifest, - parent_type: Option, + name: &PackageName, + resolved_path: &Path, ) -> miette::Result<()> { - let resolved_path = self.base_path.join(&local_manifest.path); let manifest_path = resolved_path.join(MANIFEST_FILE); - let manifest = Manifest::require_package_manifest(&manifest_path).await?; let package_type = manifest.package.as_ref().map(|p| p.kind); - Self::ensure_lib_not_depends_on_api(dependency, parent_type, package_type)?; + self.ensure_lib_not_depends_on_api(name, package_type)?; - let sub_dependencies: Vec = manifest.get_dependency_package_names(); - - // Add node + // Insert before registering sub-edges so a back-edge into this package + // resolves against an existing node instead of looping. self.nodes.insert( - dependency.package.clone(), + name.clone(), DependencyNode { - name: dependency.package.clone(), + name: name.clone(), package_type, source: DependencySource::Local { - path: resolved_path.clone(), + path: resolved_path.to_path_buf(), }, - dependencies: sub_dependencies.clone(), + dependencies: manifest.get_dependency_package_names(), version: VersionReq::STAR, + resolved_version: None, }, ); - // Recursively process dependencies with the new base path - for sub_dep in manifest.dependencies.unwrap_or_default() { - // We need to update the base path for sub-dependencies - let old_base = self.base_path.clone(); - self.base_path = resolved_path.clone(); - self.add_dependency(&sub_dep, package_type) - .await - .wrap_err_with(|| { - format!("while resolving dependencies of {}", dependency.package) - })?; - self.base_path = old_base; + // Sub-dependency paths are relative to this package's directory. + let old_base = std::mem::replace(&mut self.base_path, resolved_path.to_path_buf()); + let mut result = Ok(()); + for sub_dep in manifest.dependencies.into_iter().flatten() { + result = self.add_edge(Some(name.clone()), &sub_dep); + if result.is_err() { + break; + } } + self.base_path = old_base; - Ok(()) + result } - /// Ensures that a lib package doesn't depend on an api package - fn ensure_lib_not_depends_on_api( - dependency: &Dependency, - parent_type: Option, - package_type: Option, + async fn resolve_remote( + &mut self, + name: &PackageName, + registry: &RegistryUri, + repository: &str, ) -> miette::Result<()> { - // Validate package type constraint - if let Some(PackageType::Lib) = parent_type - && let Some(PackageType::Api) = package_type + let reqs: Vec = self.requirements[name] + .iter() + .map(|e| e.req.clone()) + .collect(); + + // 1. Lockfile: the highest pinned version satisfying every requirement. + // A pin that satisfies nothing is skipped, not an error — in a workspace + // pool it may belong to another, independently-resolved member. + if let Some(lockfile) = &self.lockfile + && let Some((locked_version, file_req)) = lockfile.find_satisfying_all(name, &reqs) + && file_req.url().as_str().starts_with(registry.as_str()) { - bail!(DependencyError::InvalidPackageTypeDependency { - parent: PackageName::unchecked("parent"), - dependency: dependency.package.clone(), - }); + tracing::debug!("lockfile pins {}@{}", name, locked_version); + + return self + .fetch_and_walk(name, registry, repository, locked_version, Some(file_req)) + .await; } - Ok(()) + // 2. Registry. + let artifactory = self.registry_client(registry)?; + let (chosen, available) = artifactory + .resolve_version_for_all(repository.to_string(), name.clone(), &reqs) + .await + .wrap_err_with(|| format!("could not resolve {} from registry {}", name, registry))?; + + let chosen = chosen.ok_or_else(|| DependencyError::NoCompatibleVersion { + package: name.clone(), + requirements: reqs, + available_versions: available, + })?; + + tracing::debug!("resolved {} -> {}", name, chosen); + + self.fetch_and_walk(name, registry, repository, chosen, None) + .await } - async fn add_remote_dependency( + async fn fetch_and_walk( &mut self, - dependency: &Dependency, - remote_manifest: &crate::manifest::RemoteDependencyManifest, - parent_type: Option, + name: &PackageName, + registry: &RegistryUri, + repository: &str, + chosen: Version, + cached: Option, ) -> miette::Result<()> { - let package_name = &dependency.package; - let registry = &remote_manifest.registry; - let repository = &remote_manifest.repository; - let version = &remote_manifest.version; - - let package = { - let reformatted = version - .to_string() - .chars() - .skip_while(|c| *c == '=') - .collect::(); - - let version = reformatted - .parse() - .map_err(|e| miette::miette!("{e}")) - .wrap_err_with(|| { - format!("expected only exact version requirements: {package_name}@{version}") - })?; - - let mut cached_package = None; - - // Try to resolve from lockfile + cache first - if let Some(lockfile) = &self.lockfile - && let Some(file_req) = lockfile.get(package_name, &version) - { - // Verify registry matches (lockfile vs manifest) - if file_req.url().as_str().starts_with(registry.as_str()) { - let cache = Cache::open().await?; - if let Ok(Some(pkg)) = cache.get(file_req).await { - tracing::debug!("resolved {}@{} from local cache", package_name, version); - cached_package = Some(pkg); - } - } + if let Some(existing) = self.nodes.get(name) { + if existing.resolved_version.as_ref() == Some(&chosen) { + return Ok(()); } - match (cached_package, self.network_mode) { - (Some(pkg), _) => pkg, - (None, NetworkMode::Online) => { - tracing::debug!("downloading {}@{} from registry", package_name, version); - - // Reuse or create artifactory client - let artifactory = if let Some(client) = self.registry_clients.get(registry) { - client.clone() - } else { - let client = Artifactory::new(registry.clone(), self.credentials) - .wrap_err_with(|| { - format!("failed to initialize registry {}", registry) - })?; - self.registry_clients - .insert(registry.clone(), client.clone()); - client - }; - - artifactory.download(dependency.clone()).await? - } - (None, NetworkMode::Offline) => { - bail!(DependencyError::Offline { - name: package_name.clone(), - version: remote_manifest.version.clone(), - }); - } + // The pick changed: everything the old version pulled in is no longer + // justified by this package. + self.retract_contributions_from(name); + } + + let cache = Cache::open().await?; + let cached_pkg = match cached { + Some(file_req) => cache.get(file_req).await.ok().flatten(), + None => None, + }; + + let package = match (cached_pkg, self.network_mode) { + (Some(pkg), _) => { + tracing::debug!("resolved {}@{} from local cache", name, chosen); + pkg + } + (None, NetworkMode::Online) => { + tracing::debug!("downloading {}@{} from registry", name, chosen); + + let artifactory = self.registry_client(registry)?; + let dependency = Dependency { + package: name.clone(), + manifest: DependencyManifest::Remote(RemoteDependencyManifest { + registry: registry.clone(), + repository: repository.to_string(), + version: exact_req(&chosen), + }), + }; + + artifactory.download(dependency, &chosen).await? + } + (None, NetworkMode::Offline) => { + bail!(DependencyError::Offline { + name: name.clone(), + requirements: self.requirements[name] + .iter() + .map(|e| e.req.clone()) + .collect(), + }); } }; - // Read the package manifest to discover dependencies and package type let manifest = package.manifest; let package_type = manifest.package.as_ref().map(|p| p.kind); - Self::ensure_lib_not_depends_on_api(dependency, parent_type, package_type)?; + self.ensure_lib_not_depends_on_api(name, package_type)?; - let sub_dependencies: Vec = manifest.get_dependency_package_names(); - - // Add node with discovered metadata + // `version` is consumed by the installer to fetch the package, so it must + // pin the version the resolver actually chose — not one of the requirements + // that produced it, which would let the installer re-resolve upward. self.nodes.insert( - package_name.clone(), + name.clone(), DependencyNode { - name: package_name.clone(), + name: name.clone(), package_type, source: DependencySource::Remote { registry: registry.clone(), - repository: repository.clone(), + repository: repository.to_string(), }, - dependencies: sub_dependencies.clone(), - version: version.clone(), + dependencies: manifest.get_dependency_package_names(), + version: exact_req(&chosen), + resolved_version: Some(chosen), }, ); - // Recursively process transitive dependencies - for sub_dep in manifest.dependencies.unwrap_or_default() { - self.add_dependency(&sub_dep, package_type) - .await - .wrap_err_with(|| format!("while resolving dependencies of {}", package_name))?; + for sub_dep in manifest.dependencies.into_iter().flatten() { + self.add_edge(Some(name.clone()), &sub_dep)?; } Ok(()) } - fn validate_compatibility( - &self, - dependency: &Dependency, - existing: &DependencyNode, - ) -> miette::Result<()> { - // Check for local/remote conflicts - self.validate_manifest_conflicts(dependency, existing)?; + /// Retracts every requirement edge contributed by `parent`, dropping packages + /// that become unreachable and rescheduling those whose pick no longer fits. + fn retract_contributions_from(&mut self, parent: &PackageName) { + let mut frontier: Vec = vec![parent.clone()]; + + while let Some(retractor) = frontier.pop() { + let affected: Vec = self + .requirements + .iter() + .filter(|(_, edges)| edges.iter().any(|e| e.from.as_ref() == Some(&retractor))) + .map(|(name, _)| name.clone()) + .collect(); - // Check for version conflicts on remote dependencies - self.validate_version_compatibility(dependency, existing)?; + for name in affected { + // Never retract the originating package's own requirement set. + if name == *parent { + continue; + } - Ok(()) + let Some(edges) = self.requirements.get_mut(&name) else { + continue; + }; + edges.retain(|e| e.from.as_ref() != Some(&retractor)); + + if edges.is_empty() { + self.requirements.remove(&name); + self.sources.remove(&name); + self.nodes.remove(&name); + frontier.push(name); + } else if self.needs_visit(&name) && !self.worklist.contains(&name) { + // Still reachable, but the shrunken set no longer fits the pick. + // We deliberately do not re-optimize upward. + self.worklist.push_back(name); + } + } + } } - /// Validates that version requirements are compatible when the same package is requested multiple times - fn validate_version_compatibility( + /// Ensures that no lib package depends on an api package + /// + /// Every `edge.from` is guaranteed to be in `nodes` already, because a parent + /// inserts its own node before registering sub-edges. Root edges (`from: None`) + /// use the root manifest's package type. + fn ensure_lib_not_depends_on_api( &self, - dependency: &Dependency, - existing: &DependencyNode, + name: &PackageName, + package_type: Option, ) -> miette::Result<()> { - // Only check version compatibility for remote dependencies - if let (DependencyManifest::Remote(new_remote), DependencySource::Remote { .. }) = - (&dependency.manifest, &existing.source) - { - // For now, buffrs only supports pinned versions (see TODO #205) - // We check if the version requirements are equal since they should be exact pins - // In the future with dynamic version resolution, this would need to check for compatibility - if new_remote.version != existing.version { - bail!(DependencyError::VersionConflict { - package: dependency.package.clone(), - required_version: new_remote.version.clone(), - existing_version: existing.version.clone(), + if package_type != Some(PackageType::Api) { + return Ok(()); + } + + let Some(edges) = self.requirements.get(name) else { + return Ok(()); + }; + + for edge in edges { + let parent_type = match &edge.from { + None => self.root_package_type, + Some(parent) => self.nodes.get(parent).and_then(|n| n.package_type), + }; + + if parent_type == Some(PackageType::Lib) { + bail!(DependencyError::InvalidPackageTypeDependency { + parent: edge + .from + .clone() + .unwrap_or_else(|| PackageName::unchecked("parent")), + dependency: name.clone(), }); } } @@ -494,27 +656,41 @@ impl<'a> GraphBuilder<'a> { Ok(()) } - /// Checks for conflicting dependencies between local / remote deps in the dependency tree - fn validate_manifest_conflicts( - &self, - dependency: &Dependency, - existing: &DependencyNode, - ) -> miette::Result<()> { - match (&dependency.manifest, &existing.source) { - (DependencyManifest::Local(_), DependencySource::Remote { .. }) => { - bail!(DependencyError::LocalRemoteConflict { - package: dependency.package.clone(), - }); - } - (DependencyManifest::Remote(_), DependencySource::Local { .. }) => { - bail!(DependencyError::LocalRemoteConflict { - package: dependency.package.clone(), - }); - } - _ => {} + fn registry_client(&mut self, registry: &RegistryUri) -> miette::Result { + if let Some(client) = self.registry_clients.get(registry) { + return Ok(client.clone()); } - Ok(()) + let client = Artifactory::new(registry.clone(), self.credentials) + .wrap_err_with(|| format!("failed to initialize registry {}", registry))?; + self.registry_clients + .insert(registry.clone(), client.clone()); + + Ok(client) + } +} + +/// Builds an exact (`=x.y.z`) requirement from a resolved version. +fn exact_req(version: &Version) -> VersionReq { + VersionReq::parse(&format!("={}", version)).expect("resolved version is a valid exact req") +} + +fn fmt_reqs(reqs: &[semver::VersionReq]) -> String { + reqs.iter() + .map(|r| r.to_string()) + .collect::>() + .join(", ") +} + +fn fmt_versions(versions: &[semver::Version]) -> String { + if versions.is_empty() { + "(none published)".to_string() + } else { + versions + .iter() + .map(|v| v.to_string()) + .collect::>() + .join(", ") } } @@ -541,38 +717,238 @@ pub enum DependencyError { #[error("circular dependency detected: {0}")] CircularDependency(String), - /// Version conflict between multiple dependants - #[error( - "version conflict for {package}: requires {required_version} but already resolved to {existing_version}" - )] - VersionConflict { - /// The package with conflicting versions - package: PackageName, - /// The version requirement that conflicts - required_version: VersionReq, - /// The version already resolved in the graph - existing_version: VersionReq, - }, - - /// Failed to download a dependency from the registry - #[error("failed to download dependency {name}@{version} from the registry")] - DownloadError { - /// Package name - name: PackageName, - /// Version requirement - version: VersionReq, - }, - /// A network request was needed but --offline mode is active - #[error("cannot fetch {name}@{version} in offline mode")] + #[error("cannot fetch {name} in offline mode (required: [{}])", fmt_reqs(.requirements))] #[diagnostic(help( - "run `buffrs install` without --offline to cache {name}@{version},\n + "run `buffrs install` without --offline to cache the package,\n\ or set BUFFRS_CACHE to a pre-populated cache directory" ))] Offline { /// Package name name: PackageName, - /// Version requirement - version: VersionReq, + /// Version requirements that could not be satisfied offline + requirements: Vec, }, + + /// No single version satisfies the union of all requirements. + #[error( + "no version of {package} satisfies all requirements: [{}]; available versions: [{}]", + fmt_reqs(.requirements), + fmt_versions(.available_versions) + )] + NoCompatibleVersion { + /// The package with conflicting requirements + package: PackageName, + /// The set of requirements that could not be jointly satisfied + requirements: Vec, + /// All versions that were published for this package + available_versions: Vec, + }, +} + +#[cfg(test)] +mod error_tests { + use super::*; + use semver::Version; + + #[test] + fn no_compatible_version_diagnostic_lists_requirements() { + let err = DependencyError::NoCompatibleVersion { + package: "leaf-lib".parse().unwrap(), + requirements: vec!["^1.1.0".parse().unwrap(), "<=1.2.0".parse().unwrap()], + available_versions: vec![Version::new(1, 0, 0), Version::new(1, 5, 0)], + }; + let msg = err.to_string(); + assert!(msg.contains("leaf-lib"), "msg: {msg}"); + assert!(msg.contains("1.0.0"), "msg: {msg}"); + assert!(msg.contains("1.5.0"), "msg: {msg}"); + assert!(msg.contains("^1.1.0"), "msg: {msg}"); + assert!(msg.contains("<=1.2.0"), "msg: {msg}"); + } +} + +#[cfg(test)] +mod retraction_tests { + use super::*; + + fn pkg(name: &str) -> PackageName { + PackageName::unchecked(name) + } + + fn remote_source() -> DependencySource { + DependencySource::Remote { + registry: "http://example.test/artifactory".parse().unwrap(), + repository: "test-repo".to_string(), + } + } + + fn node(name: &str, version: &str, dependencies: Vec) -> DependencyNode { + let resolved: Version = version.parse().unwrap(); + + DependencyNode { + name: pkg(name), + package_type: Some(PackageType::Lib), + source: remote_source(), + dependencies, + version: exact_req(&resolved), + resolved_version: Some(resolved), + } + } + + /// Builds a graph state directly, bypassing any network access. + fn builder<'a>( + credentials: &'a Credentials, + edges: &[(&str, Option<&str>, &str)], + nodes: Vec, + ) -> GraphBuilder<'a> { + let mut builder = GraphBuilder::new( + PathBuf::from("/nonexistent"), + Some(PackageType::Lib), + credentials, + None, + NetworkMode::Offline, + ); + + for (name, from, req) in edges { + builder + .requirements + .entry(pkg(name)) + .or_default() + .push(RequirementEdge { + from: from.map(pkg), + req: req.parse().unwrap(), + }); + builder + .sources + .entry(pkg(name)) + .or_insert_with(remote_source); + } + + for node in nodes { + builder.nodes.insert(node.name.clone(), node); + } + + builder + } + + /// Dropping a package must transitively drop everything that was only + /// reachable through it, not just its immediate dependencies. + /// + /// root -> a (retracted) + /// a -> b (b's only contributor) + /// b -> c (c's only contributor) + #[test] + fn retraction_cascades_through_multiple_levels() { + let credentials = Credentials::default(); + let mut builder = builder( + &credentials, + &[ + ("a", None, "^1.0.0"), + ("b", Some("a"), "^1.0.0"), + ("c", Some("b"), "^1.0.0"), + ], + vec![ + node("a", "1.5.0", vec![pkg("b")]), + node("b", "1.0.0", vec![pkg("c")]), + node("c", "1.0.0", vec![]), + ], + ); + + builder.retract_contributions_from(&pkg("a")); + + assert!( + !builder.nodes.contains_key(&pkg("b")), + "b was only reachable via a and must be dropped" + ); + assert!( + !builder.nodes.contains_key(&pkg("c")), + "c was only reachable via b and must cascade out too" + ); + for name in ["b", "c"] { + assert!(!builder.requirements.contains_key(&pkg(name))); + assert!(!builder.sources.contains_key(&pkg(name))); + } + + assert!( + builder.nodes.contains_key(&pkg("a")), + "the retracting package itself is left in place for its caller to replace" + ); + assert!( + builder.requirements.contains_key(&pkg("a")), + "a's own requirement set must survive retraction" + ); + } + + /// A package that keeps a contributor survives, but must be rescheduled when + /// the shrunken requirement set no longer admits its current version. + #[test] + fn retraction_reschedules_survivor_whose_pick_no_longer_fits() { + let credentials = Credentials::default(); + let mut builder = builder( + &credentials, + &[ + ("a", None, "^1.0.0"), + ("other", None, "^1.0.0"), + // `shared` is required by both `a` (loosely) and `other` (tightly). + ("shared", Some("a"), "^1.0.0"), + ("shared", Some("other"), "<=1.2.0"), + ], + vec![ + node("a", "1.5.0", vec![pkg("shared")]), + node("other", "1.0.0", vec![pkg("shared")]), + // Currently at 1.5.0, which only `a`'s requirement admits. + node("shared", "1.5.0", vec![]), + ], + ); + + builder.retract_contributions_from(&pkg("a")); + + assert!( + builder.nodes.contains_key(&pkg("shared")), + "shared is still reachable via other and must not be dropped" + ); + assert_eq!( + builder.requirements[&pkg("shared")].len(), + 1, + "only a's edge is retracted" + ); + assert!( + builder.worklist.contains(&pkg("shared")), + "1.5.0 no longer satisfies <=1.2.0, so shared must be re-resolved" + ); + } + + /// A survivor whose current version still satisfies the shrunken set is left + /// alone: retraction never re-optimizes upward. + #[test] + fn retraction_leaves_still_satisfied_survivor_alone() { + let credentials = Credentials::default(); + let mut builder = builder( + &credentials, + &[ + ("a", None, "^1.0.0"), + ("other", None, "^1.0.0"), + ("shared", Some("a"), "^1.0.0"), + ("shared", Some("other"), ">=1.0.0"), + ], + vec![ + node("a", "1.5.0", vec![pkg("shared")]), + node("other", "1.0.0", vec![pkg("shared")]), + node("shared", "1.5.0", vec![]), + ], + ); + + builder.retract_contributions_from(&pkg("a")); + + assert!(builder.nodes.contains_key(&pkg("shared"))); + assert!( + !builder.worklist.contains(&pkg("shared")), + "1.5.0 still satisfies >=1.0.0, so no re-resolution is scheduled" + ); + assert_eq!( + builder.nodes[&pkg("shared")].resolved_version, + Some(Version::new(1, 5, 0)), + "the existing pick is kept rather than re-optimized" + ); + } } diff --git a/tests/cmd/install/workspace/lockfile/mod.rs b/tests/cmd/install/workspace/lockfile/mod.rs index 52df9fa0..490d1a61 100644 --- a/tests/cmd/install/workspace/lockfile/mod.rs +++ b/tests/cmd/install/workspace/lockfile/mod.rs @@ -1 +1,2 @@ +mod range_pin_reuse; mod stale; diff --git a/tests/cmd/install/workspace/lockfile/range_pin_reuse/in/Proto.toml b/tests/cmd/install/workspace/lockfile/range_pin_reuse/in/Proto.toml new file mode 100644 index 00000000..0e8669f9 --- /dev/null +++ b/tests/cmd/install/workspace/lockfile/range_pin_reuse/in/Proto.toml @@ -0,0 +1,4 @@ +edition = "0.13" + +[workspace] +members = ["pkg1", "pkg2"] diff --git a/tests/cmd/install/workspace/lockfile/range_pin_reuse/in/pkg1/Proto.toml b/tests/cmd/install/workspace/lockfile/range_pin_reuse/in/pkg1/Proto.toml new file mode 100644 index 00000000..ced8ab36 --- /dev/null +++ b/tests/cmd/install/workspace/lockfile/range_pin_reuse/in/pkg1/Proto.toml @@ -0,0 +1,8 @@ +edition = "0.13" + +[package] +type = "lib" +name = "workspace-pkg1" +version = "1.0.0" + +[dependencies] diff --git a/tests/cmd/install/workspace/lockfile/range_pin_reuse/in/pkg1/proto/pkg1.proto b/tests/cmd/install/workspace/lockfile/range_pin_reuse/in/pkg1/proto/pkg1.proto new file mode 100644 index 00000000..f8a12aa7 --- /dev/null +++ b/tests/cmd/install/workspace/lockfile/range_pin_reuse/in/pkg1/proto/pkg1.proto @@ -0,0 +1,7 @@ +syntax = "proto3"; + +package pkg1; + +message Payload { + string data = 1; +} diff --git a/tests/cmd/install/workspace/lockfile/range_pin_reuse/in/pkg2/Proto.toml b/tests/cmd/install/workspace/lockfile/range_pin_reuse/in/pkg2/Proto.toml new file mode 100644 index 00000000..5504fb01 --- /dev/null +++ b/tests/cmd/install/workspace/lockfile/range_pin_reuse/in/pkg2/Proto.toml @@ -0,0 +1,8 @@ +edition = "0.13" + +[package] +type = "lib" +name = "workspace-pkg2" +version = "1.0.0" + +[dependencies] diff --git a/tests/cmd/install/workspace/lockfile/range_pin_reuse/in/pkg2/proto/pkg2.proto b/tests/cmd/install/workspace/lockfile/range_pin_reuse/in/pkg2/proto/pkg2.proto new file mode 100644 index 00000000..0b0e12a5 --- /dev/null +++ b/tests/cmd/install/workspace/lockfile/range_pin_reuse/in/pkg2/proto/pkg2.proto @@ -0,0 +1,7 @@ +syntax = "proto3"; + +package pkg2; + +message Response { + string value = 1; +} diff --git a/tests/cmd/install/workspace/lockfile/range_pin_reuse/mod.rs b/tests/cmd/install/workspace/lockfile/range_pin_reuse/mod.rs new file mode 100644 index 00000000..01226a4d --- /dev/null +++ b/tests/cmd/install/workspace/lockfile/range_pin_reuse/mod.rs @@ -0,0 +1,77 @@ +use crate::{VirtualFileSystem, with_test_registry}; + +/// A range requirement whose pin is still satisfiable must be reused from the +/// lockfile instead of being re-resolved against the registry. +/// +/// The sibling `stale` fixture only exercises exact pins (`=0.1.0`), which take +/// the equality path. Here pkg1 depends on `lib-a@^1.0.0`: the first install +/// pins 1.2.0, then 1.3.0 is published. A second install must still yield +/// 1.2.0 — proof that the pin was matched by +/// `Lockfile::find_satisfying_all` and the registry was never asked for a +/// version, since asking would have returned the newer 1.3.0. +#[test] +fn fixture() { + with_test_registry(|url| { + let vfs = VirtualFileSystem::copy(crate::parent_directory!().join("in")); + let buffrs_home = vfs.root().join("$HOME"); + let cwd = vfs.root(); + + for version in ["1.0.0", "1.2.0"] { + super::super::publish_leaf_lib(&cwd, &buffrs_home, url, "lib-a", version); + } + + crate::cli!() + .args(["add", "--registry", url, "test-repo/lib-a@^1.0.0"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(cwd.join("pkg1")) + .assert() + .success(); + + crate::cli!() + .arg("install") + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&cwd) + .assert() + .success(); + + let lib_a = |lock: &str| { + lock.split("[[packages]]") + .find(|s| s.contains("name = \"lib-a\"")) + .expect("lib-a in lockfile") + .to_string() + }; + + let lockfile_path = cwd.join("Proto.lock"); + let after_first = std::fs::read_to_string(&lockfile_path).unwrap(); + assert!( + lib_a(&after_first).contains("version = \"1.2.0\""), + "first install pins the highest version satisfying ^1.0.0: {after_first}" + ); + + // A newer compatible version appears after the pin was written. Only a + // fresh registry resolution would pick it up. + super::super::publish_leaf_lib(&cwd, &buffrs_home, url, "lib-a", "1.3.0"); + + crate::cli!() + .arg("install") + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&cwd) + .assert() + .success(); + + let after_second = std::fs::read_to_string(&lockfile_path).unwrap(); + assert!( + lib_a(&after_second).contains("version = \"1.2.0\""), + "second install must reuse the 1.2.0 pin, not upgrade to 1.3.0: {after_second}" + ); + assert!( + !after_second.contains("version = \"1.3.0\""), + "1.3.0 must not enter the lockfile while a satisfying pin exists: {after_second}" + ); + assert_eq!( + lib_a(&after_first), + lib_a(&after_second), + "the lib-a entry (version and digest) must survive the second install unchanged" + ); + }) +} diff --git a/tests/cmd/install/workspace/mod.rs b/tests/cmd/install/workspace/mod.rs index a208389f..1bf5be10 100644 --- a/tests/cmd/install/workspace/mod.rs +++ b/tests/cmd/install/workspace/mod.rs @@ -3,3 +3,100 @@ mod lockfile; mod lockfile_diamond_dependencies; mod lockfile_multiple_versions; mod lockfile_transitive; +mod range_caret_resolution; +mod range_cascade_resolution; +mod range_compatible_diamond; +mod range_downgrade_diamond; +mod range_incompatible_diamond; +mod range_lockfile_reresolution; +mod range_multi_level_tree; +mod range_tilde_resolution; + +/// Publishes a dependency-free lib at `version` and removes its scratch directory. +pub(super) fn publish_leaf_lib( + cwd: &std::path::Path, + buffrs_home: &std::path::Path, + url: &str, + name: &str, + version: &str, +) { + publish_lib(cwd, buffrs_home, url, name, version, None); +} + +/// Publishes a lib at `version` that depends on `dep_name@dep_req`. +pub(super) fn publish_lib_with_dep( + cwd: &std::path::Path, + buffrs_home: &std::path::Path, + url: &str, + name: &str, + version: &str, + dep_name: &str, + dep_req: &str, +) { + publish_lib( + cwd, + buffrs_home, + url, + name, + version, + Some((dep_name, dep_req)), + ); +} + +fn publish_lib( + cwd: &std::path::Path, + buffrs_home: &std::path::Path, + url: &str, + name: &str, + version: &str, + dependency: Option<(&str, &str)>, +) { + let dir = cwd.join(format!("{name}-v{version}")); + std::fs::create_dir(&dir).unwrap(); + + crate::cli!() + .args(["init", "--lib", name]) + .env("BUFFRS_HOME", buffrs_home) + .current_dir(&dir) + .assert() + .success(); + + let manifest_path = dir.join("Proto.toml"); + let manifest = std::fs::read_to_string(&manifest_path) + .unwrap() + .replace("version = \"0.1.0\"", &format!("version = \"{version}\"")); + std::fs::write(&manifest_path, manifest).unwrap(); + + let proto_package = name.replace('-', "_"); + std::fs::write( + dir.join(format!("proto/{proto_package}.proto")), + format!( + "syntax = \"proto3\";\n\npackage {proto_package};\n\nmessage M {{ string value = 1; }}\n" + ), + ) + .unwrap(); + + if let Some((dep_name, dep_req)) = dependency { + crate::cli!() + .args([ + "add", + "--registry", + url, + &format!("test-repo/{dep_name}@{dep_req}"), + ]) + .env("BUFFRS_HOME", buffrs_home) + .current_dir(&dir) + .assert() + .success(); + } + + crate::cli!() + .args(["publish", "--registry", url, "--repository", "test-repo"]) + .env("BUFFRS_HOME", buffrs_home) + .current_dir(&dir) + .assert() + .success(); + + // Scratch directories must not linger in the workspace root. + std::fs::remove_dir_all(&dir).unwrap(); +} diff --git a/tests/cmd/install/workspace/range_caret_resolution/in/Proto.toml b/tests/cmd/install/workspace/range_caret_resolution/in/Proto.toml new file mode 100644 index 00000000..7873c093 --- /dev/null +++ b/tests/cmd/install/workspace/range_caret_resolution/in/Proto.toml @@ -0,0 +1,4 @@ +edition = "0.12" + +[workspace] +members = ["pkg1"] diff --git a/tests/cmd/install/workspace/range_caret_resolution/in/pkg1/Proto.toml b/tests/cmd/install/workspace/range_caret_resolution/in/pkg1/Proto.toml new file mode 100644 index 00000000..055e8789 --- /dev/null +++ b/tests/cmd/install/workspace/range_caret_resolution/in/pkg1/Proto.toml @@ -0,0 +1,8 @@ +edition = "0.12" + +[package] +type = "lib" +name = "workspace-pkg1" +version = "1.0.0" + +[dependencies] diff --git a/tests/cmd/install/workspace/range_caret_resolution/in/pkg1/proto/pkg1.proto b/tests/cmd/install/workspace/range_caret_resolution/in/pkg1/proto/pkg1.proto new file mode 100644 index 00000000..f8deb6b3 --- /dev/null +++ b/tests/cmd/install/workspace/range_caret_resolution/in/pkg1/proto/pkg1.proto @@ -0,0 +1,7 @@ +syntax = "proto3"; + +package workspace.pkg1; + +message Pkg1Message { + string value = 1; +} diff --git a/tests/cmd/install/workspace/range_caret_resolution/mod.rs b/tests/cmd/install/workspace/range_caret_resolution/mod.rs new file mode 100644 index 00000000..54079b3a --- /dev/null +++ b/tests/cmd/install/workspace/range_caret_resolution/mod.rs @@ -0,0 +1,148 @@ +use crate::{VirtualFileSystem, with_test_registry}; + +/// Verifies that a caret version requirement (^1.0.0) resolves to the highest +/// compatible version available in the registry, not the minimum. +/// +/// Publishes leaf-lib at v1.0.0, v1.1.0, and v1.2.0, then installs with +/// `^1.0.0`. Expects the lockfile to pin v1.2.0. +#[test] +fn fixture() { + with_test_registry(|url| { + let vfs = VirtualFileSystem::copy(crate::parent_directory!().join("in")); + let buffrs_home = vfs.root().join("$HOME"); + let cwd = vfs.root(); + + // Publish leaf-lib at v1.0.0 + { + let lib_dir = cwd.join("leaf-lib-v1"); + std::fs::create_dir(&lib_dir).unwrap(); + + crate::cli!() + .args(["init", "--lib", "leaf-lib"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + + let manifest_path = lib_dir.join("Proto.toml"); + let manifest = std::fs::read_to_string(&manifest_path).unwrap(); + let updated = manifest.replace("version = \"0.1.0\"", "version = \"1.0.0\""); + std::fs::write(&manifest_path, updated).unwrap(); + + std::fs::write( + lib_dir.join("proto/leaf.proto"), + "syntax = \"proto3\";\n\npackage leaf;\n\nmessage LeafMessage {\n string value = 1;\n}\n", + ) + .unwrap(); + + crate::cli!() + .args(["publish", "--registry", url, "--repository", "test-repo"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + } + + // Publish leaf-lib at v1.1.0 + { + let lib_dir = cwd.join("leaf-lib-v1-1"); + std::fs::create_dir(&lib_dir).unwrap(); + + crate::cli!() + .args(["init", "--lib", "leaf-lib"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + + let manifest_path = lib_dir.join("Proto.toml"); + let manifest = std::fs::read_to_string(&manifest_path).unwrap(); + let updated = manifest.replace("version = \"0.1.0\"", "version = \"1.1.0\""); + std::fs::write(&manifest_path, updated).unwrap(); + + std::fs::write( + lib_dir.join("proto/leaf.proto"), + "syntax = \"proto3\";\n\npackage leaf;\n\nmessage LeafMessage {\n string value = 1;\n string extra = 2;\n}\n", + ) + .unwrap(); + + crate::cli!() + .args(["publish", "--registry", url, "--repository", "test-repo"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + } + + // Publish leaf-lib at v1.2.0 (should be the resolved version) + { + let lib_dir = cwd.join("leaf-lib-v1-2"); + std::fs::create_dir(&lib_dir).unwrap(); + + crate::cli!() + .args(["init", "--lib", "leaf-lib"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + + let manifest_path = lib_dir.join("Proto.toml"); + let manifest = std::fs::read_to_string(&manifest_path).unwrap(); + let updated = manifest.replace("version = \"0.1.0\"", "version = \"1.2.0\""); + std::fs::write(&manifest_path, updated).unwrap(); + + std::fs::write( + lib_dir.join("proto/leaf.proto"), + "syntax = \"proto3\";\n\npackage leaf;\n\nmessage LeafMessage {\n string value = 1;\n string extra = 2;\n int32 count = 3;\n}\n", + ) + .unwrap(); + + crate::cli!() + .args(["publish", "--registry", url, "--repository", "test-repo"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + } + + std::fs::remove_dir_all(cwd.join("leaf-lib-v1")).unwrap(); + std::fs::remove_dir_all(cwd.join("leaf-lib-v1-1")).unwrap(); + std::fs::remove_dir_all(cwd.join("leaf-lib-v1-2")).unwrap(); + + // Add leaf-lib with caret requirement to pkg1 + crate::cli!() + .args(["add", "--registry", url, "test-repo/leaf-lib@^1.0.0"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(cwd.join("pkg1")) + .assert() + .success(); + + // Install at workspace root + crate::cli!() + .arg("install") + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&cwd) + .assert() + .success(); + + let lockfile_path = cwd.join("Proto.lock"); + assert!(lockfile_path.exists(), "Proto.lock should exist"); + + let lockfile = std::fs::read_to_string(&lockfile_path).unwrap(); + + // ^1.0.0 must resolve to the highest 1.x.y: 1.2.0 + assert!( + lockfile.contains("version = \"1.2.0\""), + "^1.0.0 should resolve to 1.2.0 (highest compatible), got:\n{}", + lockfile + ); + assert!( + !lockfile.contains("version = \"1.0.0\""), + "^1.0.0 must not pin to the minimum 1.0.0" + ); + assert!( + !lockfile.contains("version = \"1.1.0\""), + "^1.0.0 must not pin to the intermediate 1.1.0" + ); + }) +} diff --git a/tests/cmd/install/workspace/range_cascade_resolution/in/Proto.toml b/tests/cmd/install/workspace/range_cascade_resolution/in/Proto.toml new file mode 100644 index 00000000..b6ce836e --- /dev/null +++ b/tests/cmd/install/workspace/range_cascade_resolution/in/Proto.toml @@ -0,0 +1,4 @@ +edition = "0.13" + +[workspace] +members = ["pkg1"] diff --git a/tests/cmd/install/workspace/range_cascade_resolution/in/pkg1/Proto.toml b/tests/cmd/install/workspace/range_cascade_resolution/in/pkg1/Proto.toml new file mode 100644 index 00000000..ced8ab36 --- /dev/null +++ b/tests/cmd/install/workspace/range_cascade_resolution/in/pkg1/Proto.toml @@ -0,0 +1,8 @@ +edition = "0.13" + +[package] +type = "lib" +name = "workspace-pkg1" +version = "1.0.0" + +[dependencies] diff --git a/tests/cmd/install/workspace/range_cascade_resolution/in/pkg1/proto/pkg1.proto b/tests/cmd/install/workspace/range_cascade_resolution/in/pkg1/proto/pkg1.proto new file mode 100644 index 00000000..8bc6ffd6 --- /dev/null +++ b/tests/cmd/install/workspace/range_cascade_resolution/in/pkg1/proto/pkg1.proto @@ -0,0 +1,7 @@ +syntax = "proto3"; + +package pkg1; + +message Pkg1Message { + string value = 1; +} diff --git a/tests/cmd/install/workspace/range_cascade_resolution/mod.rs b/tests/cmd/install/workspace/range_cascade_resolution/mod.rs new file mode 100644 index 00000000..065e64bd --- /dev/null +++ b/tests/cmd/install/workspace/range_cascade_resolution/mod.rs @@ -0,0 +1,73 @@ +use crate::{VirtualFileSystem, with_test_registry}; + +/// Cascade case: a transitive dep adds a tighter requirement *after* the +/// resolver has already chosen a higher version of the shared package and +/// walked its transitives. The resolver must: +/// 1. Re-resolve the shared package to the lower version. +/// 2. Retract requirement edges contributed by the now-discarded version. +/// 3. Drop any package that becomes unreachable as a result. +/// 4. Walk the new version's transitives. +/// +/// Setup, all inside pkg1's graph: +/// lib-a@1.5.0 -> lib-b ^2.0.0 lib-a@1.2.0 -> lib-b ^1.0.0 +/// mid@1.0.0 -> lib-a ~1.2.0 +/// pkg1 -> lib-a ^1.0.0, mid ^1.0.0 +/// +/// Root deps are traversed sorted, so lib-a resolves to 1.5.0 and pulls in +/// lib-b@2.0.0 before mid contributes ~1.2.0. lib-a must then fall to 1.2.0, +/// lib-b@2.0.0 must be retracted, and lib-b@1.0.0 walked in its place. +#[test] +fn fixture() { + with_test_registry(|url| { + let vfs = VirtualFileSystem::copy(crate::parent_directory!().join("in")); + let buffrs_home = vfs.root().join("$HOME"); + let cwd = vfs.root(); + + for version in ["1.0.0", "2.0.0"] { + super::publish_leaf_lib(&cwd, &buffrs_home, url, "lib-b", version); + } + super::publish_lib_with_dep(&cwd, &buffrs_home, url, "lib-a", "1.5.0", "lib-b", "^2.0.0"); + super::publish_lib_with_dep(&cwd, &buffrs_home, url, "lib-a", "1.2.0", "lib-b", "^1.0.0"); + super::publish_lib_with_dep(&cwd, &buffrs_home, url, "mid", "1.0.0", "lib-a", "~1.2.0"); + + for spec in ["test-repo/lib-a@^1.0.0", "test-repo/mid@^1.0.0"] { + crate::cli!() + .args(["add", "--registry", url, spec]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(cwd.join("pkg1")) + .assert() + .success(); + } + + crate::cli!() + .arg("install") + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&cwd) + .assert() + .success(); + + let lockfile = std::fs::read_to_string(cwd.join("Proto.lock")).unwrap(); + let section = |name: &str| { + lockfile + .split("[[packages]]") + .find(|s| s.contains(&format!("name = \"{name}\""))) + .unwrap_or_else(|| panic!("{name} missing from lockfile: {lockfile}")) + .to_string() + }; + + assert!( + section("lib-a").contains("version = \"1.2.0\""), + "lib-a must downgrade: {}", + section("lib-a") + ); + assert!( + section("lib-b").contains("version = \"1.0.0\""), + "lib-b must follow the new lib-a: {}", + section("lib-b") + ); + assert!( + !lockfile.contains("version = \"2.0.0\""), + "retracted lib-b@2.0.0 leaked into the lockfile: {lockfile}" + ); + }) +} diff --git a/tests/cmd/install/workspace/range_compatible_diamond/in/Proto.toml b/tests/cmd/install/workspace/range_compatible_diamond/in/Proto.toml new file mode 100644 index 00000000..4e623662 --- /dev/null +++ b/tests/cmd/install/workspace/range_compatible_diamond/in/Proto.toml @@ -0,0 +1,4 @@ +edition = "0.12" + +[workspace] +members = ["pkg1", "pkg2"] diff --git a/tests/cmd/install/workspace/range_compatible_diamond/in/pkg1/Proto.toml b/tests/cmd/install/workspace/range_compatible_diamond/in/pkg1/Proto.toml new file mode 100644 index 00000000..055e8789 --- /dev/null +++ b/tests/cmd/install/workspace/range_compatible_diamond/in/pkg1/Proto.toml @@ -0,0 +1,8 @@ +edition = "0.12" + +[package] +type = "lib" +name = "workspace-pkg1" +version = "1.0.0" + +[dependencies] diff --git a/tests/cmd/install/workspace/range_compatible_diamond/in/pkg1/proto/pkg1.proto b/tests/cmd/install/workspace/range_compatible_diamond/in/pkg1/proto/pkg1.proto new file mode 100644 index 00000000..f8deb6b3 --- /dev/null +++ b/tests/cmd/install/workspace/range_compatible_diamond/in/pkg1/proto/pkg1.proto @@ -0,0 +1,7 @@ +syntax = "proto3"; + +package workspace.pkg1; + +message Pkg1Message { + string value = 1; +} diff --git a/tests/cmd/install/workspace/range_compatible_diamond/in/pkg2/Proto.toml b/tests/cmd/install/workspace/range_compatible_diamond/in/pkg2/Proto.toml new file mode 100644 index 00000000..bfbf81a0 --- /dev/null +++ b/tests/cmd/install/workspace/range_compatible_diamond/in/pkg2/Proto.toml @@ -0,0 +1,8 @@ +edition = "0.12" + +[package] +type = "lib" +name = "workspace-pkg2" +version = "1.0.0" + +[dependencies] diff --git a/tests/cmd/install/workspace/range_compatible_diamond/in/pkg2/proto/pkg2.proto b/tests/cmd/install/workspace/range_compatible_diamond/in/pkg2/proto/pkg2.proto new file mode 100644 index 00000000..3a7da6e7 --- /dev/null +++ b/tests/cmd/install/workspace/range_compatible_diamond/in/pkg2/proto/pkg2.proto @@ -0,0 +1,7 @@ +syntax = "proto3"; + +package workspace.pkg2; + +message Pkg2Message { + string value = 1; +} diff --git a/tests/cmd/install/workspace/range_compatible_diamond/mod.rs b/tests/cmd/install/workspace/range_compatible_diamond/mod.rs new file mode 100644 index 00000000..f862b788 --- /dev/null +++ b/tests/cmd/install/workspace/range_compatible_diamond/mod.rs @@ -0,0 +1,219 @@ +use crate::{VirtualFileSystem, with_test_registry}; + +/// Verifies that compatible but different range requirements on the same +/// transitive leaf are resolved to a single version within one package's +/// dependency graph. +/// +/// Diamond shape (all within pkg1): +/// pkg1 --(^1.0)--> lib-a --(^1.0.0)--> leaf-lib +/// pkg1 --(^1.0)--> lib-b --(>=1.2.0)--> leaf-lib +/// +/// leaf-lib is available at v1.0.0 and v1.5.0. The resolver merges both +/// requirements before picking, and v1.5.0 is the highest version satisfying +/// ^1.0.0 and >=1.2.0 together. Install must succeed and the lockfile must +/// pin leaf-lib at v1.5.0 exactly once. +#[test] +fn fixture() { + with_test_registry(|url| { + let vfs = VirtualFileSystem::copy(crate::parent_directory!().join("in")); + let buffrs_home = vfs.root().join("$HOME"); + let cwd = vfs.root(); + + // Publish leaf-lib at v1.0.0 + { + let lib_dir = cwd.join("leaf-lib-v1-0"); + std::fs::create_dir(&lib_dir).unwrap(); + + crate::cli!() + .args(["init", "--lib", "leaf-lib"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + + let manifest_path = lib_dir.join("Proto.toml"); + let manifest = std::fs::read_to_string(&manifest_path).unwrap(); + let updated = manifest.replace("version = \"0.1.0\"", "version = \"1.0.0\""); + std::fs::write(&manifest_path, updated).unwrap(); + + std::fs::write( + lib_dir.join("proto/leaf.proto"), + "syntax = \"proto3\";\n\npackage leaf;\n\nmessage LeafMessage {\n string value = 1;\n}\n", + ) + .unwrap(); + + crate::cli!() + .args(["publish", "--registry", url, "--repository", "test-repo"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + } + + // Publish leaf-lib at v1.5.0 (satisfies both ^1.0.0 and >=1.2.0) + { + let lib_dir = cwd.join("leaf-lib-v1-5"); + std::fs::create_dir(&lib_dir).unwrap(); + + crate::cli!() + .args(["init", "--lib", "leaf-lib"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + + let manifest_path = lib_dir.join("Proto.toml"); + let manifest = std::fs::read_to_string(&manifest_path).unwrap(); + let updated = manifest.replace("version = \"0.1.0\"", "version = \"1.5.0\""); + std::fs::write(&manifest_path, updated).unwrap(); + + std::fs::write( + lib_dir.join("proto/leaf.proto"), + "syntax = \"proto3\";\n\npackage leaf;\n\nmessage LeafMessage {\n string value = 1;\n int32 count = 2;\n}\n", + ) + .unwrap(); + + crate::cli!() + .args(["publish", "--registry", url, "--repository", "test-repo"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + } + + // Publish lib-a at v1.0.0 (depends on leaf-lib@^1.0.0) + { + let lib_dir = cwd.join("lib-a"); + std::fs::create_dir(&lib_dir).unwrap(); + + crate::cli!() + .args(["init", "--lib", "lib-a"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + + let manifest_path = lib_dir.join("Proto.toml"); + let manifest = std::fs::read_to_string(&manifest_path).unwrap(); + let updated = manifest.replace("version = \"0.1.0\"", "version = \"1.0.0\""); + std::fs::write(&manifest_path, updated).unwrap(); + + std::fs::write( + lib_dir.join("proto/liba.proto"), + "syntax = \"proto3\";\n\npackage liba;\n\nmessage LibAMessage {\n string value = 1;\n}\n", + ) + .unwrap(); + + crate::cli!() + .args(["add", "--registry", url, "test-repo/leaf-lib@^1.0.0"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + + crate::cli!() + .args(["publish", "--registry", url, "--repository", "test-repo"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + } + + // Publish lib-b at v1.0.0 (depends on leaf-lib@>=1.2.0, compatible with 1.5.0) + { + let lib_dir = cwd.join("lib-b"); + std::fs::create_dir(&lib_dir).unwrap(); + + crate::cli!() + .args(["init", "--lib", "lib-b"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + + let manifest_path = lib_dir.join("Proto.toml"); + let manifest = std::fs::read_to_string(&manifest_path).unwrap(); + let updated = manifest.replace("version = \"0.1.0\"", "version = \"1.0.0\""); + std::fs::write(&manifest_path, updated).unwrap(); + + std::fs::write( + lib_dir.join("proto/libb.proto"), + "syntax = \"proto3\";\n\npackage libb;\n\nmessage LibBMessage {\n string value = 1;\n}\n", + ) + .unwrap(); + + crate::cli!() + .args(["add", "--registry", url, "test-repo/leaf-lib@>=1.2.0"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + + crate::cli!() + .args(["publish", "--registry", url, "--repository", "test-repo"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + } + + std::fs::remove_dir_all(cwd.join("leaf-lib-v1-0")).unwrap(); + std::fs::remove_dir_all(cwd.join("leaf-lib-v1-5")).unwrap(); + std::fs::remove_dir_all(cwd.join("lib-a")).unwrap(); + std::fs::remove_dir_all(cwd.join("lib-b")).unwrap(); + + // pkg1 depends on BOTH lib-a and lib-b — creating a diamond on leaf-lib + // within a single package's dependency graph. + crate::cli!() + .args(["add", "--registry", url, "test-repo/lib-a@^1.0.0"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(cwd.join("pkg1")) + .assert() + .success(); + + crate::cli!() + .args(["add", "--registry", url, "test-repo/lib-b@^1.0.0"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(cwd.join("pkg1")) + .assert() + .success(); + + // Install must succeed — 1.5.0 satisfies both ^1.0.0 and >=1.2.0 + crate::cli!() + .arg("install") + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&cwd) + .assert() + .success(); + + let lockfile_path = cwd.join("Proto.lock"); + assert!(lockfile_path.exists(), "Proto.lock should exist"); + + let lockfile = std::fs::read_to_string(&lockfile_path).unwrap(); + + // leaf-lib must appear exactly once in the lockfile + let leaf_count = lockfile.matches("name = \"leaf-lib\"").count(); + assert_eq!( + leaf_count, 1, + "leaf-lib should appear exactly once in the lockfile (got {})", + leaf_count + ); + + // Find the leaf-lib section and verify its pinned version + let leaf_section = lockfile + .split("[[packages]]") + .find(|s| s.contains("name = \"leaf-lib\"")) + .expect("leaf-lib section should exist in lockfile"); + + assert!( + leaf_section.contains("version = \"1.5.0\""), + "leaf-lib should be pinned at 1.5.0 (satisfies both ^1.0.0 and >=1.2.0), got:\n{}", + leaf_section + ); + assert!( + !leaf_section.contains("version = \"1.0.0\""), + "leaf-lib must not be pinned at 1.0.0 (does not satisfy >=1.2.0), got:\n{}", + leaf_section + ); + }) +} diff --git a/tests/cmd/install/workspace/range_downgrade_diamond/in/Proto.toml b/tests/cmd/install/workspace/range_downgrade_diamond/in/Proto.toml new file mode 100644 index 00000000..b6ce836e --- /dev/null +++ b/tests/cmd/install/workspace/range_downgrade_diamond/in/Proto.toml @@ -0,0 +1,4 @@ +edition = "0.13" + +[workspace] +members = ["pkg1"] diff --git a/tests/cmd/install/workspace/range_downgrade_diamond/in/pkg1/Proto.toml b/tests/cmd/install/workspace/range_downgrade_diamond/in/pkg1/Proto.toml new file mode 100644 index 00000000..ced8ab36 --- /dev/null +++ b/tests/cmd/install/workspace/range_downgrade_diamond/in/pkg1/Proto.toml @@ -0,0 +1,8 @@ +edition = "0.13" + +[package] +type = "lib" +name = "workspace-pkg1" +version = "1.0.0" + +[dependencies] diff --git a/tests/cmd/install/workspace/range_downgrade_diamond/in/pkg1/proto/pkg1.proto b/tests/cmd/install/workspace/range_downgrade_diamond/in/pkg1/proto/pkg1.proto new file mode 100644 index 00000000..8bc6ffd6 --- /dev/null +++ b/tests/cmd/install/workspace/range_downgrade_diamond/in/pkg1/proto/pkg1.proto @@ -0,0 +1,7 @@ +syntax = "proto3"; + +package pkg1; + +message Pkg1Message { + string value = 1; +} diff --git a/tests/cmd/install/workspace/range_downgrade_diamond/mod.rs b/tests/cmd/install/workspace/range_downgrade_diamond/mod.rs new file mode 100644 index 00000000..ffdae5a2 --- /dev/null +++ b/tests/cmd/install/workspace/range_downgrade_diamond/mod.rs @@ -0,0 +1,72 @@ +use crate::{VirtualFileSystem, with_test_registry}; + +/// Transitive diamond inside a single package's graph, where the highest +/// available version satisfies the first-encountered requirement but not the +/// second. +/// +/// pkg1 --(^1.0.0)--> lib-x --(^1.0.0)--> lib-a +/// pkg1 --(^1.0.0)--> lib-y --(<=1.2.0)-> lib-a +/// +/// lib-a is published at 1.0.0, 1.2.0 and 1.5.0. First-encounter-wins resolves +/// lib-a to 1.5.0 via lib-x and then fails against lib-y's <=1.2.0. +/// Merge-and-resolve must intersect both requirements and pick 1.2.0. +/// +/// The diamond is expressed transitively on purpose: workspace members are +/// resolved independently by design, so two members are two resolutions rather +/// than one merge. +#[test] +fn fixture() { + with_test_registry(|url| { + let vfs = VirtualFileSystem::copy(crate::parent_directory!().join("in")); + let buffrs_home = vfs.root().join("$HOME"); + let cwd = vfs.root(); + + for version in ["1.0.0", "1.2.0", "1.5.0"] { + super::publish_leaf_lib(&cwd, &buffrs_home, url, "lib-a", version); + } + super::publish_lib_with_dep(&cwd, &buffrs_home, url, "lib-x", "1.0.0", "lib-a", "^1.0.0"); + super::publish_lib_with_dep( + &cwd, + &buffrs_home, + url, + "lib-y", + "1.0.0", + "lib-a", + "<=1.2.0", + ); + + for dep in ["lib-x", "lib-y"] { + crate::cli!() + .args(["add", "--registry", url, &format!("test-repo/{dep}@^1.0.0")]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(cwd.join("pkg1")) + .assert() + .success(); + } + + crate::cli!() + .arg("install") + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&cwd) + .assert() + .success(); + + let lockfile = std::fs::read_to_string(cwd.join("Proto.lock")).unwrap(); + + assert_eq!( + lockfile.matches("name = \"lib-a\"").count(), + 1, + "lib-a must be resolved to a single version: {lockfile}" + ); + + let section = lockfile + .split("[[packages]]") + .find(|s| s.contains("name = \"lib-a\"")) + .expect("lib-a in lockfile"); + + assert!( + section.contains("version = \"1.2.0\""), + "expected lib-a 1.2.0, got: {section}" + ); + }) +} diff --git a/tests/cmd/install/workspace/range_incompatible_diamond/in/Proto.toml b/tests/cmd/install/workspace/range_incompatible_diamond/in/Proto.toml new file mode 100644 index 00000000..4e623662 --- /dev/null +++ b/tests/cmd/install/workspace/range_incompatible_diamond/in/Proto.toml @@ -0,0 +1,4 @@ +edition = "0.12" + +[workspace] +members = ["pkg1", "pkg2"] diff --git a/tests/cmd/install/workspace/range_incompatible_diamond/in/pkg1/Proto.toml b/tests/cmd/install/workspace/range_incompatible_diamond/in/pkg1/Proto.toml new file mode 100644 index 00000000..055e8789 --- /dev/null +++ b/tests/cmd/install/workspace/range_incompatible_diamond/in/pkg1/Proto.toml @@ -0,0 +1,8 @@ +edition = "0.12" + +[package] +type = "lib" +name = "workspace-pkg1" +version = "1.0.0" + +[dependencies] diff --git a/tests/cmd/install/workspace/range_incompatible_diamond/in/pkg1/proto/pkg1.proto b/tests/cmd/install/workspace/range_incompatible_diamond/in/pkg1/proto/pkg1.proto new file mode 100644 index 00000000..f8deb6b3 --- /dev/null +++ b/tests/cmd/install/workspace/range_incompatible_diamond/in/pkg1/proto/pkg1.proto @@ -0,0 +1,7 @@ +syntax = "proto3"; + +package workspace.pkg1; + +message Pkg1Message { + string value = 1; +} diff --git a/tests/cmd/install/workspace/range_incompatible_diamond/in/pkg2/Proto.toml b/tests/cmd/install/workspace/range_incompatible_diamond/in/pkg2/Proto.toml new file mode 100644 index 00000000..bfbf81a0 --- /dev/null +++ b/tests/cmd/install/workspace/range_incompatible_diamond/in/pkg2/Proto.toml @@ -0,0 +1,8 @@ +edition = "0.12" + +[package] +type = "lib" +name = "workspace-pkg2" +version = "1.0.0" + +[dependencies] diff --git a/tests/cmd/install/workspace/range_incompatible_diamond/in/pkg2/proto/pkg2.proto b/tests/cmd/install/workspace/range_incompatible_diamond/in/pkg2/proto/pkg2.proto new file mode 100644 index 00000000..3a7da6e7 --- /dev/null +++ b/tests/cmd/install/workspace/range_incompatible_diamond/in/pkg2/proto/pkg2.proto @@ -0,0 +1,7 @@ +syntax = "proto3"; + +package workspace.pkg2; + +message Pkg2Message { + string value = 1; +} diff --git a/tests/cmd/install/workspace/range_incompatible_diamond/mod.rs b/tests/cmd/install/workspace/range_incompatible_diamond/mod.rs new file mode 100644 index 00000000..4b291870 --- /dev/null +++ b/tests/cmd/install/workspace/range_incompatible_diamond/mod.rs @@ -0,0 +1,205 @@ +use crate::{VirtualFileSystem, with_test_registry}; + +/// Verifies that incompatible range requirements on the same transitive leaf +/// within a single package's dependency graph cause `buffrs install` to fail +/// with a clear version-conflict error. +/// +/// Diamond shape (all within pkg1): +/// pkg1 --(^1.0)--> lib-a --(^1.0.0)--> leaf-lib (resolves to v1.0.0) +/// pkg1 --(^1.0)--> lib-b --(^2.0.0)--> leaf-lib (requires v2.x — conflict) +/// +/// No single version of leaf-lib satisfies both ^1.0.0 and ^2.0.0. The resolver +/// merges both requirements before picking and must report that the intersection +/// is empty, listing every requirement it gathered. +#[test] +fn fixture() { + with_test_registry(|url| { + let vfs = VirtualFileSystem::copy(crate::parent_directory!().join("in")); + let buffrs_home = vfs.root().join("$HOME"); + let cwd = vfs.root(); + + // Publish leaf-lib at v1.0.0 + { + let lib_dir = cwd.join("leaf-lib-v1"); + std::fs::create_dir(&lib_dir).unwrap(); + + crate::cli!() + .args(["init", "--lib", "leaf-lib"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + + let manifest_path = lib_dir.join("Proto.toml"); + let manifest = std::fs::read_to_string(&manifest_path).unwrap(); + let updated = manifest.replace("version = \"0.1.0\"", "version = \"1.0.0\""); + std::fs::write(&manifest_path, updated).unwrap(); + + std::fs::write( + lib_dir.join("proto/leaf.proto"), + "syntax = \"proto3\";\n\npackage leaf;\n\nmessage LeafMessage {\n string value = 1;\n}\n", + ) + .unwrap(); + + crate::cli!() + .args(["publish", "--registry", url, "--repository", "test-repo"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + } + + // Publish leaf-lib at v2.0.0 + { + let lib_dir = cwd.join("leaf-lib-v2"); + std::fs::create_dir(&lib_dir).unwrap(); + + crate::cli!() + .args(["init", "--lib", "leaf-lib"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + + let manifest_path = lib_dir.join("Proto.toml"); + let manifest = std::fs::read_to_string(&manifest_path).unwrap(); + let updated = manifest.replace("version = \"0.1.0\"", "version = \"2.0.0\""); + std::fs::write(&manifest_path, updated).unwrap(); + + std::fs::write( + lib_dir.join("proto/leaf.proto"), + "syntax = \"proto3\";\n\npackage leaf;\n\nmessage LeafV2 {\n string value = 1;\n}\n", + ) + .unwrap(); + + crate::cli!() + .args(["publish", "--registry", url, "--repository", "test-repo"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + } + + // Publish lib-a at v1.0.0 (depends on leaf-lib@^1.0.0 — resolves to v1.0.0) + { + let lib_dir = cwd.join("lib-a"); + std::fs::create_dir(&lib_dir).unwrap(); + + crate::cli!() + .args(["init", "--lib", "lib-a"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + + let manifest_path = lib_dir.join("Proto.toml"); + let manifest = std::fs::read_to_string(&manifest_path).unwrap(); + let updated = manifest.replace("version = \"0.1.0\"", "version = \"1.0.0\""); + std::fs::write(&manifest_path, updated).unwrap(); + + std::fs::write( + lib_dir.join("proto/liba.proto"), + "syntax = \"proto3\";\n\npackage liba;\n\nmessage LibAMessage {\n string value = 1;\n}\n", + ) + .unwrap(); + + crate::cli!() + .args(["add", "--registry", url, "test-repo/leaf-lib@^1.0.0"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + + crate::cli!() + .args(["publish", "--registry", url, "--repository", "test-repo"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + } + + // Publish lib-b at v1.0.0 (depends on leaf-lib@^2.0.0 — incompatible with ^1.0.0) + { + let lib_dir = cwd.join("lib-b"); + std::fs::create_dir(&lib_dir).unwrap(); + + crate::cli!() + .args(["init", "--lib", "lib-b"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + + let manifest_path = lib_dir.join("Proto.toml"); + let manifest = std::fs::read_to_string(&manifest_path).unwrap(); + let updated = manifest.replace("version = \"0.1.0\"", "version = \"1.0.0\""); + std::fs::write(&manifest_path, updated).unwrap(); + + std::fs::write( + lib_dir.join("proto/libb.proto"), + "syntax = \"proto3\";\n\npackage libb;\n\nmessage LibBMessage {\n string value = 1;\n}\n", + ) + .unwrap(); + + crate::cli!() + .args(["add", "--registry", url, "test-repo/leaf-lib@^2.0.0"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + + crate::cli!() + .args(["publish", "--registry", url, "--repository", "test-repo"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + } + + std::fs::remove_dir_all(cwd.join("leaf-lib-v1")).unwrap(); + std::fs::remove_dir_all(cwd.join("leaf-lib-v2")).unwrap(); + std::fs::remove_dir_all(cwd.join("lib-a")).unwrap(); + std::fs::remove_dir_all(cwd.join("lib-b")).unwrap(); + + // pkg1 depends on BOTH lib-a and lib-b — the diamond conflict is within + // a single package's dependency graph, where validate_version_compatibility + // must reject leaf-lib@^2.0.0 after resolving leaf-lib to 1.0.0 via lib-a. + crate::cli!() + .args(["add", "--registry", url, "test-repo/lib-a@^1.0.0"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(cwd.join("pkg1")) + .assert() + .success(); + + crate::cli!() + .args(["add", "--registry", url, "test-repo/lib-b@^1.0.0"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(cwd.join("pkg1")) + .assert() + .success(); + + // Install must fail: leaf-lib cannot satisfy both ^1.0.0 and ^2.0.0 + let output = crate::cli!() + .arg("install") + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&cwd) + .output() + .unwrap(); + + assert!( + !output.status.success(), + "install should fail when two branches of the same package require incompatible \ + versions of leaf-lib (^1.0.0 vs ^2.0.0)" + ); + + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("no version of leaf-lib satisfies all requirements"), + "expected NoCompatibleVersion error mentioning leaf-lib, got:\n{stderr}" + ); + assert!( + stderr.contains("^1.0.0") && stderr.contains("^2.0.0"), + "error must list both incompatible requirements, got:\n{stderr}" + ); + }) +} diff --git a/tests/cmd/install/workspace/range_lockfile_reresolution/in/Proto.toml b/tests/cmd/install/workspace/range_lockfile_reresolution/in/Proto.toml new file mode 100644 index 00000000..b6ce836e --- /dev/null +++ b/tests/cmd/install/workspace/range_lockfile_reresolution/in/Proto.toml @@ -0,0 +1,4 @@ +edition = "0.13" + +[workspace] +members = ["pkg1"] diff --git a/tests/cmd/install/workspace/range_lockfile_reresolution/in/pkg1/Proto.toml b/tests/cmd/install/workspace/range_lockfile_reresolution/in/pkg1/Proto.toml new file mode 100644 index 00000000..ced8ab36 --- /dev/null +++ b/tests/cmd/install/workspace/range_lockfile_reresolution/in/pkg1/Proto.toml @@ -0,0 +1,8 @@ +edition = "0.13" + +[package] +type = "lib" +name = "workspace-pkg1" +version = "1.0.0" + +[dependencies] diff --git a/tests/cmd/install/workspace/range_lockfile_reresolution/in/pkg1/proto/pkg1.proto b/tests/cmd/install/workspace/range_lockfile_reresolution/in/pkg1/proto/pkg1.proto new file mode 100644 index 00000000..8bc6ffd6 --- /dev/null +++ b/tests/cmd/install/workspace/range_lockfile_reresolution/in/pkg1/proto/pkg1.proto @@ -0,0 +1,7 @@ +syntax = "proto3"; + +package pkg1; + +message Pkg1Message { + string value = 1; +} diff --git a/tests/cmd/install/workspace/range_lockfile_reresolution/mod.rs b/tests/cmd/install/workspace/range_lockfile_reresolution/mod.rs new file mode 100644 index 00000000..9ebdacdb --- /dev/null +++ b/tests/cmd/install/workspace/range_lockfile_reresolution/mod.rs @@ -0,0 +1,90 @@ +use crate::{VirtualFileSystem, with_test_registry}; + +/// The first install pins lib-a@1.5.0 via lib-x's ^1.0.0. pkg1 then gains +/// lib-y, whose <=1.2.0 the pin no longer satisfies. The second install must +/// re-resolve lib-a to 1.2.0 and rewrite the lockfile — a pin that fails the +/// merged requirements is not an error, it is simply not a usable answer. +/// +/// This matters for workspaces in particular: the workspace lockfile is a pool +/// shared by independently-resolved members, so an entry that satisfies nothing +/// here may legitimately belong to another member. +#[test] +fn fixture() { + with_test_registry(|url| { + let vfs = VirtualFileSystem::copy(crate::parent_directory!().join("in")); + let buffrs_home = vfs.root().join("$HOME"); + let cwd = vfs.root(); + + for version in ["1.0.0", "1.2.0", "1.5.0"] { + super::publish_leaf_lib(&cwd, &buffrs_home, url, "lib-a", version); + } + super::publish_lib_with_dep(&cwd, &buffrs_home, url, "lib-x", "1.0.0", "lib-a", "^1.0.0"); + super::publish_lib_with_dep( + &cwd, + &buffrs_home, + url, + "lib-y", + "1.0.0", + "lib-a", + "<=1.2.0", + ); + + crate::cli!() + .args(["add", "--registry", url, "test-repo/lib-x@^1.0.0"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(cwd.join("pkg1")) + .assert() + .success(); + + crate::cli!() + .arg("install") + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&cwd) + .assert() + .success(); + + let lib_a = |lock: &str| { + lock.split("[[packages]]") + .find(|s| s.contains("name = \"lib-a\"")) + .expect("lib-a in lockfile") + .to_string() + }; + + let lockfile = std::fs::read_to_string(cwd.join("Proto.lock")).unwrap(); + assert!( + lib_a(&lockfile).contains("version = \"1.5.0\""), + "first install pins the highest version satisfying ^1.0.0: {lockfile}" + ); + + // Introduce a second path to lib-a that the existing pin cannot satisfy. + crate::cli!() + .args(["add", "--registry", url, "test-repo/lib-y@^1.0.0"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(cwd.join("pkg1")) + .assert() + .success(); + + crate::cli!() + .arg("install") + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&cwd) + .assert() + .success(); + + let lockfile = std::fs::read_to_string(cwd.join("Proto.lock")).unwrap(); + assert_eq!( + lockfile.matches("name = \"lib-a\"").count(), + 1, + "lib-a must be re-pinned, not duplicated: {lockfile}" + ); + assert!( + lib_a(&lockfile).contains("version = \"1.2.0\""), + "stale pin must be re-resolved to 1.2.0: {}", + lib_a(&lockfile) + ); + assert!( + lockfile.contains("name = \"lib-x\"") && lockfile.contains("name = \"lib-y\""), + "both intermediates must remain locked: {lockfile}" + ); + }) +} diff --git a/tests/cmd/install/workspace/range_multi_level_tree/in/Proto.toml b/tests/cmd/install/workspace/range_multi_level_tree/in/Proto.toml new file mode 100644 index 00000000..7873c093 --- /dev/null +++ b/tests/cmd/install/workspace/range_multi_level_tree/in/Proto.toml @@ -0,0 +1,4 @@ +edition = "0.12" + +[workspace] +members = ["pkg1"] diff --git a/tests/cmd/install/workspace/range_multi_level_tree/in/pkg1/Proto.toml b/tests/cmd/install/workspace/range_multi_level_tree/in/pkg1/Proto.toml new file mode 100644 index 00000000..055e8789 --- /dev/null +++ b/tests/cmd/install/workspace/range_multi_level_tree/in/pkg1/Proto.toml @@ -0,0 +1,8 @@ +edition = "0.12" + +[package] +type = "lib" +name = "workspace-pkg1" +version = "1.0.0" + +[dependencies] diff --git a/tests/cmd/install/workspace/range_multi_level_tree/in/pkg1/proto/pkg1.proto b/tests/cmd/install/workspace/range_multi_level_tree/in/pkg1/proto/pkg1.proto new file mode 100644 index 00000000..f8deb6b3 --- /dev/null +++ b/tests/cmd/install/workspace/range_multi_level_tree/in/pkg1/proto/pkg1.proto @@ -0,0 +1,7 @@ +syntax = "proto3"; + +package workspace.pkg1; + +message Pkg1Message { + string value = 1; +} diff --git a/tests/cmd/install/workspace/range_multi_level_tree/mod.rs b/tests/cmd/install/workspace/range_multi_level_tree/mod.rs new file mode 100644 index 00000000..2f9193d2 --- /dev/null +++ b/tests/cmd/install/workspace/range_multi_level_tree/mod.rs @@ -0,0 +1,168 @@ +use crate::{VirtualFileSystem, with_test_registry}; + +/// Verifies that range resolution propagates correctly through a three-level +/// dependency tree: pkg1 --(^2.0)--> mid-lib --(^1.0)--> leaf-lib. +/// +/// Publishes leaf-lib at v1.0.0 and v1.5.0, then mid-lib at v2.0.0 (which +/// declares `leaf-lib@^1.0.0`), then installs pkg1 with `mid-lib@^2.0.0`. +/// Expects mid-lib pinned at 2.0.0 and leaf-lib pinned at 1.5.0. +#[test] +fn fixture() { + with_test_registry(|url| { + let vfs = VirtualFileSystem::copy(crate::parent_directory!().join("in")); + let buffrs_home = vfs.root().join("$HOME"); + let cwd = vfs.root(); + + // Publish leaf-lib at v1.0.0 + { + let lib_dir = cwd.join("leaf-lib-v1-0"); + std::fs::create_dir(&lib_dir).unwrap(); + + crate::cli!() + .args(["init", "--lib", "leaf-lib"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + + let manifest_path = lib_dir.join("Proto.toml"); + let manifest = std::fs::read_to_string(&manifest_path).unwrap(); + let updated = manifest.replace("version = \"0.1.0\"", "version = \"1.0.0\""); + std::fs::write(&manifest_path, updated).unwrap(); + + std::fs::write( + lib_dir.join("proto/leaf.proto"), + "syntax = \"proto3\";\n\npackage leaf;\n\nmessage LeafMessage {\n string value = 1;\n}\n", + ) + .unwrap(); + + crate::cli!() + .args(["publish", "--registry", url, "--repository", "test-repo"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + } + + // Publish leaf-lib at v1.5.0 (should be resolved by mid-lib's ^1.0.0) + { + let lib_dir = cwd.join("leaf-lib-v1-5"); + std::fs::create_dir(&lib_dir).unwrap(); + + crate::cli!() + .args(["init", "--lib", "leaf-lib"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + + let manifest_path = lib_dir.join("Proto.toml"); + let manifest = std::fs::read_to_string(&manifest_path).unwrap(); + let updated = manifest.replace("version = \"0.1.0\"", "version = \"1.5.0\""); + std::fs::write(&manifest_path, updated).unwrap(); + + std::fs::write( + lib_dir.join("proto/leaf.proto"), + "syntax = \"proto3\";\n\npackage leaf;\n\nmessage LeafMessage {\n string value = 1;\n int32 count = 2;\n}\n", + ) + .unwrap(); + + crate::cli!() + .args(["publish", "--registry", url, "--repository", "test-repo"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + } + + // Publish mid-lib at v2.0.0 with a transitive dependency on leaf-lib@^1.0.0 + { + let lib_dir = cwd.join("mid-lib"); + std::fs::create_dir(&lib_dir).unwrap(); + + crate::cli!() + .args(["init", "--lib", "mid-lib"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + + let manifest_path = lib_dir.join("Proto.toml"); + let manifest = std::fs::read_to_string(&manifest_path).unwrap(); + let updated = manifest.replace("version = \"0.1.0\"", "version = \"2.0.0\""); + std::fs::write(&manifest_path, updated).unwrap(); + + std::fs::write( + lib_dir.join("proto/mid.proto"), + "syntax = \"proto3\";\n\npackage mid;\n\nmessage MidMessage {\n string value = 1;\n}\n", + ) + .unwrap(); + + // mid-lib depends on leaf-lib@^1.0.0 + crate::cli!() + .args(["add", "--registry", url, "test-repo/leaf-lib@^1.0.0"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + + crate::cli!() + .args(["publish", "--registry", url, "--repository", "test-repo"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + } + + std::fs::remove_dir_all(cwd.join("leaf-lib-v1-0")).unwrap(); + std::fs::remove_dir_all(cwd.join("leaf-lib-v1-5")).unwrap(); + std::fs::remove_dir_all(cwd.join("mid-lib")).unwrap(); + + // Add mid-lib with caret requirement to pkg1 + crate::cli!() + .args(["add", "--registry", url, "test-repo/mid-lib@^2.0.0"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(cwd.join("pkg1")) + .assert() + .success(); + + // Install at workspace root + crate::cli!() + .arg("install") + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&cwd) + .assert() + .success(); + + let lockfile_path = cwd.join("Proto.lock"); + assert!(lockfile_path.exists(), "Proto.lock should exist"); + + let lockfile = std::fs::read_to_string(&lockfile_path).unwrap(); + + // mid-lib@^2.0.0 should resolve to the only available: 2.0.0 + assert!( + lockfile.contains("name = \"mid-lib\""), + "lockfile should contain mid-lib" + ); + assert!( + lockfile.contains("version = \"2.0.0\""), + "mid-lib@^2.0.0 should resolve to 2.0.0, got:\n{}", + lockfile + ); + + // mid-lib's transitive dep leaf-lib@^1.0.0 should resolve to 1.5.0 + assert!( + lockfile.contains("name = \"leaf-lib\""), + "lockfile should contain transitive dependency leaf-lib" + ); + assert!( + lockfile.contains("version = \"1.5.0\""), + "leaf-lib@^1.0.0 should resolve to 1.5.0 (highest compatible), got:\n{}", + lockfile + ); + assert!( + !lockfile.contains("version = \"1.0.0\""), + "leaf-lib must not pin to the minimum 1.0.0 when 1.5.0 is available" + ); + }) +} diff --git a/tests/cmd/install/workspace/range_tilde_resolution/in/Proto.toml b/tests/cmd/install/workspace/range_tilde_resolution/in/Proto.toml new file mode 100644 index 00000000..7873c093 --- /dev/null +++ b/tests/cmd/install/workspace/range_tilde_resolution/in/Proto.toml @@ -0,0 +1,4 @@ +edition = "0.12" + +[workspace] +members = ["pkg1"] diff --git a/tests/cmd/install/workspace/range_tilde_resolution/in/pkg1/Proto.toml b/tests/cmd/install/workspace/range_tilde_resolution/in/pkg1/Proto.toml new file mode 100644 index 00000000..055e8789 --- /dev/null +++ b/tests/cmd/install/workspace/range_tilde_resolution/in/pkg1/Proto.toml @@ -0,0 +1,8 @@ +edition = "0.12" + +[package] +type = "lib" +name = "workspace-pkg1" +version = "1.0.0" + +[dependencies] diff --git a/tests/cmd/install/workspace/range_tilde_resolution/in/pkg1/proto/pkg1.proto b/tests/cmd/install/workspace/range_tilde_resolution/in/pkg1/proto/pkg1.proto new file mode 100644 index 00000000..f8deb6b3 --- /dev/null +++ b/tests/cmd/install/workspace/range_tilde_resolution/in/pkg1/proto/pkg1.proto @@ -0,0 +1,7 @@ +syntax = "proto3"; + +package workspace.pkg1; + +message Pkg1Message { + string value = 1; +} diff --git a/tests/cmd/install/workspace/range_tilde_resolution/mod.rs b/tests/cmd/install/workspace/range_tilde_resolution/mod.rs new file mode 100644 index 00000000..6c3dc663 --- /dev/null +++ b/tests/cmd/install/workspace/range_tilde_resolution/mod.rs @@ -0,0 +1,148 @@ +use crate::{VirtualFileSystem, with_test_registry}; + +/// Verifies that a tilde version requirement (~1.2.0) resolves to the highest +/// patch version within the same minor, and does not cross into the next minor. +/// +/// Publishes leaf-lib at v1.2.0, v1.2.5, and v1.3.0, then installs with +/// `~1.2.0`. Expects the lockfile to pin v1.2.5 (not v1.3.0). +#[test] +fn fixture() { + with_test_registry(|url| { + let vfs = VirtualFileSystem::copy(crate::parent_directory!().join("in")); + let buffrs_home = vfs.root().join("$HOME"); + let cwd = vfs.root(); + + // Publish leaf-lib at v1.2.0 + { + let lib_dir = cwd.join("leaf-lib-v1-2-0"); + std::fs::create_dir(&lib_dir).unwrap(); + + crate::cli!() + .args(["init", "--lib", "leaf-lib"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + + let manifest_path = lib_dir.join("Proto.toml"); + let manifest = std::fs::read_to_string(&manifest_path).unwrap(); + let updated = manifest.replace("version = \"0.1.0\"", "version = \"1.2.0\""); + std::fs::write(&manifest_path, updated).unwrap(); + + std::fs::write( + lib_dir.join("proto/leaf.proto"), + "syntax = \"proto3\";\n\npackage leaf;\n\nmessage LeafMessage {\n string value = 1;\n}\n", + ) + .unwrap(); + + crate::cli!() + .args(["publish", "--registry", url, "--repository", "test-repo"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + } + + // Publish leaf-lib at v1.2.5 (highest in the 1.2.x range — should be selected) + { + let lib_dir = cwd.join("leaf-lib-v1-2-5"); + std::fs::create_dir(&lib_dir).unwrap(); + + crate::cli!() + .args(["init", "--lib", "leaf-lib"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + + let manifest_path = lib_dir.join("Proto.toml"); + let manifest = std::fs::read_to_string(&manifest_path).unwrap(); + let updated = manifest.replace("version = \"0.1.0\"", "version = \"1.2.5\""); + std::fs::write(&manifest_path, updated).unwrap(); + + std::fs::write( + lib_dir.join("proto/leaf.proto"), + "syntax = \"proto3\";\n\npackage leaf;\n\nmessage LeafMessage {\n string value = 1;\n string extra = 2;\n}\n", + ) + .unwrap(); + + crate::cli!() + .args(["publish", "--registry", url, "--repository", "test-repo"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + } + + // Publish leaf-lib at v1.3.0 (out of ~1.2.x range — must NOT be selected) + { + let lib_dir = cwd.join("leaf-lib-v1-3-0"); + std::fs::create_dir(&lib_dir).unwrap(); + + crate::cli!() + .args(["init", "--lib", "leaf-lib"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + + let manifest_path = lib_dir.join("Proto.toml"); + let manifest = std::fs::read_to_string(&manifest_path).unwrap(); + let updated = manifest.replace("version = \"0.1.0\"", "version = \"1.3.0\""); + std::fs::write(&manifest_path, updated).unwrap(); + + std::fs::write( + lib_dir.join("proto/leaf.proto"), + "syntax = \"proto3\";\n\npackage leaf;\n\nmessage LeafMessage {\n string value = 1;\n string extra = 2;\n int32 count = 3;\n}\n", + ) + .unwrap(); + + crate::cli!() + .args(["publish", "--registry", url, "--repository", "test-repo"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&lib_dir) + .assert() + .success(); + } + + std::fs::remove_dir_all(cwd.join("leaf-lib-v1-2-0")).unwrap(); + std::fs::remove_dir_all(cwd.join("leaf-lib-v1-2-5")).unwrap(); + std::fs::remove_dir_all(cwd.join("leaf-lib-v1-3-0")).unwrap(); + + // Add leaf-lib with tilde requirement to pkg1 + crate::cli!() + .args(["add", "--registry", url, "test-repo/leaf-lib@~1.2.0"]) + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(cwd.join("pkg1")) + .assert() + .success(); + + // Install at workspace root + crate::cli!() + .arg("install") + .env("BUFFRS_HOME", &buffrs_home) + .current_dir(&cwd) + .assert() + .success(); + + let lockfile_path = cwd.join("Proto.lock"); + assert!(lockfile_path.exists(), "Proto.lock should exist"); + + let lockfile = std::fs::read_to_string(&lockfile_path).unwrap(); + + // ~1.2.0 must resolve to the highest 1.2.x: 1.2.5 + assert!( + lockfile.contains("version = \"1.2.5\""), + "~1.2.0 should resolve to 1.2.5 (highest patch in 1.2.x), got:\n{}", + lockfile + ); + assert!( + !lockfile.contains("version = \"1.3.0\""), + "~1.2.0 must not cross into the next minor (1.3.0)" + ); + assert!( + !lockfile.contains("version = \"1.2.0\""), + "~1.2.0 must not pin to the minimum patch 1.2.0" + ); + }) +} diff --git a/tests/registry.rs b/tests/registry.rs index 6a7197ac..f0602235 100644 --- a/tests/registry.rs +++ b/tests/registry.rs @@ -11,6 +11,7 @@ use axum::{ }; use bytes::Bytes; use miette::{Context as _, IntoDiagnostic, miette}; +use serde::Deserialize; use tokio::net::TcpListener; type State = Arc>>; @@ -33,6 +34,8 @@ async fn test_registry( required_token, }; let app = Router::new() + // Artifactory-compatible artifact search endpoint (must be registered before the wildcard) + .route("/artifactory/api/search/artifact", get(search_artifacts)) .route("/{*path}", get(get_package).put(put_package)) .with_state(state); axum::serve(listener, app) @@ -41,6 +44,52 @@ async fn test_registry( .wrap_err(miette!("failed to read the token from the user")) } +/// Query parameters for the Artifactory artifact search endpoint +#[derive(Deserialize)] +struct SearchQuery { + name: String, + repos: String, +} + +/// Implements the Artifactory artifact search endpoint used by `list_versions`. +/// +/// Scans stored packages whose path matches `{any}/{repos}/{name}/{name}-{version}.tgz` +/// and returns them as an `ArtifactSearchResult` JSON payload. +async fn search_artifacts( + extract::State(state): extract::State, + extract::Query(query): extract::Query, +) -> impl IntoResponse { + let state = state.packages.read().unwrap(); + + // Stored keys look like: "{prefix}/{repos}/{name}/{name}-{version}.tgz" + let results: Vec = state + .keys() + .filter(|path| { + let segments: Vec<&str> = path.split('/').collect(); + // Need at least 4 segments: prefix / repos / name / filename + if segments.len() < 4 { + return false; + } + let repo_seg = segments[segments.len() - 3]; + let name_seg = segments[segments.len() - 2]; + repo_seg == query.repos && name_seg == query.name + }) + .map(|path| { + // The URI only needs to end with the correct filename for version parsing. + serde_json::json!({ "uri": format!("http://test-registry/{}", path) }) + }) + .collect(); + + let body = serde_json::json!({ "results": results }).to_string(); + ( + [( + header::CONTENT_TYPE, + "application/vnd.org.jfrog.artifactory.search.ArtifactSearchResult+json", + )], + body, + ) +} + // basic handler that responds with a static string async fn get_package( extract::State(state): extract::State, diff --git a/tests/resolver/mod.rs b/tests/resolver/mod.rs index 96819f89..88c4315e 100644 --- a/tests/resolver/mod.rs +++ b/tests/resolver/mod.rs @@ -824,6 +824,7 @@ fn build_test_graph(nodes: Vec<(PackageName, Vec)>) -> DependencyGr }, dependencies, version: VersionReq::STAR, + resolved_version: None, }, ); } @@ -1102,6 +1103,7 @@ fn test_topo_sort_detects_cycle() { }, dependencies: vec!["b".parse().expect("valid package name")], version: VersionReq::STAR, + resolved_version: None, }, ); @@ -1115,6 +1117,7 @@ fn test_topo_sort_detects_cycle() { }, dependencies: vec!["a".parse().expect("valid package name")], version: VersionReq::STAR, + resolved_version: None, }, );