From f2d707b2ea29187687f087c05d6406b7c5103ebb Mon Sep 17 00:00:00 2001 From: John Mumm Date: Wed, 11 Jun 2025 11:44:03 -0400 Subject: [PATCH 1/4] Take account of path dependency source indexes when validating lockfile --- crates/uv-resolver/src/lock/mod.rs | 6 +- crates/uv-workspace/src/workspace.rs | 125 +++++- crates/uv/src/commands/project/lock.rs | 20 +- crates/uv/src/commands/project/lock_target.rs | 8 + crates/uv/tests/it/lock.rs | 394 ++++++++++++++++++ 5 files changed, 543 insertions(+), 10 deletions(-) diff --git a/crates/uv-resolver/src/lock/mod.rs b/crates/uv-resolver/src/lock/mod.rs index faacae736c594..d5f9485ba7361 100644 --- a/crates/uv-resolver/src/lock/mod.rs +++ b/crates/uv-resolver/src/lock/mod.rs @@ -1233,7 +1233,7 @@ impl Lock { build_constraints: &[Requirement], dependency_groups: &BTreeMap>, dependency_metadata: &DependencyMetadata, - indexes: Option<&IndexLocations>, + indexes: Option>, tags: &Tags, hasher: &HashStrategy, index: &InMemoryIndex, @@ -1399,7 +1399,7 @@ impl Lock { } // Collect the set of available indexes (both `--index-url` and `--find-links` entries). - let remotes = indexes.map(|locations| { + let remotes = indexes.as_ref().map(|locations| { locations .allowed_indexes() .into_iter() @@ -1412,7 +1412,7 @@ impl Lock { .collect::>() }); - let locals = indexes.map(|locations| { + let locals = indexes.as_ref().map(|locations| { locations .allowed_indexes() .into_iter() diff --git a/crates/uv-workspace/src/workspace.rs b/crates/uv-workspace/src/workspace.rs index 3caaa8f8cb7a1..0ec8ad5317ed0 100644 --- a/crates/uv-workspace/src/workspace.rs +++ b/crates/uv-workspace/src/workspace.rs @@ -19,7 +19,7 @@ use uv_warnings::warn_user_once; use crate::dependency_groups::{DependencyGroupError, FlatDependencyGroups}; use crate::pyproject::{ - Project, PyProjectToml, PyprojectTomlError, Sources, ToolUvSources, ToolUvWorkspace, + Project, PyProjectToml, PyprojectTomlError, Source, Sources, ToolUvSources, ToolUvWorkspace, }; type WorkspaceMembers = Arc>; @@ -114,6 +114,10 @@ pub struct Workspace { /// /// This table is overridden by the project indexes. indexes: Vec, + /// Indexes defined as sources in transitive path dependencies of this workspace. + /// + /// Used for lockfile validation. + path_dependency_source_indexes: Vec, /// The `pyproject.toml` of the workspace root. pyproject_toml: PyProjectToml, } @@ -635,6 +639,11 @@ impl Workspace { &self.indexes } + /// Indexes defined as sources in transitive path dependencies of this workspace. + pub fn path_dependency_source_indexes(&self) -> &[Index] { + &self.path_dependency_source_indexes + } + /// The `pyproject.toml` of the workspace. pub fn pyproject_toml(&self) -> &PyProjectToml { &self.pyproject_toml @@ -742,11 +751,18 @@ impl Workspace { .and_then(|uv| uv.index) .unwrap_or_default(); + let path_dependency_source_indexes = collect_path_dependency_source_indexes( + &workspace_root, + &workspace_indexes, + &workspace_pyproject_toml, + ); + Ok(Workspace { install_path: workspace_root, packages: workspace_members, sources: workspace_sources, indexes: workspace_indexes, + path_dependency_source_indexes, pyproject_toml: workspace_pyproject_toml, }) } @@ -765,9 +781,7 @@ impl Workspace { // project. If it is the current project, it is added as such in the next step. if let Some(project) = &workspace_pyproject_toml.project { let pyproject_path = workspace_root.join("pyproject.toml"); - let contents = fs_err::read_to_string(&pyproject_path)?; - let pyproject_toml = PyProjectToml::from_string(contents) - .map_err(|err| WorkspaceError::Toml(pyproject_path.clone(), Box::new(err)))?; + let pyproject_toml = pyproject_toml_from_path(pyproject_path.clone())?; debug!( "Adding root workspace member: `{}`", @@ -1238,6 +1252,7 @@ impl ProjectWorkspace { // workspace sources. sources: BTreeMap::default(), indexes: Vec::default(), + path_dependency_source_indexes: Vec::default(), pyproject_toml: project_pyproject_toml.clone(), }, }); @@ -1557,6 +1572,99 @@ impl VirtualProject { } } +/// Parses a `pyproject.toml` file from a path. +fn pyproject_toml_from_path(pyproject_path: PathBuf) -> Result { + let contents = fs_err::read_to_string(&pyproject_path)?; + PyProjectToml::from_string(contents) + .map_err(|err| WorkspaceError::Toml(pyproject_path, Box::new(err))) +} + +/// Collects indexes provided as sources in (transitive) path dependencies that +/// have not already been defined in the workspace. +fn collect_path_dependency_source_indexes( + workspace_root: &Path, + workspace_indexes: &[Index], + workspace_pyproject_toml: &PyProjectToml, +) -> Vec { + let mut dependency_indexes = FxHashSet::default(); + let mut seen = FxHashSet::default(); + + // We will only add indexes if we have not already seen the URLs. + let known_urls: FxHashSet<_> = workspace_indexes.iter().map(Index::url).collect(); + + let mut pyprojects = std::collections::VecDeque::new(); + pyprojects.push_back(( + workspace_root.to_path_buf(), + workspace_pyproject_toml.clone(), + )); + + while let Some((base_path, pyproject)) = pyprojects.pop_front() { + if let Some(tool_uv_sources) = pyproject + .tool + .as_ref() + .and_then(|tool| tool.uv.as_ref()) + .and_then(|uv| uv.sources.as_ref()) + { + for sources in tool_uv_sources.inner().values() { + for source in sources.iter() { + if let Source::Path { path, .. } = source { + let dep_path = if path.as_ref().is_absolute() { + path.as_ref().to_path_buf() + } else { + base_path.join(path) + }; + + // Canonicalize path to compare symlinks and relative paths correctly + let Ok(canonical_path) = dep_path.canonicalize() else { + debug!( + "Failed to canonicalize path dependency path: {}", + dep_path.display() + ); + continue; + }; + + // Prevent infinite loops from circular dependencies + if !seen.insert(canonical_path.clone()) { + continue; + } + + let dep_pyproject_path = canonical_path.join("pyproject.toml"); + + match pyproject_toml_from_path(dep_pyproject_path.clone()) { + Ok(dep_pyproject) => { + if let Some(dep_indexes) = dep_pyproject + .tool + .as_ref() + .and_then(|tool| tool.uv.as_ref()) + .and_then(|uv| uv.index.as_ref()) + { + dependency_indexes.extend( + dep_indexes + .iter() + .filter(|idx| !known_urls.contains(idx.url())) + .cloned(), + ); + } + + pyprojects.push_back((canonical_path, dep_pyproject)); + } + Err(e) => { + debug!( + "Failed to read `pyproject.toml` in path dependency `{}`: {}", + dep_pyproject_path.display(), + e + ); + } + } + } + } + } + } + } + + dependency_indexes.into_iter().collect::>() +} + #[cfg(test)] #[cfg(unix)] // Avoid path escaping for the unit tests mod tests { @@ -1645,6 +1753,7 @@ mod tests { }, "sources": {}, "indexes": [], + "path_dependency_source_indexes": [], "pyproject_toml": { "project": { "name": "bird-feeder", @@ -1698,6 +1807,7 @@ mod tests { }, "sources": {}, "indexes": [], + "path_dependency_source_indexes": [], "pyproject_toml": { "project": { "name": "bird-feeder", @@ -1786,6 +1896,7 @@ mod tests { ] }, "indexes": [], + "path_dependency_source_indexes": [], "pyproject_toml": { "project": { "name": "albatross", @@ -1898,6 +2009,7 @@ mod tests { }, "sources": {}, "indexes": [], + "path_dependency_source_indexes": [], "pyproject_toml": { "project": null, "tool": { @@ -1964,6 +2076,7 @@ mod tests { }, "sources": {}, "indexes": [], + "path_dependency_source_indexes": [], "pyproject_toml": { "project": { "name": "albatross", @@ -2098,6 +2211,7 @@ mod tests { }, "sources": {}, "indexes": [], + "path_dependency_source_indexes": [], "pyproject_toml": { "project": { "name": "albatross", @@ -2204,6 +2318,7 @@ mod tests { }, "sources": {}, "indexes": [], + "path_dependency_source_indexes": [], "pyproject_toml": { "project": { "name": "albatross", @@ -2324,6 +2439,7 @@ mod tests { }, "sources": {}, "indexes": [], + "path_dependency_source_indexes": [], "pyproject_toml": { "project": { "name": "albatross", @@ -2418,6 +2534,7 @@ mod tests { }, "sources": {}, "indexes": [], + "path_dependency_source_indexes": [], "pyproject_toml": { "project": { "name": "albatross", diff --git a/crates/uv/src/commands/project/lock.rs b/crates/uv/src/commands/project/lock.rs index 89b3713ccff7a..35c69c33850e2 100644 --- a/crates/uv/src/commands/project/lock.rs +++ b/crates/uv/src/commands/project/lock.rs @@ -1,5 +1,6 @@ #![allow(clippy::single_match_else)] +use std::borrow::Cow; use std::collections::{BTreeMap, BTreeSet}; use std::fmt::Write; use std::path::Path; @@ -694,6 +695,7 @@ async fn do_lock( interpreter, &requires_python, index_locations, + target.path_dependency_source_indexes(), upgrade, &options, &hasher, @@ -907,6 +909,7 @@ impl ValidatedLock { interpreter: &Interpreter, requires_python: &RequiresPython, index_locations: &IndexLocations, + path_dependency_source_indexes: Option<&[Index]>, upgrade: &Upgrade, options: &Options, hasher: &HashStrategy, @@ -1066,10 +1069,21 @@ impl ValidatedLock { // However, if _no_ indexes were provided, we assume that the user wants to reuse the existing // distributions, even though a failure to reuse the lockfile will result in re-resolving // against PyPI by default. - let indexes = if index_locations.is_none() { + let validation_indexes = if index_locations.is_none() { None } else { - Some(index_locations) + // If indexes were defined as sources in path dependencies, add them to the + // index locations to use for validation. + path_dependency_source_indexes + .filter(|source_indexes| !source_indexes.is_empty()) + .map(|source_indexes| { + Cow::Owned(index_locations.clone().combine( + source_indexes.to_vec(), + Vec::new(), + false, + )) + }) + .or(Some(Cow::Borrowed(index_locations))) }; // Determine whether the lockfile satisfies the workspace requirements. @@ -1084,7 +1098,7 @@ impl ValidatedLock { build_constraints, dependency_groups, dependency_metadata, - indexes, + validation_indexes, interpreter.tags()?, hasher, index, diff --git a/crates/uv/src/commands/project/lock_target.rs b/crates/uv/src/commands/project/lock_target.rs index cb45aa8ec6810..31e4d68833f3d 100644 --- a/crates/uv/src/commands/project/lock_target.rs +++ b/crates/uv/src/commands/project/lock_target.rs @@ -215,6 +215,14 @@ impl<'lock> LockTarget<'lock> { } } + /// If this is a workspace, returns path dependency source indexes, otherwise return [`None`]. + pub(crate) fn path_dependency_source_indexes(&self) -> Option<&[Index]> { + match self { + Self::Workspace(workspace) => Some(workspace.path_dependency_source_indexes()), + Self::Script(..) => None, + } + } + /// Return the `Requires-Python` bound for the [`LockTarget`]. #[allow(clippy::result_large_err)] pub(crate) fn requires_python(self) -> Result, ProjectError> { diff --git a/crates/uv/tests/it/lock.rs b/crates/uv/tests/it/lock.rs index 4387d348afe8e..e0e04c1183b7f 100644 --- a/crates/uv/tests/it/lock.rs +++ b/crates/uv/tests/it/lock.rs @@ -27557,3 +27557,397 @@ fn lock_conflict_for_disjoint_platform() -> Result<()> { Ok(()) } + +/// Test that lockfile validation includes explicit indexes from path dependencies. +/// `` +#[test] +fn lock_path_dependency_explicit_index() -> Result<()> { + let context = TestContext::new("3.12"); + + // Create the path dependency with explicit index + let pkg_a = context.temp_dir.child("pkg_a"); + fs_err::create_dir_all(&pkg_a)?; + + let pyproject_toml = pkg_a.child("pyproject.toml"); + pyproject_toml.write_str( + r#" + [project] + name = "pkg-a" + version = "0.1.0" + requires-python = ">=3.12" + dependencies = ["iniconfig"] + + [tool.uv.sources] + iniconfig = { index = "inner-index" } + + [[tool.uv.index]] + name = "inner-index" + url = "https://pypi-proxy.fly.dev/simple" + explicit = true + "#, + )?; + + // Create a project that depends on pkg_a + let pkg_b = context.temp_dir.child("pkg_b"); + fs_err::create_dir_all(&pkg_b)?; + + let pyproject_toml = pkg_b.child("pyproject.toml"); + pyproject_toml.write_str( + r#" + [project] + name = "pkg-b" + version = "0.1.0" + requires-python = ">=3.12" + dependencies = ["pkg-a"] + + [tool.uv.sources] + pkg-a = { path = "../pkg_a/", editable = true } + black = { index = "outer-index" } + + [[tool.uv.index]] + name = "outer-index" + url = "https://outer-index.com/simple" + explicit = true + "#, + )?; + + uv_snapshot!(context.filters(), context.lock().current_dir(&pkg_b), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Using CPython 3.12.[X] interpreter at: [PYTHON-3.12] + Resolved 3 packages in [TIME] + "); + + uv_snapshot!(context.filters(), context.lock().arg("--check").current_dir(&pkg_b), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Using CPython 3.12.[X] interpreter at: [PYTHON-3.12] + Resolved 3 packages in [TIME] + "); + + Ok(()) +} + +/// Test that lockfile validation works correctly when path dependency has +/// both explicit and non-explicit indexes. +#[test] +fn lock_path_dependency_mixed_indexes() -> Result<()> { + let context = TestContext::new("3.12"); + + // Create the path dependency with both explicit and non-explicit indexes + let pkg_a = context.temp_dir.child("pkg_a"); + fs_err::create_dir_all(&pkg_a)?; + + let pyproject_toml = pkg_a.child("pyproject.toml"); + pyproject_toml.write_str( + r#" + [project] + name = "pkg-a" + version = "0.1.0" + requires-python = ">=3.12" + dependencies = ["iniconfig", "anyio"] + + [tool.uv.sources] + iniconfig = { index = "explicit-index" } + anyio = { index = "non-explicit-index" } + + [[tool.uv.index]] + name = "non-explicit-index" + url = "https://pypi-proxy.fly.dev/simple" + + [[tool.uv.index]] + name = "explicit-index" + url = "https://pypi.org/simple" + explicit = true + "#, + )?; + + // Create a project that depends on pkg_a + let pkg_b = context.temp_dir.child("pkg_b"); + fs_err::create_dir_all(&pkg_b)?; + + let pyproject_toml = pkg_b.child("pyproject.toml"); + pyproject_toml.write_str( + r#" + [project] + name = "pkg-b" + version = "0.1.0" + requires-python = ">=3.12" + dependencies = ["pkg-a"] + + [tool.uv.sources] + pkg-a = { path = "../pkg_a/", editable = true } + black = { index = "outer-index" } + + [[tool.uv.index]] + name = "outer-index" + url = "https://outer-index.com/simple" + explicit = true + "#, + )?; + + uv_snapshot!(context.filters(), context.lock().current_dir(&pkg_b), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Using CPython 3.12.[X] interpreter at: [PYTHON-3.12] + Resolved 6 packages in [TIME] + "); + + uv_snapshot!(context.filters(), context.lock().arg("--check").current_dir(&pkg_b), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Using CPython 3.12.[X] interpreter at: [PYTHON-3.12] + Resolved 6 packages in [TIME] + "); + + Ok(()) +} + +/// Test that path dependencies without an index don't affect validation. +#[test] +fn lock_path_dependency_no_index() -> Result<()> { + let context = TestContext::new("3.12"); + + // Create the path dependency without explicit indexes + let pkg_a = context.temp_dir.child("pkg_a"); + fs_err::create_dir_all(&pkg_a)?; + + let pyproject_toml = pkg_a.child("pyproject.toml"); + pyproject_toml.write_str( + r#" + [project] + name = "pkg-a" + version = "0.1.0" + requires-python = ">=3.12" + dependencies = ["requests"] + "#, + )?; + + // Create a project that depends on pkg_a + let pkg_b = context.temp_dir.child("pkg_b"); + fs_err::create_dir_all(&pkg_b)?; + + let pyproject_toml = pkg_b.child("pyproject.toml"); + pyproject_toml.write_str( + r#" + [project] + name = "pkg-b" + version = "0.1.0" + requires-python = ">=3.12" + dependencies = ["pkg-a"] + + [tool.uv.sources] + pkg-a = { path = "../pkg_a/", editable = true } + "#, + )?; + + uv_snapshot!(context.filters(), context.lock().current_dir(&pkg_b), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Using CPython 3.12.[X] interpreter at: [PYTHON-3.12] + Resolved 7 packages in [TIME] + "); + + uv_snapshot!(context.filters(), context.lock().arg("--check").current_dir(&pkg_b), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Using CPython 3.12.[X] interpreter at: [PYTHON-3.12] + Resolved 7 packages in [TIME] + "); + + Ok(()) +} + +/// Test that a nested path dependency with an explicit index validates correctly. +#[test] +fn lock_nested_path_dependency_explicit_index() -> Result<()> { + let context = TestContext::new("3.12"); + + // Create the inner dependency with explicit index + let pkg_a = context.temp_dir.child("pkg_a"); + fs_err::create_dir_all(&pkg_a)?; + + let pyproject_toml = pkg_a.child("pyproject.toml"); + pyproject_toml.write_str( + r#" + [project] + name = "pkg-a" + version = "0.1.0" + requires-python = ">=3.12" + dependencies = ["iniconfig"] + + [tool.uv.sources] + iniconfig = { index = "inner-index" } + + [[tool.uv.index]] + name = "inner-index" + url = "https://pypi-proxy.fly.dev/simple" + explicit = true + "#, + )?; + + // Create intermediate dependency that depends on pkg_a + let pkg_b = context.temp_dir.child("pkg_b"); + fs_err::create_dir_all(&pkg_b)?; + + let pyproject_toml = pkg_b.child("pyproject.toml"); + pyproject_toml.write_str( + r#" + [project] + name = "pkg-b" + version = "0.1.0" + requires-python = ">=3.12" + dependencies = ["pkg-a"] + + [tool.uv.sources] + pkg-a = { path = "../pkg_a/", editable = true } + "#, + )?; + + // Create a project that depends on intermediate dependency + let pkg_c = context.temp_dir.child("pkg_c"); + fs_err::create_dir_all(&pkg_c)?; + + let pyproject_toml = pkg_c.child("pyproject.toml"); + pyproject_toml.write_str( + r#" + [project] + name = "pkg-c" + version = "0.1.0" + requires-python = ">=3.12" + dependencies = ["pkg-b"] + + [tool.uv.sources] + pkg-b = { path = "../pkg_b/", editable = true } + black = { index = "outer-index" } + + [[tool.uv.index]] + name = "outer-index" + url = "https://outer-index.com/simple" + explicit = true + "#, + )?; + + uv_snapshot!(context.filters(), context.lock().current_dir(&pkg_c), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Using CPython 3.12.[X] interpreter at: [PYTHON-3.12] + Resolved 4 packages in [TIME] + "); + + uv_snapshot!(context.filters(), context.lock().arg("--check").current_dir(&pkg_c), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Using CPython 3.12.[X] interpreter at: [PYTHON-3.12] + Resolved 4 packages in [TIME] + "); + + Ok(()) +} + +/// Test that validating circular path dependency indexes doesn't cause an infinite loop. +// TODO(john): Validation doesn't hang but the `uv lock --check` step fails to validate +// in the circular path dependency case (same behavior as prior to the fix for path +// dependency index lock validation). +#[test] +fn lock_circular_path_dependency_explicit_index() -> Result<()> { + let context = TestContext::new("3.12"); + + // Create pkg_a (with explicit index) that depends on pkg_b + let pkg_a = context.temp_dir.child("pkg_a"); + fs_err::create_dir_all(&pkg_a)?; + + let pyproject_toml = pkg_a.child("pyproject.toml"); + pyproject_toml.write_str( + r#" + [project] + name = "pkg-a" + version = "0.1.0" + requires-python = ">=3.10" + dependencies = ["pkg-b", "iniconfig"] + + [tool.uv.sources] + pkg-b = { path = "../pkg_b/", editable = true } + iniconfig = { index = "index-a" } + + [[tool.uv.index]] + name = "index-a" + url = "https://pypi-proxy.fly.dev/simple" + explicit = true + "#, + )?; + + // Create pkg_b that depends on pkg_a. This is a circular dependency. + let pkg_b = context.temp_dir.child("pkg_b"); + fs_err::create_dir_all(&pkg_b)?; + + let pyproject_toml = pkg_b.child("pyproject.toml"); + pyproject_toml.write_str( + r#" + [project] + name = "pkg-b" + version = "0.1.0" + requires-python = ">=3.10" + dependencies = ["pkg-a", "anyio"] + + [tool.uv.sources] + pkg-a = { path = "../pkg_a/", editable = true } + anyio = { index = "index-b" } + + [[tool.uv.index]] + name = "index-b" + url = "https://pypi.org/simple" + explicit = true + default = true + "#, + )?; + + // This should not hang or crash due to the circular dependency + uv_snapshot!(context.filters(), context.lock().current_dir(&pkg_a), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Using CPython 3.12.[X] interpreter at: [PYTHON-3.12] + Resolved 8 packages in [TIME] + "); + + // TODO(john): This currently fails due to circular dependency validation issues + uv_snapshot!(context.filters(), context.lock().arg("--check").current_dir(&pkg_a), @r" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + Using CPython 3.12.[X] interpreter at: [PYTHON-3.12] + Resolved 8 packages in [TIME] + error: The lockfile at `uv.lock` needs to be updated, but `--locked` was provided. To update the lockfile, run `uv lock`. + "); + + Ok(()) +} From 3ad4ba7935b027c08b4f241f8499210357a06663 Mon Sep 17 00:00:00 2001 From: John Mumm Date: Tue, 17 Jun 2025 15:27:20 +0200 Subject: [PATCH 2/4] .. --- crates/uv-resolver/src/lock/mod.rs | 2 ++ crates/uv/tests/it/lock.rs | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/uv-resolver/src/lock/mod.rs b/crates/uv-resolver/src/lock/mod.rs index d5f9485ba7361..ba9150b6fe73e 100644 --- a/crates/uv-resolver/src/lock/mod.rs +++ b/crates/uv-resolver/src/lock/mod.rs @@ -1445,6 +1445,8 @@ impl Lock { queue.push_back(root); } + // Unlike path dependencies, Git dependencies are immutable. Their sources cannot change + // without the hashes changing, so we know their indexes are still present. while let Some(package) = queue.pop_front() { // If the lockfile references an index that was not provided, we can't validate it. if let Source::Registry(index) = &package.id.source { diff --git a/crates/uv/tests/it/lock.rs b/crates/uv/tests/it/lock.rs index e0e04c1183b7f..62575de3129d6 100644 --- a/crates/uv/tests/it/lock.rs +++ b/crates/uv/tests/it/lock.rs @@ -27559,7 +27559,7 @@ fn lock_conflict_for_disjoint_platform() -> Result<()> { } /// Test that lockfile validation includes explicit indexes from path dependencies. -/// `` +/// #[test] fn lock_path_dependency_explicit_index() -> Result<()> { let context = TestContext::new("3.12"); From 9e5c16d8332c750aa734041012b337a5687bb504 Mon Sep 17 00:00:00 2001 From: John Mumm Date: Thu, 19 Jun 2025 13:40:53 +0200 Subject: [PATCH 3/4] Only collect path dependency source indexes during lockfile validation --- crates/uv-workspace/src/workspace.rs | 191 ++++++++---------- crates/uv/src/commands/project/lock.rs | 29 +-- crates/uv/src/commands/project/lock_target.rs | 8 - 3 files changed, 95 insertions(+), 133 deletions(-) diff --git a/crates/uv-workspace/src/workspace.rs b/crates/uv-workspace/src/workspace.rs index 0ec8ad5317ed0..967f01e96ad0f 100644 --- a/crates/uv-workspace/src/workspace.rs +++ b/crates/uv-workspace/src/workspace.rs @@ -114,10 +114,6 @@ pub struct Workspace { /// /// This table is overridden by the project indexes. indexes: Vec, - /// Indexes defined as sources in transitive path dependencies of this workspace. - /// - /// Used for lockfile validation. - path_dependency_source_indexes: Vec, /// The `pyproject.toml` of the workspace root. pyproject_toml: PyProjectToml, } @@ -639,11 +635,6 @@ impl Workspace { &self.indexes } - /// Indexes defined as sources in transitive path dependencies of this workspace. - pub fn path_dependency_source_indexes(&self) -> &[Index] { - &self.path_dependency_source_indexes - } - /// The `pyproject.toml` of the workspace. pub fn pyproject_toml(&self) -> &PyProjectToml { &self.pyproject_toml @@ -751,18 +742,11 @@ impl Workspace { .and_then(|uv| uv.index) .unwrap_or_default(); - let path_dependency_source_indexes = collect_path_dependency_source_indexes( - &workspace_root, - &workspace_indexes, - &workspace_pyproject_toml, - ); - Ok(Workspace { install_path: workspace_root, packages: workspace_members, sources: workspace_sources, indexes: workspace_indexes, - path_dependency_source_indexes, pyproject_toml: workspace_pyproject_toml, }) } @@ -945,6 +929,85 @@ impl Workspace { } Ok(workspace_members) } + + /// Collects indexes provided as sources in (transitive) path dependencies that + /// have not already been defined in the workspace. + pub fn collect_path_dependency_source_indexes(&self) -> Vec { + let mut dependency_indexes = FxHashSet::default(); + let mut seen = FxHashSet::default(); + + // We will only add indexes if we have not already seen the URLs. + let known_urls: FxHashSet<_> = self.indexes.iter().map(Index::url).collect(); + + let mut pyprojects = std::collections::VecDeque::new(); + pyprojects.push_back((self.install_path.clone(), self.pyproject_toml.clone())); + + while let Some((base_path, pyproject)) = pyprojects.pop_front() { + if let Some(tool_uv_sources) = pyproject + .tool + .as_ref() + .and_then(|tool| tool.uv.as_ref()) + .and_then(|uv| uv.sources.as_ref()) + { + for sources in tool_uv_sources.inner().values() { + for source in sources.iter() { + if let Source::Path { path, .. } = source { + let dep_path = if path.as_ref().is_absolute() { + path.as_ref().to_path_buf() + } else { + base_path.join(path) + }; + + // Canonicalize path to compare symlinks and relative paths correctly + let Ok(canonical_path) = dep_path.canonicalize() else { + debug!( + "Failed to canonicalize path dependency path: {}", + dep_path.display() + ); + continue; + }; + + // Prevent infinite loops from circular dependencies + if !seen.insert(canonical_path.clone()) { + continue; + } + + let dep_pyproject_path = canonical_path.join("pyproject.toml"); + + match pyproject_toml_from_path(dep_pyproject_path.clone()) { + Ok(dep_pyproject) => { + if let Some(dep_indexes) = dep_pyproject + .tool + .as_ref() + .and_then(|tool| tool.uv.as_ref()) + .and_then(|uv| uv.index.as_ref()) + { + dependency_indexes.extend( + dep_indexes + .iter() + .filter(|idx| !known_urls.contains(idx.url())) + .cloned(), + ); + } + + pyprojects.push_back((canonical_path, dep_pyproject)); + } + Err(e) => { + debug!( + "Failed to read `pyproject.toml` in path dependency `{}`: {}", + dep_pyproject_path.display(), + e + ); + } + } + } + } + } + } + } + + dependency_indexes.into_iter().collect::>() + } } /// A project in a workspace. @@ -1252,7 +1315,6 @@ impl ProjectWorkspace { // workspace sources. sources: BTreeMap::default(), indexes: Vec::default(), - path_dependency_source_indexes: Vec::default(), pyproject_toml: project_pyproject_toml.clone(), }, }); @@ -1579,92 +1641,6 @@ fn pyproject_toml_from_path(pyproject_path: PathBuf) -> Result Vec { - let mut dependency_indexes = FxHashSet::default(); - let mut seen = FxHashSet::default(); - - // We will only add indexes if we have not already seen the URLs. - let known_urls: FxHashSet<_> = workspace_indexes.iter().map(Index::url).collect(); - - let mut pyprojects = std::collections::VecDeque::new(); - pyprojects.push_back(( - workspace_root.to_path_buf(), - workspace_pyproject_toml.clone(), - )); - - while let Some((base_path, pyproject)) = pyprojects.pop_front() { - if let Some(tool_uv_sources) = pyproject - .tool - .as_ref() - .and_then(|tool| tool.uv.as_ref()) - .and_then(|uv| uv.sources.as_ref()) - { - for sources in tool_uv_sources.inner().values() { - for source in sources.iter() { - if let Source::Path { path, .. } = source { - let dep_path = if path.as_ref().is_absolute() { - path.as_ref().to_path_buf() - } else { - base_path.join(path) - }; - - // Canonicalize path to compare symlinks and relative paths correctly - let Ok(canonical_path) = dep_path.canonicalize() else { - debug!( - "Failed to canonicalize path dependency path: {}", - dep_path.display() - ); - continue; - }; - - // Prevent infinite loops from circular dependencies - if !seen.insert(canonical_path.clone()) { - continue; - } - - let dep_pyproject_path = canonical_path.join("pyproject.toml"); - - match pyproject_toml_from_path(dep_pyproject_path.clone()) { - Ok(dep_pyproject) => { - if let Some(dep_indexes) = dep_pyproject - .tool - .as_ref() - .and_then(|tool| tool.uv.as_ref()) - .and_then(|uv| uv.index.as_ref()) - { - dependency_indexes.extend( - dep_indexes - .iter() - .filter(|idx| !known_urls.contains(idx.url())) - .cloned(), - ); - } - - pyprojects.push_back((canonical_path, dep_pyproject)); - } - Err(e) => { - debug!( - "Failed to read `pyproject.toml` in path dependency `{}`: {}", - dep_pyproject_path.display(), - e - ); - } - } - } - } - } - } - } - - dependency_indexes.into_iter().collect::>() -} - #[cfg(test)] #[cfg(unix)] // Avoid path escaping for the unit tests mod tests { @@ -1753,7 +1729,6 @@ mod tests { }, "sources": {}, "indexes": [], - "path_dependency_source_indexes": [], "pyproject_toml": { "project": { "name": "bird-feeder", @@ -1807,7 +1782,6 @@ mod tests { }, "sources": {}, "indexes": [], - "path_dependency_source_indexes": [], "pyproject_toml": { "project": { "name": "bird-feeder", @@ -1896,7 +1870,6 @@ mod tests { ] }, "indexes": [], - "path_dependency_source_indexes": [], "pyproject_toml": { "project": { "name": "albatross", @@ -2009,7 +1982,6 @@ mod tests { }, "sources": {}, "indexes": [], - "path_dependency_source_indexes": [], "pyproject_toml": { "project": null, "tool": { @@ -2076,7 +2048,6 @@ mod tests { }, "sources": {}, "indexes": [], - "path_dependency_source_indexes": [], "pyproject_toml": { "project": { "name": "albatross", @@ -2211,7 +2182,6 @@ mod tests { }, "sources": {}, "indexes": [], - "path_dependency_source_indexes": [], "pyproject_toml": { "project": { "name": "albatross", @@ -2318,7 +2288,6 @@ mod tests { }, "sources": {}, "indexes": [], - "path_dependency_source_indexes": [], "pyproject_toml": { "project": { "name": "albatross", @@ -2439,7 +2408,6 @@ mod tests { }, "sources": {}, "indexes": [], - "path_dependency_source_indexes": [], "pyproject_toml": { "project": { "name": "albatross", @@ -2534,7 +2502,6 @@ mod tests { }, "sources": {}, "indexes": [], - "path_dependency_source_indexes": [], "pyproject_toml": { "project": { "name": "albatross", diff --git a/crates/uv/src/commands/project/lock.rs b/crates/uv/src/commands/project/lock.rs index 35c69c33850e2..ae94d5ca42bb5 100644 --- a/crates/uv/src/commands/project/lock.rs +++ b/crates/uv/src/commands/project/lock.rs @@ -680,7 +680,7 @@ async fn do_lock( let existing_lock = if let Some(existing_lock) = existing_lock { match ValidatedLock::validate( existing_lock, - target.install_path(), + target, packages, &members, &requirements, @@ -695,7 +695,6 @@ async fn do_lock( interpreter, &requires_python, index_locations, - target.path_dependency_source_indexes(), upgrade, &options, &hasher, @@ -894,7 +893,7 @@ impl ValidatedLock { /// Validate a [`Lock`] against the workspace requirements. async fn validate( lock: Lock, - install_path: &Path, + target: LockTarget<'_>, packages: &BTreeMap, members: &[PackageName], requirements: &[Requirement], @@ -909,7 +908,6 @@ impl ValidatedLock { interpreter: &Interpreter, requires_python: &RequiresPython, index_locations: &IndexLocations, - path_dependency_source_indexes: Option<&[Index]>, upgrade: &Upgrade, options: &Options, hasher: &HashStrategy, @@ -1074,22 +1072,27 @@ impl ValidatedLock { } else { // If indexes were defined as sources in path dependencies, add them to the // index locations to use for validation. - path_dependency_source_indexes - .filter(|source_indexes| !source_indexes.is_empty()) - .map(|source_indexes| { - Cow::Owned(index_locations.clone().combine( - source_indexes.to_vec(), + if let LockTarget::Workspace(workspace) = target { + let path_dependency_source_indexes = + workspace.collect_path_dependency_source_indexes(); + if path_dependency_source_indexes.is_empty() { + Some(Cow::Borrowed(index_locations)) + } else { + Some(Cow::Owned(index_locations.clone().combine( + path_dependency_source_indexes, Vec::new(), false, - )) - }) - .or(Some(Cow::Borrowed(index_locations))) + ))) + } + } else { + Some(Cow::Borrowed(index_locations)) + } }; // Determine whether the lockfile satisfies the workspace requirements. match lock .satisfies( - install_path, + target.install_path(), packages, members, requirements, diff --git a/crates/uv/src/commands/project/lock_target.rs b/crates/uv/src/commands/project/lock_target.rs index 31e4d68833f3d..cb45aa8ec6810 100644 --- a/crates/uv/src/commands/project/lock_target.rs +++ b/crates/uv/src/commands/project/lock_target.rs @@ -215,14 +215,6 @@ impl<'lock> LockTarget<'lock> { } } - /// If this is a workspace, returns path dependency source indexes, otherwise return [`None`]. - pub(crate) fn path_dependency_source_indexes(&self) -> Option<&[Index]> { - match self { - Self::Workspace(workspace) => Some(workspace.path_dependency_source_indexes()), - Self::Script(..) => None, - } - } - /// Return the `Requires-Python` bound for the [`LockTarget`]. #[allow(clippy::result_large_err)] pub(crate) fn requires_python(self) -> Result, ProjectError> { From ec0b75bcfcb6f476be7e5e61dd4562c722ef14ab Mon Sep 17 00:00:00 2001 From: John Mumm Date: Thu, 24 Jul 2025 14:13:00 +0200 Subject: [PATCH 4/4] Collect path dependency indexes separately --- crates/uv-resolver/src/lock/mod.rs | 14 ++-- crates/uv-workspace/src/workspace.rs | 19 +++-- crates/uv/src/commands/project/lock.rs | 44 ++++++------ crates/uv/tests/it/lock.rs | 98 ++++++++++++++++++++++++++ 4 files changed, 139 insertions(+), 36 deletions(-) diff --git a/crates/uv-resolver/src/lock/mod.rs b/crates/uv-resolver/src/lock/mod.rs index ba9150b6fe73e..3291041494be8 100644 --- a/crates/uv-resolver/src/lock/mod.rs +++ b/crates/uv-resolver/src/lock/mod.rs @@ -1233,7 +1233,8 @@ impl Lock { build_constraints: &[Requirement], dependency_groups: &BTreeMap>, dependency_metadata: &DependencyMetadata, - indexes: Option>, + indexes: Option<&IndexLocations>, + path_dependency_indexes: &BTreeSet, tags: &Tags, hasher: &HashStrategy, index: &InMemoryIndex, @@ -1399,7 +1400,7 @@ impl Lock { } // Collect the set of available indexes (both `--index-url` and `--find-links` entries). - let remotes = indexes.as_ref().map(|locations| { + let remotes = indexes.map(|locations| { locations .allowed_indexes() .into_iter() @@ -1412,7 +1413,7 @@ impl Lock { .collect::>() }); - let locals = indexes.as_ref().map(|locations| { + let locals = indexes.map(|locations| { locations .allowed_indexes() .into_iter() @@ -1452,10 +1453,9 @@ impl Lock { if let Source::Registry(index) = &package.id.source { match index { RegistrySource::Url(url) => { - if remotes - .as_ref() - .is_some_and(|remotes| !remotes.contains(url)) - { + if remotes.as_ref().is_some_and(|remotes| { + !remotes.contains(url) && !path_dependency_indexes.contains(url) + }) { let name = &package.id.name; let version = &package .id diff --git a/crates/uv-workspace/src/workspace.rs b/crates/uv-workspace/src/workspace.rs index 967f01e96ad0f..b6ae9ee0ae695 100644 --- a/crates/uv-workspace/src/workspace.rs +++ b/crates/uv-workspace/src/workspace.rs @@ -1,6 +1,7 @@ //! Resolve the current [`ProjectWorkspace`] or [`Workspace`]. -use std::collections::{BTreeMap, BTreeSet}; +use std::borrow::Cow; +use std::collections::{BTreeMap, BTreeSet, VecDeque}; use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex}; @@ -939,10 +940,13 @@ impl Workspace { // We will only add indexes if we have not already seen the URLs. let known_urls: FxHashSet<_> = self.indexes.iter().map(Index::url).collect(); - let mut pyprojects = std::collections::VecDeque::new(); - pyprojects.push_back((self.install_path.clone(), self.pyproject_toml.clone())); + let mut pyproject_queue = VecDeque::new(); + for package in self.packages.values() { + pyproject_queue + .push_back((package.root.clone(), Cow::Borrowed(&package.pyproject_toml))); + } - while let Some((base_path, pyproject)) = pyprojects.pop_front() { + while let Some((base_path, pyproject)) = pyproject_queue.pop_front() { if let Some(tool_uv_sources) = pyproject .tool .as_ref() @@ -975,8 +979,8 @@ impl Workspace { let dep_pyproject_path = canonical_path.join("pyproject.toml"); match pyproject_toml_from_path(dep_pyproject_path.clone()) { - Ok(dep_pyproject) => { - if let Some(dep_indexes) = dep_pyproject + Ok(pyproject_toml) => { + if let Some(dep_indexes) = pyproject_toml .tool .as_ref() .and_then(|tool| tool.uv.as_ref()) @@ -990,7 +994,8 @@ impl Workspace { ); } - pyprojects.push_back((canonical_path, dep_pyproject)); + pyproject_queue + .push_back((canonical_path, Cow::Owned(pyproject_toml))); } Err(e) => { debug!( diff --git a/crates/uv/src/commands/project/lock.rs b/crates/uv/src/commands/project/lock.rs index ae94d5ca42bb5..07116c526e03c 100644 --- a/crates/uv/src/commands/project/lock.rs +++ b/crates/uv/src/commands/project/lock.rs @@ -1,6 +1,5 @@ #![allow(clippy::single_match_else)] -use std::borrow::Cow; use std::collections::{BTreeMap, BTreeSet}; use std::fmt::Write; use std::path::Path; @@ -18,8 +17,8 @@ use uv_configuration::{ use uv_dispatch::BuildDispatch; use uv_distribution::DistributionDatabase; use uv_distribution_types::{ - DependencyMetadata, HashGeneration, Index, IndexLocations, NameRequirementSpecification, - Requirement, UnresolvedRequirementSpecification, + DependencyMetadata, HashGeneration, Index, IndexLocations, IndexUrl, + NameRequirementSpecification, Requirement, UnresolvedRequirementSpecification, UrlString, }; use uv_git::ResolvedRepositoryReference; use uv_normalize::{GroupName, PackageName}; @@ -1067,26 +1066,26 @@ impl ValidatedLock { // However, if _no_ indexes were provided, we assume that the user wants to reuse the existing // distributions, even though a failure to reuse the lockfile will result in re-resolving // against PyPI by default. - let validation_indexes = if index_locations.is_none() { + let indexes = if index_locations.is_none() { None } else { - // If indexes were defined as sources in path dependencies, add them to the - // index locations to use for validation. - if let LockTarget::Workspace(workspace) = target { - let path_dependency_source_indexes = - workspace.collect_path_dependency_source_indexes(); - if path_dependency_source_indexes.is_empty() { - Some(Cow::Borrowed(index_locations)) - } else { - Some(Cow::Owned(index_locations.clone().combine( - path_dependency_source_indexes, - Vec::new(), - false, - ))) - } - } else { - Some(Cow::Borrowed(index_locations)) - } + Some(index_locations) + }; + + // Collect indexes specified in path dependencies + let path_dependency_indexes = if let LockTarget::Workspace(workspace) = target { + workspace + .collect_path_dependency_source_indexes() + .into_iter() + .filter_map(|index| match index.url() { + IndexUrl::Pypi(_) | IndexUrl::Url(_) => { + Some(UrlString::from(index.url().without_credentials().as_ref())) + } + IndexUrl::Path(_) => None, + }) + .collect::>() + } else { + BTreeSet::default() }; // Determine whether the lockfile satisfies the workspace requirements. @@ -1101,7 +1100,8 @@ impl ValidatedLock { build_constraints, dependency_groups, dependency_metadata, - validation_indexes, + indexes, + &path_dependency_indexes, interpreter.tags()?, hasher, index, diff --git a/crates/uv/tests/it/lock.rs b/crates/uv/tests/it/lock.rs index 62575de3129d6..3ecea9f1997f7 100644 --- a/crates/uv/tests/it/lock.rs +++ b/crates/uv/tests/it/lock.rs @@ -27634,6 +27634,104 @@ fn lock_path_dependency_explicit_index() -> Result<()> { Ok(()) } +/// Test that lockfile validation includes explicit indexes from path dependencies +/// defined in a non-root workspace member. +#[test] +fn lock_path_dependency_explicit_index_workspace_member() -> Result<()> { + let context = TestContext::new("3.12"); + + // Create the path dependency with explicit index + let pkg_a = context.temp_dir.child("pkg_a"); + fs_err::create_dir_all(&pkg_a)?; + + let pyproject_toml = pkg_a.child("pyproject.toml"); + pyproject_toml.write_str( + r#" + [project] + name = "pkg-a" + version = "0.1.0" + requires-python = ">=3.12" + dependencies = ["iniconfig"] + + [tool.uv.sources] + iniconfig = { index = "inner-index" } + + [[tool.uv.index]] + name = "inner-index" + url = "https://pypi-proxy.fly.dev/simple" + explicit = true + "#, + )?; + + // Create a project that depends on pkg_a + let member = context.temp_dir.child("member"); + fs_err::create_dir_all(&member)?; + + let pyproject_toml = member.child("pyproject.toml"); + pyproject_toml.write_str( + r#" + [project] + name = "member" + version = "0.1.0" + requires-python = ">=3.12" + dependencies = ["pkg-a"] + + [tool.uv.sources] + pkg-a = { path = "../pkg_a/", editable = true } + black = { index = "middle-index" } + + [[tool.uv.index]] + name = "middle-index" + url = "https://middle-index.com/simple" + explicit = true + "#, + )?; + + // Create a root with workspace member + let pyproject_toml = context.temp_dir.child("pyproject.toml"); + pyproject_toml.write_str( + r#" + [project] + name = "root-project" + version = "0.1.0" + requires-python = ">=3.12" + dependencies = ["member"] + + [tool.uv.workspace] + members = ["member"] + + [tool.uv.sources] + member = { workspace = true } + anyio = { index = "outer-index" } + + [[tool.uv.index]] + name = "outer-index" + url = "https://outer-index.com/simple" + explicit = true + "#, + )?; + + uv_snapshot!(context.filters(), context.lock(), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 4 packages in [TIME] + "); + + uv_snapshot!(context.filters(), context.lock().arg("--check"), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 4 packages in [TIME] + "); + + Ok(()) +} + /// Test that lockfile validation works correctly when path dependency has /// both explicit and non-explicit indexes. #[test]