diff --git a/crates/distribution-types/src/lib.rs b/crates/distribution-types/src/lib.rs index 5ef1567b8..2c62947ad 100644 --- a/crates/distribution-types/src/lib.rs +++ b/crates/distribution-types/src/lib.rs @@ -216,8 +216,12 @@ pub struct DirectUrlBuiltDist { #[derive(Debug, Clone, Hash)] pub struct PathBuiltDist { pub filename: WheelFilename, - /// The path to the wheel. - pub path: PathBuf, + /// The resolved, absolute path to the wheel which we use for installing. + pub install_path: PathBuf, + /// The absolute path or path relative to the workspace root pointing to the wheel + /// which we use for locking. Unlike `given` on the verbatim URL all environment variables + /// are resolved, and unlike the install path, we did not yet join it on the base directory. + pub lock_path: PathBuf, /// The URL as it was provided by the user. pub url: VerbatimUrl, } @@ -372,7 +376,8 @@ impl Dist { } Ok(Self::Built(BuiltDist::Path(PathBuiltDist { filename, - path: canonicalized_path, + install_path: canonicalized_path.clone(), + lock_path: lock_path.to_path_buf(), url, }))) } diff --git a/crates/distribution-types/src/resolution.rs b/crates/distribution-types/src/resolution.rs index 064333559..87919bd04 100644 --- a/crates/distribution-types/src/resolution.rs +++ b/crates/distribution-types/src/resolution.rs @@ -160,8 +160,8 @@ impl From<&ResolvedDist> for Requirement { } } Dist::Built(BuiltDist::Path(wheel)) => RequirementSource::Path { - install_path: wheel.path.clone(), - lock_path: wheel.path.clone(), + install_path: wheel.install_path.clone(), + lock_path: wheel.lock_path.clone(), url: wheel.url.clone(), ext: DistExtension::Wheel, }, diff --git a/crates/uv-client/src/registry_client.rs b/crates/uv-client/src/registry_client.rs index 8eb16f8d0..ba5882a8e 100644 --- a/crates/uv-client/src/registry_client.rs +++ b/crates/uv-client/src/registry_client.rs @@ -453,7 +453,7 @@ impl RegistryClient { .await? } BuiltDist::Path(wheel) => { - let file = fs_err::tokio::File::open(&wheel.path) + let file = fs_err::tokio::File::open(&wheel.install_path) .await .map_err(ErrorKind::Io)?; let reader = tokio::io::BufReader::new(file); diff --git a/crates/uv-distribution/src/distribution_database.rs b/crates/uv-distribution/src/distribution_database.rs index 64c88f6f5..d5271e7b3 100644 --- a/crates/uv-distribution/src/distribution_database.rs +++ b/crates/uv-distribution/src/distribution_database.rs @@ -89,7 +89,7 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> { io::Error::new( io::ErrorKind::TimedOut, format!( - "Failed to download distribution due to network timeout. Try increasing UV_HTTP_TIMEOUT (current value: {}s).", self.client.unmanaged.timeout() + "Failed to download distribution due to network timeout. Try increasing UV_HTTP_TIMEOUT (current value: {}s).", self.client.unmanaged.timeout() ), ) } else { @@ -351,8 +351,14 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> { wheel.filename.stem(), ); - self.load_wheel(&wheel.path, &wheel.filename, cache_entry, dist, hashes) - .await + self.load_wheel( + &wheel.install_path, + &wheel.filename, + cache_entry, + dist, + hashes, + ) + .await } } } diff --git a/crates/uv-installer/src/plan.rs b/crates/uv-installer/src/plan.rs index e02274edd..ab2f6b8cf 100644 --- a/crates/uv-installer/src/plan.rs +++ b/crates/uv-installer/src/plan.rs @@ -330,13 +330,14 @@ impl<'a> Planner<'a> { let wheel = PathBuiltDist { filename, url: url.clone(), - path, + install_path: install_path.clone(), + lock_path: lock_path.clone(), }; if !wheel.filename.is_compatible(tags) { bail!( "A path dependency is incompatible with the current platform: {}", - wheel.path.user_display() + wheel.lock_path.user_display() ); } @@ -357,7 +358,7 @@ impl<'a> Planner<'a> { .entry(format!("{}.rev", wheel.filename.stem())); if let Some(pointer) = LocalArchivePointer::read_from(&cache_entry)? { - let timestamp = ArchiveTimestamp::from_file(&wheel.path)?; + let timestamp = ArchiveTimestamp::from_file(&wheel.install_path)?; if pointer.is_up_to_date(timestamp) { let archive = pointer.into_archive(); if archive.satisfies(hasher.get(&wheel)) { diff --git a/crates/uv-resolver/src/lock.rs b/crates/uv-resolver/src/lock.rs index ea1ee264b..901b1530e 100644 --- a/crates/uv-resolver/src/lock.rs +++ b/crates/uv-resolver/src/lock.rs @@ -820,7 +820,7 @@ impl Package { .wheels .iter() .map(|wheel| wheel.to_registry_dist(url.to_url())) - .collect(); + .collect::>()?; let reg_built_dist = RegistryBuiltDist { wheels, best_wheel_index, @@ -833,7 +833,8 @@ impl Package { let path_dist = PathBuiltDist { filename, url: verbatim_url(workspace_root.join(path), &self.id)?, - path: path.clone(), + install_path: workspace_root.join(path), + lock_path: path.clone(), }; let built_dist = BuiltDist::Path(path_dist); Ok(Dist::Built(built_dist)) @@ -971,7 +972,8 @@ impl Package { }; let file_url = sdist.url().ok_or_else(|| LockErrorKind::MissingUrl { - id: self.id.clone(), + name: self.id.name.clone(), + version: self.id.version.clone(), })?; let filename = sdist .filename() @@ -1035,7 +1037,7 @@ impl Package { // Add any wheels. for wheel in &self.wheels { let hash = wheel.hash.as_ref().map(|h| h.0.clone()); - let wheel = wheel.to_registry_dist(url.to_url()); + let wheel = wheel.to_registry_dist(url.to_url())?; let compat = WheelCompatibility::Compatible(HashComparison::Matched, None, None); prioritized_dist.insert_built(wheel, hash, compat); @@ -1531,7 +1533,7 @@ impl Source { } fn from_path_built_dist(path_dist: &PathBuiltDist) -> Source { - Source::Path(path_dist.path.clone()) + Source::Path(path_dist.lock_path.clone()) } fn from_path_source_dist(path_dist: &PathSourceDist) -> Source { @@ -2059,7 +2061,7 @@ struct Wheel { /// against was found. The location does not need to exist in the future, /// so this should be treated as only a hint to where to look and/or /// recording where the wheel file originally came from. - url: UrlString, + url: WheelWireSource, /// A hash of the built distribution. /// /// This is only present for wheels that come from registries and direct @@ -2132,7 +2134,7 @@ impl Wheel { let hash = wheel.file.hashes.iter().max().cloned().map(Hash::from); let size = wheel.file.size; Ok(Wheel { - url, + url: WheelWireSource::Url { url }, hash, size, filename, @@ -2141,7 +2143,9 @@ impl Wheel { fn from_direct_dist(direct_dist: &DirectUrlBuiltDist, hashes: &[HashDigest]) -> Wheel { Wheel { - url: direct_dist.url.to_url().into(), + url: WheelWireSource::Url { + url: direct_dist.url.to_url().into(), + }, hash: hashes.iter().max().cloned().map(Hash::from), size: None, filename: direct_dist.filename.clone(), @@ -2150,15 +2154,27 @@ impl Wheel { fn from_path_dist(path_dist: &PathBuiltDist, hashes: &[HashDigest]) -> Wheel { Wheel { - url: path_dist.url.to_url().into(), + url: WheelWireSource::Filename { + filename: path_dist.filename.clone(), + }, hash: hashes.iter().max().cloned().map(Hash::from), size: None, filename: path_dist.filename.clone(), } } - fn to_registry_dist(&self, url: Url) -> RegistryBuiltWheel { + fn to_registry_dist(&self, url: Url) -> Result { let filename: WheelFilename = self.filename.clone(); + let url_string = match &self.url { + WheelWireSource::Url { url } => url.clone(), + WheelWireSource::Filename { .. } => { + return Err(LockErrorKind::MissingUrl { + name: self.filename.name.clone(), + version: self.filename.version.clone(), + } + .into()) + } + }; let file = Box::new(distribution_types::File { dist_info_metadata: false, filename: filename.to_string(), @@ -2166,25 +2182,22 @@ impl Wheel { requires_python: None, size: self.size, upload_time_utc_ms: None, - url: FileLocation::AbsoluteUrl(self.url.clone()), + url: FileLocation::AbsoluteUrl(url_string), yanked: None, }); let index = IndexUrl::Url(VerbatimUrl::from_url(url)); - RegistryBuiltWheel { + Ok(RegistryBuiltWheel { filename, file, index, - } + }) } } #[derive(Clone, Debug, serde::Deserialize)] struct WheelWire { - /// A URL or file path (via `file://`) where the wheel that was locked - /// against was found. The location does not need to exist in the future, - /// so this should be treated as only a hint to where to look and/or - /// recording where the wheel file originally came from. - url: UrlString, + #[serde(flatten)] + url: WheelWireSource, /// A hash of the built distribution. /// /// This is only present for wheels that come from registries and direct @@ -2197,11 +2210,39 @@ struct WheelWire { size: Option, } +#[derive(Clone, Debug, serde::Deserialize, PartialEq, Eq)] +#[serde(untagged)] +enum WheelWireSource { + /// Used for all wheels except path wheels. + Url { + /// A URL or file path (via `file://`) where the wheel that was locked + /// against was found. The location does not need to exist in the future, + /// so this should be treated as only a hint to where to look and/or + /// recording where the wheel file originally came from. + url: UrlString, + }, + /// Used for path wheels. + /// + /// We only store the filename for path wheel, since we can't store a relative path in the url + Filename { + /// We duplicate the filename since a lot of code relies on having the filename on the + /// wheel entry. + filename: WheelFilename, + }, +} + impl Wheel { /// Returns the TOML representation of this wheel. fn to_toml(&self) -> anyhow::Result { let mut table = InlineTable::new(); - table.insert("url", Value::from(self.url.base())); + match &self.url { + WheelWireSource::Url { url } => { + table.insert("url", Value::from(url.base())); + } + WheelWireSource::Filename { filename } => { + table.insert("filename", Value::from(filename.to_string())); + } + } if let Some(ref hash) = self.hash { table.insert("hash", Value::from(hash.to_string())); } @@ -2216,13 +2257,16 @@ impl TryFrom for Wheel { type Error = String; fn try_from(wire: WheelWire) -> Result { - // Extract the filename segment from the URL. - let filename = wire.url.filename().map_err(|err| err.to_string())?; - - // Parse the filename as a wheel filename. - let filename = filename - .parse::() - .map_err(|err| format!("failed to parse `{filename}` as wheel filename: {err}"))?; + // If necessary, extract the filename from the URL. + let filename = match &wire.url { + WheelWireSource::Url { url } => { + let filename = url.filename().map_err(|err| err.to_string())?; + filename.parse::().map_err(|err| { + format!("failed to parse `{filename}` as wheel filename: {err}") + })? + } + WheelWireSource::Filename { filename } => filename.clone(), + }; Ok(Wheel { url: wire.url, @@ -2593,10 +2637,12 @@ enum LockErrorKind { }, /// An error that occurs when a distribution indicates that it is sourced from a registry, but /// is missing a URL. - #[error("found registry distribution {id} without a valid URL")] + #[error("found registry distribution {name}=={version} without a valid URL")] MissingUrl { - /// The ID of the distribution that is missing a URL. - id: PackageId, + /// The name of the distribution that is missing a URL. + name: PackageName, + /// The version of the distribution that is missing a URL. + version: Version, }, /// An error that occurs when a distribution indicates that it is sourced from a registry, but /// is missing a filename. diff --git a/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__hash_optional_missing.snap b/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__hash_optional_missing.snap index 2ad8d3067..622384878 100644 --- a/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__hash_optional_missing.snap +++ b/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__hash_optional_missing.snap @@ -28,9 +28,11 @@ Ok( sdist: None, wheels: [ Wheel { - url: UrlString( - "https://files.pythonhosted.org/packages/14/fd/2f20c40b45e4fb4324834aea24bd4afdf1143390242c0b33774da0e2e34f/anyio-4.3.0-py3-none-any.whl", - ), + url: Url { + url: UrlString( + "https://files.pythonhosted.org/packages/14/fd/2f20c40b45e4fb4324834aea24bd4afdf1143390242c0b33774da0e2e34f/anyio-4.3.0-py3-none-any.whl", + ), + }, hash: None, size: None, filename: WheelFilename { diff --git a/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__hash_optional_present.snap b/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__hash_optional_present.snap index 785129e15..56b522382 100644 --- a/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__hash_optional_present.snap +++ b/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__hash_optional_present.snap @@ -28,9 +28,11 @@ Ok( sdist: None, wheels: [ Wheel { - url: UrlString( - "https://files.pythonhosted.org/packages/14/fd/2f20c40b45e4fb4324834aea24bd4afdf1143390242c0b33774da0e2e34f/anyio-4.3.0-py3-none-any.whl", - ), + url: Url { + url: UrlString( + "https://files.pythonhosted.org/packages/14/fd/2f20c40b45e4fb4324834aea24bd4afdf1143390242c0b33774da0e2e34f/anyio-4.3.0-py3-none-any.whl", + ), + }, hash: Some( Hash( HashDigest { diff --git a/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__hash_required_present.snap b/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__hash_required_present.snap index d3a40abe8..ffab6d439 100644 --- a/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__hash_required_present.snap +++ b/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__hash_required_present.snap @@ -26,9 +26,11 @@ Ok( sdist: None, wheels: [ Wheel { - url: UrlString( - "file:///foo/bar/anyio-4.3.0-py3-none-any.whl", - ), + url: Url { + url: UrlString( + "file:///foo/bar/anyio-4.3.0-py3-none-any.whl", + ), + }, hash: Some( Hash( HashDigest { diff --git a/crates/uv/tests/sync.rs b/crates/uv/tests/sync.rs index 8d86bccc3..06fe52722 100644 --- a/crates/uv/tests/sync.rs +++ b/crates/uv/tests/sync.rs @@ -3,6 +3,7 @@ use anyhow::Result; use assert_cmd::prelude::*; use assert_fs::prelude::*; +use insta::assert_snapshot; use common::{uv_snapshot, TestContext}; @@ -677,3 +678,109 @@ fn sync_build_isolation_package() -> Result<()> { Ok(()) } + +/// Test that relative wheel paths are correctly preserved. +#[test] +fn sync_relative_wheel() -> Result<()> { + // TODO(charlie): On Windows, this test currently prepares two packages (vs. one of Unix). This + // is due to an error in path canonicalization, and GitHub Actions on Windows have a symlink + // in the path. + // + // See: https://github.com/astral-sh/uv/issues/5979 + let context = TestContext::new("3.12").with_filtered_counts(); + + let requirements = r#"[project] + name = "relative_wheel" + version = "0.1.0" + requires-python = ">=3.12" + dependencies = ["ok"] + + [tool.uv.sources] + ok = { path = "wheels/ok-1.0.0-py3-none-any.whl" } + + [build-system] + requires = ["hatchling"] + build-backend = "hatchling.build" + "#; + + context + .temp_dir + .child("src/relative_wheel/__init__.py") + .touch()?; + + context + .temp_dir + .child("pyproject.toml") + .write_str(requirements)?; + + context.temp_dir.child("wheels").create_dir_all()?; + fs_err::copy( + "../../scripts/links/ok-1.0.0-py3-none-any.whl", + context.temp_dir.join("wheels/ok-1.0.0-py3-none-any.whl"), + )?; + + uv_snapshot!(context.filters(), context.sync(), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + warning: `uv sync` is experimental and may change without warning + warning: `uv.sources` is experimental and may change without warning + Resolved [N] packages in [TIME] + Prepared [N] packages in [TIME] + Installed [N] packages in [TIME] + + ok==1.0.0 (from file://[TEMP_DIR]/wheels/ok-1.0.0-py3-none-any.whl) + + relative-wheel==0.1.0 (from file://[TEMP_DIR]/) + "###); + + let lock = fs_err::read_to_string(context.temp_dir.join("uv.lock"))?; + + insta::with_settings!( + { + filters => context.filters(), + }, + { + assert_snapshot!( + lock, @r###" + version = 1 + requires-python = ">=3.12" + + [options] + exclude-newer = "2024-03-25 00:00:00 UTC" + + [[package]] + name = "ok" + version = "1.0.0" + source = { path = "wheels/ok-1.0.0-py3-none-any.whl" } + wheels = [ + { filename = "ok-1.0.0-py3-none-any.whl", hash = "sha256:79f0b33e6ce1e09eaa1784c8eee275dfe84d215d9c65c652f07c18e85fdaac5f" }, + ] + + [[package]] + name = "relative-wheel" + version = "0.1.0" + source = { editable = "." } + dependencies = [ + { name = "ok" }, + ] + "### + ); + } + ); + + // Check that we can re-read the lockfile. + uv_snapshot!(context.filters(), context.sync(), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + warning: `uv sync` is experimental and may change without warning + warning: `uv.sources` is experimental and may change without warning + Resolved [N] packages in [TIME] + Audited [N] packages in [TIME] + "###); + + Ok(()) +}