mirror of https://github.com/astral-sh/uv
Avoid trailing slash when deserializing from lockfile (#9848)
## 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 https://github.com/astral-sh/uv/issues/9832#issuecomment-2539121761.
This commit is contained in:
parent
a13e3f5f69
commit
f80ddf10b6
|
|
@ -1682,10 +1682,11 @@ impl Package {
|
||||||
Source::Path(path) => {
|
Source::Path(path) => {
|
||||||
let filename: WheelFilename =
|
let filename: WheelFilename =
|
||||||
self.wheels[best_wheel_index].filename.clone();
|
self.wheels[best_wheel_index].filename.clone();
|
||||||
|
let install_path = absolute_path(workspace_root, path)?;
|
||||||
let path_dist = PathBuiltDist {
|
let path_dist = PathBuiltDist {
|
||||||
filename,
|
filename,
|
||||||
url: verbatim_url(workspace_root.join(path), &self.id)?,
|
url: verbatim_url(&install_path, &self.id)?,
|
||||||
install_path: workspace_root.join(path),
|
install_path: absolute_path(workspace_root, path)?,
|
||||||
};
|
};
|
||||||
let built_dist = BuiltDist::Path(path_dist);
|
let built_dist = BuiltDist::Path(path_dist);
|
||||||
Ok(Dist::Built(built_dist))
|
Ok(Dist::Built(built_dist))
|
||||||
|
|
@ -1780,40 +1781,44 @@ impl Package {
|
||||||
let DistExtension::Source(ext) = DistExtension::from_path(path)? else {
|
let DistExtension::Source(ext) = DistExtension::from_path(path)? else {
|
||||||
return Ok(None);
|
return Ok(None);
|
||||||
};
|
};
|
||||||
|
let install_path = absolute_path(workspace_root, path)?;
|
||||||
let path_dist = PathSourceDist {
|
let path_dist = PathSourceDist {
|
||||||
name: self.id.name.clone(),
|
name: self.id.name.clone(),
|
||||||
version: Some(self.id.version.clone()),
|
version: Some(self.id.version.clone()),
|
||||||
url: verbatim_url(workspace_root.join(path), &self.id)?,
|
url: verbatim_url(&install_path, &self.id)?,
|
||||||
install_path: workspace_root.join(path),
|
install_path,
|
||||||
ext,
|
ext,
|
||||||
};
|
};
|
||||||
uv_distribution_types::SourceDist::Path(path_dist)
|
uv_distribution_types::SourceDist::Path(path_dist)
|
||||||
}
|
}
|
||||||
Source::Directory(path) => {
|
Source::Directory(path) => {
|
||||||
|
let install_path = absolute_path(workspace_root, path)?;
|
||||||
let dir_dist = DirectorySourceDist {
|
let dir_dist = DirectorySourceDist {
|
||||||
name: self.id.name.clone(),
|
name: self.id.name.clone(),
|
||||||
url: verbatim_url(workspace_root.join(path), &self.id)?,
|
url: verbatim_url(&install_path, &self.id)?,
|
||||||
install_path: workspace_root.join(path),
|
install_path,
|
||||||
editable: false,
|
editable: false,
|
||||||
r#virtual: false,
|
r#virtual: false,
|
||||||
};
|
};
|
||||||
uv_distribution_types::SourceDist::Directory(dir_dist)
|
uv_distribution_types::SourceDist::Directory(dir_dist)
|
||||||
}
|
}
|
||||||
Source::Editable(path) => {
|
Source::Editable(path) => {
|
||||||
|
let install_path = absolute_path(workspace_root, path)?;
|
||||||
let dir_dist = DirectorySourceDist {
|
let dir_dist = DirectorySourceDist {
|
||||||
name: self.id.name.clone(),
|
name: self.id.name.clone(),
|
||||||
url: verbatim_url(workspace_root.join(path), &self.id)?,
|
url: verbatim_url(&install_path, &self.id)?,
|
||||||
install_path: workspace_root.join(path),
|
install_path,
|
||||||
editable: true,
|
editable: true,
|
||||||
r#virtual: false,
|
r#virtual: false,
|
||||||
};
|
};
|
||||||
uv_distribution_types::SourceDist::Directory(dir_dist)
|
uv_distribution_types::SourceDist::Directory(dir_dist)
|
||||||
}
|
}
|
||||||
Source::Virtual(path) => {
|
Source::Virtual(path) => {
|
||||||
|
let install_path = absolute_path(workspace_root, path)?;
|
||||||
let dir_dist = DirectorySourceDist {
|
let dir_dist = DirectorySourceDist {
|
||||||
name: self.id.name.clone(),
|
name: self.id.name.clone(),
|
||||||
url: verbatim_url(workspace_root.join(path), &self.id)?,
|
url: verbatim_url(&install_path, &self.id)?,
|
||||||
install_path: workspace_root.join(path),
|
install_path,
|
||||||
editable: false,
|
editable: false,
|
||||||
r#virtual: true,
|
r#virtual: true,
|
||||||
};
|
};
|
||||||
|
|
@ -2181,15 +2186,21 @@ impl Package {
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Attempts to construct a `VerbatimUrl` from the given `Path`.
|
/// 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 {
|
let url = VerbatimUrl::from_absolute_path(path).map_err(|err| LockErrorKind::VerbatimUrl {
|
||||||
id: id.clone(),
|
id: id.clone(),
|
||||||
err,
|
err,
|
||||||
})?;
|
})?;
|
||||||
|
|
||||||
Ok(url)
|
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)]
|
#[derive(Clone, Debug, serde::Deserialize)]
|
||||||
#[serde(rename_all = "kebab-case")]
|
#[serde(rename_all = "kebab-case")]
|
||||||
struct PackageWire {
|
struct PackageWire {
|
||||||
|
|
@ -4059,6 +4070,13 @@ enum LockErrorKind {
|
||||||
#[source]
|
#[source]
|
||||||
std::io::Error,
|
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
|
/// An error that occurs when an ambiguous `package.dependency` is
|
||||||
/// missing a `version` field.
|
/// missing a `version` field.
|
||||||
#[error(
|
#[error(
|
||||||
|
|
|
||||||
|
|
@ -6242,6 +6242,111 @@ fn lock_exclusion() -> Result<()> {
|
||||||
Ok(())
|
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.
|
/// Lock a workspace member with a non-workspace source.
|
||||||
#[test]
|
#[test]
|
||||||
fn lock_non_workspace_source() -> Result<()> {
|
fn lock_non_workspace_source() -> Result<()> {
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue