Skip to content

Commit

Permalink
Avoid trailing slash when deserializing from lockfile (#9848)
Browse files Browse the repository at this point in the history
## Summary

Very tricky problem whereby `workspace_root.join(path)` returns the
workspace root with a trailing slash if `path` is empty... This caused
us to accidentally _include_ excluded members during workspace
discovery, since (e.g.) `packages/seeds` doesn't match
`packages/seeds/`.

Closes
#9832 (comment).
  • Loading branch information
charliermarsh authored Dec 12, 2024
1 parent a13e3f5 commit f80ddf1
Show file tree
Hide file tree
Showing 2 changed files with 135 additions and 12 deletions.
42 changes: 30 additions & 12 deletions crates/uv-resolver/src/lock/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1682,10 +1682,11 @@ impl Package {
Source::Path(path) => {
let filename: WheelFilename =
self.wheels[best_wheel_index].filename.clone();
let install_path = absolute_path(workspace_root, path)?;
let path_dist = PathBuiltDist {
filename,
url: verbatim_url(workspace_root.join(path), &self.id)?,
install_path: workspace_root.join(path),
url: verbatim_url(&install_path, &self.id)?,
install_path: absolute_path(workspace_root, path)?,
};
let built_dist = BuiltDist::Path(path_dist);
Ok(Dist::Built(built_dist))
Expand Down Expand Up @@ -1780,40 +1781,44 @@ impl Package {
let DistExtension::Source(ext) = DistExtension::from_path(path)? else {
return Ok(None);
};
let install_path = absolute_path(workspace_root, path)?;
let path_dist = PathSourceDist {
name: self.id.name.clone(),
version: Some(self.id.version.clone()),
url: verbatim_url(workspace_root.join(path), &self.id)?,
install_path: workspace_root.join(path),
url: verbatim_url(&install_path, &self.id)?,
install_path,
ext,
};
uv_distribution_types::SourceDist::Path(path_dist)
}
Source::Directory(path) => {
let install_path = absolute_path(workspace_root, path)?;
let dir_dist = DirectorySourceDist {
name: self.id.name.clone(),
url: verbatim_url(workspace_root.join(path), &self.id)?,
install_path: workspace_root.join(path),
url: verbatim_url(&install_path, &self.id)?,
install_path,
editable: false,
r#virtual: false,
};
uv_distribution_types::SourceDist::Directory(dir_dist)
}
Source::Editable(path) => {
let install_path = absolute_path(workspace_root, path)?;
let dir_dist = DirectorySourceDist {
name: self.id.name.clone(),
url: verbatim_url(workspace_root.join(path), &self.id)?,
install_path: workspace_root.join(path),
url: verbatim_url(&install_path, &self.id)?,
install_path,
editable: true,
r#virtual: false,
};
uv_distribution_types::SourceDist::Directory(dir_dist)
}
Source::Virtual(path) => {
let install_path = absolute_path(workspace_root, path)?;
let dir_dist = DirectorySourceDist {
name: self.id.name.clone(),
url: verbatim_url(workspace_root.join(path), &self.id)?,
install_path: workspace_root.join(path),
url: verbatim_url(&install_path, &self.id)?,
install_path,
editable: false,
r#virtual: true,
};
Expand Down Expand Up @@ -2181,15 +2186,21 @@ impl Package {
}

/// Attempts to construct a `VerbatimUrl` from the given `Path`.
fn verbatim_url(path: PathBuf, id: &PackageId) -> Result<VerbatimUrl, LockError> {
fn verbatim_url(path: &Path, id: &PackageId) -> Result<VerbatimUrl, LockError> {
let url = VerbatimUrl::from_absolute_path(path).map_err(|err| LockErrorKind::VerbatimUrl {
id: id.clone(),
err,
})?;

Ok(url)
}

/// Attempts to construct an absolute path from the given `Path`.
fn absolute_path(workspace_root: &Path, path: &Path) -> Result<PathBuf, LockError> {
let path = uv_fs::normalize_absolute_path(&workspace_root.join(path))
.map_err(LockErrorKind::AbsolutePath)?;
Ok(path)
}

#[derive(Clone, Debug, serde::Deserialize)]
#[serde(rename_all = "kebab-case")]
struct PackageWire {
Expand Down Expand Up @@ -4059,6 +4070,13 @@ enum LockErrorKind {
#[source]
std::io::Error,
),
/// An error that occurs when converting a lockfile path from relative to absolute.
#[error("Could not compute absolute path from workspace root and lockfile path")]
AbsolutePath(
/// The inner error we forward.
#[source]
std::io::Error,
),
/// An error that occurs when an ambiguous `package.dependency` is
/// missing a `version` field.
#[error(
Expand Down
105 changes: 105 additions & 0 deletions crates/uv/tests/it/lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6242,6 +6242,111 @@ fn lock_exclusion() -> Result<()> {
Ok(())
}

/// See: <https://github.com/astral-sh/uv/issues/9832#issuecomment-2539121761>
#[test]
fn lock_relative_lock_deserialization() -> Result<()> {
let context = TestContext::new("3.12");

let pyproject_toml = context.temp_dir.child("pyproject.toml");
pyproject_toml.write_str(
r#"
[project]
name = "project"
requires-python = ">=3.12"
dependencies = ["member"]
dynamic = ["version"]

[tool.uv.sources]
member = { workspace = true }

[tool.uv.workspace]
members = ["packages/*"]
exclude = ["packages/child"]

[build-system]
requires = ["setuptools>=42"]
build-backend = "setuptools.build_meta"
"#,
)?;

let packages = context.temp_dir.child("packages");

let member = packages.child("member");
member.child("pyproject.toml").write_str(
r#"
[project]
name = "member"
requires-python = ">=3.12"
dependencies = []
dynamic = ["version"]

[build-system]
requires = ["setuptools>=42"]
build-backend = "setuptools.build_meta"
"#,
)?;

let child = packages.child("child");
child.child("pyproject.toml").write_str(
r#"
[project]
name = "child"
version = "0.1.0"
requires-python = ">=3.12"
dependencies = ["member"]

[build-system]
requires = ["setuptools>=42"]
build-backend = "setuptools.build_meta"

[tool.uv.sources]
member = { workspace = true }
"#,
)?;

// Add an arbitrary lockfile, to ensure that we attempt to validate it, which is necessary to
// trigger the bug.
child.child("uv.lock").write_str(
r#"
version = 1
requires-python = ">=3.12"

[options]
exclude-newer = "2024-03-25T00:00:00Z"

[[package]]
name = "child"
version = "0.1.0"
source = { editable = "." }
dependencies = [
{ name = "project" },
]

[package.metadata]
requires-dist = [{ name = "project", directory = "../" }]

[[package]]
name = "project"
version = "0.1.0"
source = { directory = "../" }
"#,
)?;

uv_snapshot!(context.filters(), context.lock().current_dir(&child), @r###"
success: false
exit_code: 2
----- stdout -----

----- stderr -----
Using CPython 3.12.[X] interpreter at: [PYTHON-3.12]
error: Failed to generate package metadata for `child==0.1.0 @ editable+.`
Caused by: Failed to parse entry: `member`
Caused by: `member` references a workspace in `tool.uv.sources` (e.g., `member = { workspace = true }`), but is not a workspace member
"###);

Ok(())
}

/// Lock a workspace member with a non-workspace source.
#[test]
fn lock_non_workspace_source() -> Result<()> {
Expand Down

0 comments on commit f80ddf1

Please sign in to comment.