Skip to content

Commit

Permalink
Store --find-links as relative paths
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Aug 22, 2024
1 parent 9ee52e4 commit 09b4aad
Show file tree
Hide file tree
Showing 8 changed files with 102 additions and 52 deletions.
26 changes: 10 additions & 16 deletions crates/distribution-types/src/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,12 @@ pub enum FileLocation {
/// Absolute URL.
AbsoluteUrl(UrlString),
/// Absolute path to a file.
Path(#[with(rkyv::with::AsString)] PathBuf),
Path {
#[with(rkyv::with::AsString)]
install_path: PathBuf,
#[with(rkyv::with::AsString)]
lock_path: PathBuf
}
}

impl FileLocation {
Expand Down Expand Up @@ -110,36 +115,25 @@ impl FileLocation {
Ok(joined)
}
FileLocation::AbsoluteUrl(ref absolute) => Ok(absolute.to_url()),
FileLocation::Path(ref path) => {
let path = path
FileLocation::Path { ref install_path, lock_path: _ } => {
let path = install_path
.to_str()
.ok_or_else(|| ToUrlError::PathNotUtf8 { path: path.clone() })?;
.ok_or_else(|| ToUrlError::PathNotUtf8 { path: install_path.clone() })?;
let url = Url::from_file_path(path).map_err(|()| ToUrlError::InvalidPath {
path: path.to_string(),
})?;
Ok(url)
}
}
}

/// Convert this location to a URL.
///
/// This method is identical to [`FileLocation::to_url`] except it avoids parsing absolute URLs
/// as they are already guaranteed to be valid.
pub fn to_url_string(&self) -> Result<UrlString, ToUrlError> {
match *self {
FileLocation::AbsoluteUrl(ref absolute) => Ok(absolute.clone()),
_ => Ok(self.to_url()?.into()),
}
}
}

impl Display for FileLocation {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self {
Self::RelativeUrl(_base, url) => Display::fmt(&url, f),
Self::AbsoluteUrl(url) => Display::fmt(&url.0, f),
Self::Path(path) => Display::fmt(&path.display(), f),
Self::Path { install_path, lock_path: _ } => Display::fmt(&install_path.display(), f),
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions crates/distribution-types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1035,7 +1035,7 @@ impl Identifier for FileLocation {
DistributionId::RelativeUrl(base.to_string(), url.to_string())
}
Self::AbsoluteUrl(url) => DistributionId::AbsoluteUrl(url.to_string()),
Self::Path(path) => path.distribution_id(),
Self::Path { install_path, lock_path: _ } => install_path.distribution_id(),
}
}

Expand All @@ -1045,7 +1045,7 @@ impl Identifier for FileLocation {
ResourceId::RelativeUrl(base.to_string(), url.to_string())
}
Self::AbsoluteUrl(url) => ResourceId::AbsoluteUrl(url.to_string()),
Self::Path(path) => path.resource_id(),
Self::Path { install_path, lock_path: _ } => install_path.resource_id(),
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/uv-client/src/flat_index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ impl<'a> FlatIndexClient<'a> {
requires_python: None,
size: None,
upload_time_utc_ms: None,
url: FileLocation::Path(entry.path().clone()),
url: FileLocation::Path { install_path: entry.path().clone(), lock_path: PathBuf::from(entry.path().file_name().unwrap()) },
yanked: None,
};

Expand Down
2 changes: 1 addition & 1 deletion crates/uv-client/src/registry_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ impl RegistryClient {
WheelLocation::Url(url)
}
}
FileLocation::Path(path) => WheelLocation::Path(path.clone()),
FileLocation::Path { install_path, lock_path: _ } => WheelLocation::Path(install_path.clone()),
};

match location {
Expand Down
4 changes: 2 additions & 2 deletions crates/uv-distribution/src/distribution_database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,14 +151,14 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> {
pypi_types::base_url_join_relative(base, url)?
}
FileLocation::AbsoluteUrl(url) => url.to_url(),
FileLocation::Path(path) => {
FileLocation::Path { install_path, lock_path: _} => {
let cache_entry = self.build_context.cache().entry(
CacheBucket::Wheels,
WheelCache::Index(&wheel.index).wheel_dir(wheel.name().as_ref()),
wheel.filename.stem(),
);
return self
.load_wheel(path, &wheel.filename, cache_entry, dist, hashes)
.load_wheel(install_path, &wheel.filename, cache_entry, dist, hashes)
.await;
}
};
Expand Down
16 changes: 8 additions & 8 deletions crates/uv-distribution/src/source/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,15 +100,15 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
pypi_types::base_url_join_relative(base, url)?
}
FileLocation::AbsoluteUrl(url) => url.to_url(),
FileLocation::Path(path) => {
let url = Url::from_file_path(path)
.map_err(|()| Error::RelativePath(path.clone()))?;
FileLocation::Path { install_path, lock_path: _} => {
let url = Url::from_file_path(install_path)
.map_err(|()| Error::RelativePath(install_path.clone()))?;
return self
.archive(
source,
&PathSourceUrl {
url: &url,
path: Cow::Borrowed(path),
path: Cow::Borrowed(install_path),
ext: dist.ext,
},
&cache_shard,
Expand Down Expand Up @@ -277,15 +277,15 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
pypi_types::base_url_join_relative(base, url)?
}
FileLocation::AbsoluteUrl(url) => url.to_url(),
FileLocation::Path(path) => {
let url = Url::from_file_path(path)
.map_err(|()| Error::RelativePath(path.clone()))?;
FileLocation::Path { install_path, lock_path: _} => {
let url = Url::from_file_path(install_path)
.map_err(|()| Error::RelativePath(install_path.clone()))?;
return self
.archive_metadata(
source,
&PathSourceUrl {
url: &url,
path: Cow::Borrowed(path),
path: Cow::Borrowed(install_path),
ext: dist.ext,
},
&cache_shard,
Expand Down
96 changes: 76 additions & 20 deletions crates/uv-resolver/src/lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1343,6 +1343,8 @@ impl Package {
return Ok(None);
};



let file_url = sdist.url().ok_or_else(|| LockErrorKind::MissingUrl {
name: self.id.name.clone(),
version: self.id.version.clone(),
Expand Down Expand Up @@ -2237,15 +2239,28 @@ impl SourceDist {
return Ok(None);
}

let url = normalize_file_location(&reg_dist.file.url)
.map_err(LockErrorKind::InvalidFileUrl)
.map_err(LockError::from)?;
let hash = reg_dist.file.hashes.iter().max().cloned().map(Hash::from);
let size = reg_dist.file.size;
Ok(Some(SourceDist::Url {
url,
metadata: SourceDistMetadata { hash, size },
}))
match &reg_dist.file.url {
FileLocation::RelativeUrl(_, _) | FileLocation::AbsoluteUrl(_) => {
let url = normalize_file_location(&reg_dist.file.url)
.map_err(LockErrorKind::InvalidFileUrl)
.map_err(LockError::from)?;
let hash = reg_dist.file.hashes.iter().max().cloned().map(Hash::from);
let size = reg_dist.file.size;
Ok(Some(SourceDist::Url {
url,
metadata: SourceDistMetadata { hash, size },
}))
}
FileLocation::Path { install_path: _, lock_path } => {
let path = lock_path.simplified().to_path_buf();
let hash = reg_dist.file.hashes.iter().max().cloned().map(Hash::from);
let size = reg_dist.file.size;
Ok(Some(SourceDist::Path {
path,
metadata: SourceDistMetadata { hash, size },
}))
}
}
}

fn from_direct_dist(
Expand Down Expand Up @@ -2491,17 +2506,31 @@ impl Wheel {

fn from_registry_wheel(wheel: &RegistryBuiltWheel) -> Result<Wheel, LockError> {
let filename = wheel.filename.clone();
let url = normalize_file_location(&wheel.file.url)
.map_err(LockErrorKind::InvalidFileUrl)
.map_err(LockError::from)?;
let hash = wheel.file.hashes.iter().max().cloned().map(Hash::from);
let size = wheel.file.size;
Ok(Wheel {
url: WheelWireSource::Url { url },
hash,
size,
filename,
})
match &wheel.file.url {
FileLocation::RelativeUrl(_, _) | FileLocation::AbsoluteUrl(_) => {
let url = normalize_file_location(&wheel.file.url)
.map_err(LockErrorKind::InvalidFileUrl)
.map_err(LockError::from)?;
let hash = wheel.file.hashes.iter().max().cloned().map(Hash::from);
let size = wheel.file.size;
Ok(Wheel {
url: WheelWireSource::Url { url },
hash,
size,
filename,
})
}
FileLocation::Path { install_path: _, lock_path } => {
let path = lock_path.simplified().to_path_buf();
Ok(Wheel {
url: WheelWireSource::Path { path },
hash: None,
size: None,
filename,
})
}
}

}

fn from_direct_dist(direct_dist: &DirectUrlBuiltDist, hashes: &[HashDigest]) -> Wheel {
Expand Down Expand Up @@ -2530,6 +2559,12 @@ impl Wheel {
let filename: WheelFilename = self.filename.clone();
let url_string = match &self.url {
WheelWireSource::Url { url } => url.clone(),
WheelWireSource::Path { path } => {
// Create path relative to the source index.
let index = url.to_file_path().expect("url is a file URL");
let path = index.join(path);
UrlString::from(Url::from_file_path(path).expect("path is a file path"))
}
WheelWireSource::Filename { .. } => {
return Err(LockErrorKind::MissingUrl {
name: self.filename.name.clone(),
Expand Down Expand Up @@ -2585,6 +2620,16 @@ enum WheelWireSource {
url: UrlString,
},
/// Used for path wheels.
Path {
/// The filename of the wheel.
///
/// This isn't part of the wire format since it's redundant with the
/// URL. But we do use it for various things, and thus compute it at
/// deserialization time. Not being able to extract a wheel filename from a
/// wheel URL is thus a deserialization error.
path: PathBuf,
},
/// Used for path wheels.
///
/// We only store the filename for path wheel, since we can't store a relative path in the url
Filename {
Expand All @@ -2605,6 +2650,9 @@ impl Wheel {
WheelWireSource::Filename { filename } => {
table.insert("filename", Value::from(filename.to_string()));
}
WheelWireSource::Path { path } => {
table.insert("path", Value::from(PortablePath::from(path).to_string()));
}
}
if let Some(ref hash) = self.hash {
table.insert("hash", Value::from(hash.to_string()));
Expand All @@ -2629,6 +2677,14 @@ impl TryFrom<WheelWire> for Wheel {
})?
}
WheelWireSource::Filename { filename } => filename.clone(),
WheelWireSource::Path { path } => {
let filename = path.file_name().and_then(|filename| filename.to_str()).ok_or(
"path wheels must have a filename in the path, but this path does not",
)?;
filename.parse::<WheelFilename>().map_err(|err| {
format!("failed to parse `{filename}` as wheel filename: {err}")
})?
}
};

Ok(Wheel {
Expand Down
4 changes: 2 additions & 2 deletions crates/uv/tests/lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6542,7 +6542,7 @@ fn lock_find_links_local_wheel() -> Result<()> {
version = "1000.0.0"
source = { registry = "file://[WORKSPACE]/scripts/links" }
wheels = [
{ url = "file://[WORKSPACE]/scripts/links/tqdm-1000.0.0-py3-none-any.whl" },
{ path = "tqdm-1000.0.0-py3-none-any.whl" },
]
"###
);
Expand Down Expand Up @@ -6630,7 +6630,7 @@ fn lock_find_links_local_sdist() -> Result<()> {
name = "tqdm"
version = "999.0.0"
source = { registry = "file://[WORKSPACE]/scripts/links" }
sdist = { url = "file://[WORKSPACE]/scripts/links/tqdm-999.0.0.tar.gz" }
sdist = { path = "tqdm-999.0.0.tar.gz" }
"###
);
});
Expand Down

0 comments on commit 09b4aad

Please sign in to comment.