Support relative path wheels (#5969)

Surprisingly, this is a lockfile schema change: We can't store relative
paths in urls, so we have to store a `filename` entry instead of the
whole url.

Fixes #4355
This commit is contained in:
konsti 2024-08-09 23:57:16 +02:00 committed by GitHub
parent dd32087842
commit fcbee9ce25
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 221 additions and 50 deletions

View file

@ -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,
})))
}

View file

@ -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,
},

View file

@ -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);

View file

@ -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
}
}
}

View file

@ -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)) {

View file

@ -820,7 +820,7 @@ impl Package {
.wheels
.iter()
.map(|wheel| wheel.to_registry_dist(url.to_url()))
.collect();
.collect::<Result<_, LockError>>()?;
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<RegistryBuiltWheel, LockError> {
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<u64>,
}
#[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<InlineTable> {
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<WheelWire> for Wheel {
type Error = String;
fn try_from(wire: WheelWire) -> Result<Wheel, String> {
// 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::<WheelFilename>()
.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::<WheelFilename>().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.

View file

@ -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 {

View file

@ -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 {

View file

@ -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 {

View file

@ -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(())
}