From 4b3478aa19acf13341eea4e693f4cd3a857eb421 Mon Sep 17 00:00:00 2001 From: Tim de Jager Date: Tue, 5 Nov 2024 11:20:04 +0100 Subject: [PATCH 1/4] wip: use relative find links location --- src/lock_file/resolve/pypi.rs | 58 +++++++++++++++++++++++++++-------- 1 file changed, 46 insertions(+), 12 deletions(-) diff --git a/src/lock_file/resolve/pypi.rs b/src/lock_file/resolve/pypi.rs index f23d11e31..615161db8 100644 --- a/src/lock_file/resolve/pypi.rs +++ b/src/lock_file/resolve/pypi.rs @@ -35,7 +35,7 @@ use uv_dispatch::BuildDispatch; use uv_distribution::DistributionDatabase; use uv_distribution_types::{ BuiltDist, DependencyMetadata, Diagnostic, Dist, FileLocation, HashPolicy, IndexCapabilities, - InstalledDist, InstalledRegistryDist, Name, Resolution, ResolvedDist, SourceDist, + IndexUrl, InstalledDist, InstalledRegistryDist, Name, Resolution, ResolvedDist, SourceDist, }; use uv_git::GitResolver; use uv_install_wheel::linker::LinkMode; @@ -463,23 +463,58 @@ async fn lock_pypi_packages<'a>( } let pypi_package_data = match dist { + // Ignore installed distributions ResolvedDist::Installed(_) => { - // TODO handle installed distributions continue; } + ResolvedDist::Installable(Dist::Built(dist)) => { let (url_or_path, hash) = match &dist { BuiltDist::Registry(dist) => { let best_wheel = dist.best_wheel(); - let url = match &best_wheel.file.url { - FileLocation::AbsoluteUrl(url) => UrlOrPath::Url( - Url::from_str(url.as_ref()).expect("invalid absolute url"), - ), - // This happens when it is relative to the non-standard index - FileLocation::RelativeUrl(base, relative) => { - let base = Url::from_str(base).expect("invalid base url"); - let url = base.join(relative).expect("could not join urls"); - UrlOrPath::Url(url) + let url = match &best_wheel.index { + // This is the case where the registry index is a PyPI index + // or an URL + IndexUrl::Pypi(_) | IndexUrl::Url(_) => { + let url = match &best_wheel.file.url { + // Normal case + FileLocation::AbsoluteUrl(url) => UrlOrPath::Url( + Url::from_str(url.as_ref()).expect("invalid absolute url"), + ), + // This happens when it is relative to the non-standard index + FileLocation::RelativeUrl(base, relative) => { + let base = Url::from_str(base).expect("invalid base url"); + let url = base.join(relative).expect("could not join urls"); + UrlOrPath::Url(url) + } + }; + url + } + // This is the case where the index is a `--find-links` path + IndexUrl::Path(verbatim_url) => { + let base = verbatim_url + .to_url() + .to_file_path() + .expect("expected a file path for index url"); + let url = match &best_wheel.file.url { + // Normal case + FileLocation::AbsoluteUrl(url) => { + // Convert to a relative path from the base path + let absolute = url + .to_url() + .to_file_path() + .expect("expected a file path for url"); + let relative = absolute + .strip_prefix(base) + .expect("index url is not a prefix of the base path"); + UrlOrPath::Path(relative.to_path_buf()) + } + // This happens when it is relative to the non-standard index + FileLocation::RelativeUrl(_, relative) => { + UrlOrPath::Path(PathBuf::from(relative)) + } + }; + url } }; @@ -578,7 +613,6 @@ async fn lock_pypi_packages<'a>( (url_or_path, hash, false) } SourceDist::Directory(dir) => { - // TODO: check that `install_path` is correct // Compute the hash of the package based on the source tree. let hash = if dir.install_path.is_dir() { Some( From 174ab0f1ade88b703af033d1e0404af3bfcdeff9 Mon Sep 17 00:00:00 2001 From: Tim de Jager Date: Tue, 5 Nov 2024 15:39:57 +0100 Subject: [PATCH 2/4] fix: find links should be relative again --- examples/pypi-find-links/pixi.lock | 10 +- examples/pypi-find-links/pixi.toml | 1 + src/lock_file/resolve/pypi.rs | 289 +++++++++++++++++++---------- 3 files changed, 202 insertions(+), 98 deletions(-) diff --git a/examples/pypi-find-links/pixi.lock b/examples/pypi-find-links/pixi.lock index cdcf005ea..266aa0ae2 100644 --- a/examples/pypi-find-links/pixi.lock +++ b/examples/pypi-find-links/pixi.lock @@ -33,8 +33,8 @@ environments: - pypi: https://files.pythonhosted.org/packages/12/90/3c9ff0512038035f59d279fddeb79f5f1eccd8859f06d6163c58798b9487/certifi-2024.8.30-py3-none-any.whl - pypi: https://files.pythonhosted.org/packages/16/92/92a76dc2ff3a12e69ba94e7e05168d37d0345fa08c87e1fe24d0c2a42223/charset_normalizer-3.4.0-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.whl - pypi: https://files.pythonhosted.org/packages/76/c6/c88e154df9c4e1a2a66ccf0005a88dfb2650c1dffb6f5ce603dfbd452ce3/idna-3.10-py3-none-any.whl - - pypi: file:///Users/tdejager/development/prefix/pixi/examples/pypi-find-links/links/requests-2.31.0-py3-none-any.whl - pypi: https://files.pythonhosted.org/packages/ce/d9/5f4c13cecde62396b0d3fe530a50ccea91e7dfc1ccf0e09c228841bb5ba8/urllib3-2.2.3-py3-none-any.whl + - pypi: ./links/requests-2.31.0-py3-none-any.whl osx-64: - conda: https://conda.anaconda.org/conda-forge/osx-64/bzip2-1.0.8-hfdf4475_7.conda - conda: https://conda.anaconda.org/conda-forge/osx-64/ca-certificates-2024.8.30-h8857fd0_0.conda @@ -52,8 +52,8 @@ environments: - pypi: https://files.pythonhosted.org/packages/12/90/3c9ff0512038035f59d279fddeb79f5f1eccd8859f06d6163c58798b9487/certifi-2024.8.30-py3-none-any.whl - pypi: https://files.pythonhosted.org/packages/50/89/354cc56cf4dd2449715bc9a0f54f3aef3dc700d2d62d1fa5bbea53b13426/charset_normalizer-3.4.0-cp312-cp312-macosx_10_13_x86_64.whl - pypi: https://files.pythonhosted.org/packages/76/c6/c88e154df9c4e1a2a66ccf0005a88dfb2650c1dffb6f5ce603dfbd452ce3/idna-3.10-py3-none-any.whl - - pypi: file:///Users/tdejager/development/prefix/pixi/examples/pypi-find-links/links/requests-2.31.0-py3-none-any.whl - pypi: https://files.pythonhosted.org/packages/ce/d9/5f4c13cecde62396b0d3fe530a50ccea91e7dfc1ccf0e09c228841bb5ba8/urllib3-2.2.3-py3-none-any.whl + - pypi: ./links/requests-2.31.0-py3-none-any.whl osx-arm64: - conda: https://conda.anaconda.org/conda-forge/osx-arm64/bzip2-1.0.8-h99b78c6_7.conda - conda: https://conda.anaconda.org/conda-forge/osx-arm64/ca-certificates-2024.8.30-hf0a4a13_0.conda @@ -71,8 +71,8 @@ environments: - pypi: https://files.pythonhosted.org/packages/12/90/3c9ff0512038035f59d279fddeb79f5f1eccd8859f06d6163c58798b9487/certifi-2024.8.30-py3-none-any.whl - pypi: https://files.pythonhosted.org/packages/fa/44/b730e2a2580110ced837ac083d8ad222343c96bb6b66e9e4e706e4d0b6df/charset_normalizer-3.4.0-cp312-cp312-macosx_11_0_arm64.whl - pypi: https://files.pythonhosted.org/packages/76/c6/c88e154df9c4e1a2a66ccf0005a88dfb2650c1dffb6f5ce603dfbd452ce3/idna-3.10-py3-none-any.whl - - pypi: file:///Users/tdejager/development/prefix/pixi/examples/pypi-find-links/links/requests-2.31.0-py3-none-any.whl - pypi: https://files.pythonhosted.org/packages/ce/d9/5f4c13cecde62396b0d3fe530a50ccea91e7dfc1ccf0e09c228841bb5ba8/urllib3-2.2.3-py3-none-any.whl + - pypi: ./links/requests-2.31.0-py3-none-any.whl win-64: - conda: https://conda.anaconda.org/conda-forge/win-64/bzip2-1.0.8-h2466b09_7.conda - conda: https://conda.anaconda.org/conda-forge/win-64/ca-certificates-2024.8.30-h56e8100_0.conda @@ -92,8 +92,8 @@ environments: - pypi: https://files.pythonhosted.org/packages/12/90/3c9ff0512038035f59d279fddeb79f5f1eccd8859f06d6163c58798b9487/certifi-2024.8.30-py3-none-any.whl - pypi: https://files.pythonhosted.org/packages/3e/67/7b72b69d25b89c0b3cea583ee372c43aa24df15f0e0f8d3982c57804984b/charset_normalizer-3.4.0-cp312-cp312-win_amd64.whl - pypi: https://files.pythonhosted.org/packages/76/c6/c88e154df9c4e1a2a66ccf0005a88dfb2650c1dffb6f5ce603dfbd452ce3/idna-3.10-py3-none-any.whl - - pypi: file:///Users/tdejager/development/prefix/pixi/examples/pypi-find-links/links/requests-2.31.0-py3-none-any.whl - pypi: https://files.pythonhosted.org/packages/ce/d9/5f4c13cecde62396b0d3fe530a50ccea91e7dfc1ccf0e09c228841bb5ba8/urllib3-2.2.3-py3-none-any.whl + - pypi: ./links/requests-2.31.0-py3-none-any.whl packages: - kind: conda name: _libgcc_mutex @@ -929,7 +929,7 @@ packages: - kind: pypi name: requests version: 2.31.0 - url: file:///Users/tdejager/development/prefix/pixi/examples/pypi-find-links/links/requests-2.31.0-py3-none-any.whl + path: ./links/requests-2.31.0-py3-none-any.whl requires_dist: - charset-normalizer>=2,<4 - idna>=2.5,<4 diff --git a/examples/pypi-find-links/pixi.toml b/examples/pypi-find-links/pixi.toml index 36dcacef2..7d1bd85d6 100644 --- a/examples/pypi-find-links/pixi.toml +++ b/examples/pypi-find-links/pixi.toml @@ -3,6 +3,7 @@ authors = ["Tim de Jager "] channels = ["conda-forge"] name = "pypi-find-links" platforms = ["osx-arm64", "osx-64", "linux-64", "win-64"] + [project.pypi-options] # This is similar to the --find-links option in pip find-links = [{ path = "./links" }] diff --git a/src/lock_file/resolve/pypi.rs b/src/lock_file/resolve/pypi.rs index 615161db8..0a7566cfe 100644 --- a/src/lock_file/resolve/pypi.rs +++ b/src/lock_file/resolve/pypi.rs @@ -55,7 +55,13 @@ use crate::{ uv_reporter::{UvReporter, UvReporterOptions}, }; -fn parse_hashes_from_hash_vec(hashes: &Vec) -> Option { +#[derive(Debug, thiserror::Error)] +#[error("Invalid hash: {0} type: {1}")] +struct InvalidHash(String, String); + +fn parse_hashes_from_hash_vec( + hashes: &Vec, +) -> Result, InvalidHash> { let mut sha256 = None; let mut md5 = None; @@ -74,20 +80,32 @@ fn parse_hashes_from_hash_vec(hashes: &Vec) -> Option } match (sha256, md5) { - (Some(sha256), None) => Some(PackageHashes::Sha256( - parse_digest_from_hex::(&sha256).expect("invalid sha256"), - )), - (None, Some(md5)) => Some(PackageHashes::Md5( - parse_digest_from_hex::(&md5).expect("invalid md5"), - )), - (Some(sha256), Some(md5)) => Some(PackageHashes::Md5Sha256( - parse_digest_from_hex::(&md5).expect("invalid md5"), - parse_digest_from_hex::(&sha256).expect("invalid sha256"), - )), - (None, None) => None, + (Some(sha256), None) => Ok(Some(PackageHashes::Sha256( + parse_digest_from_hex::(&sha256) + .ok_or_else(|| InvalidHash(sha256.clone(), "sha256".to_string()))?, + ))), + (None, Some(md5)) => Ok(Some(PackageHashes::Md5( + parse_digest_from_hex::(&md5) + .ok_or_else(|| InvalidHash(md5.clone(), "md5".to_string()))?, + ))), + (Some(sha256), Some(md5)) => Ok(Some(PackageHashes::Md5Sha256( + parse_digest_from_hex::(&md5) + .ok_or_else(|| InvalidHash(md5.clone(), "md5".to_string()))?, + parse_digest_from_hex::(&sha256) + .ok_or_else(|| InvalidHash(sha256.clone(), "sha256".to_string()))?, + ))), + (None, None) => Ok(None), } } +#[derive(Debug, thiserror::Error)] +enum ProcessPathUrlError { + #[error("expected given path for {0} but none found")] + NoGivenPath(String), + #[error("given path is an invalid file path")] + InvalidFilePath(String), +} + /// Given a pyproject.toml and either case: /// 1) dependencies = [ foo @ /home/foo ] /// 2) tool.pixi.pypi-dependencies.foo = { path = "/home/foo"} @@ -104,14 +122,16 @@ fn parse_hashes_from_hash_vec(hashes: &Vec) -> Option /// relative /// /// I think this has to do with the order of UV processing the requirements -fn process_uv_path_url(path_url: &uv_pep508::VerbatimUrl) -> PathBuf { - let given = path_url.given().expect("path should have a given url"); +fn process_uv_path_url(path_url: &uv_pep508::VerbatimUrl) -> Result { + let given = path_url + .given() + .ok_or_else(|| ProcessPathUrlError::NoGivenPath(path_url.to_string()))?; if given.starts_with("file://") { path_url .to_file_path() - .expect("path should be a valid file path") + .map_err(|_| ProcessPathUrlError::InvalidFilePath(path_url.to_string())) } else { - PathBuf::from(given) + Ok(PathBuf::from(given)) } } @@ -162,13 +182,13 @@ pub async fn resolve_pypi( ) }) .map_ok(|(record, p)| { - ( - uv_normalize::PackageName::new(p.name.as_normalized().to_string()) - .expect("cannot convert to package name"), + Ok(( + uv_normalize::PackageName::new(p.name.as_normalized().to_string())?, (record.clone(), p), - ) + )) }) - .collect::, _>>() + .collect::, uv_normalize::InvalidNameError>, _>>() + .into_diagnostic()? .into_diagnostic() .context("failed to extract python packages from conda metadata")?; @@ -400,7 +420,7 @@ pub async fn resolve_pypi( // one. As we have noted in the comment above. let resolver_in_memory_index = InMemoryIndex::default(); let python_version = PythonVersion::from_str(&interpreter_version.to_string()) - .expect("could not get version from interpreter"); + .map_err(|e| miette::miette!("{}", e))?; let resolution = Resolver::new_custom_io( manifest, options, @@ -441,10 +461,124 @@ pub async fn resolve_pypi( resolution, &context.capabilities, context.concurrency.downloads, + project_root, ) .await } +#[derive(Debug, thiserror::Error)] +enum GetUrlOrPathError { + #[error("expected absolute path found: {path}", path = .0.display())] + InvalidAbsolutePath(PathBuf), + #[error("invalid base url: {0}")] + InvalidBaseUrl(String), + #[error("cannot join these urls {0} + {1}")] + CannotJoin(String, String), + #[error("expected path found: {0}")] + ExpectedPath(String), + #[error("{base} should be the base of {0}", base = .1.display())] + NotABase(String, PathBuf), + #[error("{base} should be the base of {absolute}", absolute = .0.display(), base = .1.display())] + NotABaseDir(PathBuf, PathBuf), +} + +/// Get the UrlOrPath from the index url and file location +/// This will be used to handle the case of a source or built distribution +/// coming from a registry index or a `--find-links` path +fn get_url_or_path( + index_url: &IndexUrl, + file_location: &FileLocation, + abs_project_root: &Path, +) -> Result { + const RELATIVE_BASE: &str = "./"; + match index_url { + // This is the case where the registry index is a PyPI index + // or an URL + IndexUrl::Pypi(_) | IndexUrl::Url(_) => { + let url = match file_location { + // Normal case, can be something like: + // https://files.pythonhosted.org/packages/12/90/3c9ff0512038035f59d279fddeb79f5f1eccd8859f06d6163c58798b9487/certifi-2024.8.30-py3-none-any.whl + FileLocation::AbsoluteUrl(url) => { + UrlOrPath::Url(Url::from_str(url.as_ref()).map_err(|_| { + GetUrlOrPathError::InvalidAbsolutePath(PathBuf::from(url.to_string())) + })?) + } + // This happens when it is relative to the non-standard index + // because we only lock absolute URLs, we need to join with the base + FileLocation::RelativeUrl(base, relative) => { + let base = Url::from_str(base) + .map_err(|_| GetUrlOrPathError::InvalidBaseUrl(base.clone()))?; + let url = base.join(relative).map_err(|_| { + GetUrlOrPathError::CannotJoin(base.to_string(), relative.clone()) + })?; + UrlOrPath::Url(url) + } + }; + Ok(url) + } + // From observation this is the case where the index is a `--find-links` path + // i.e a path to a directory. This is not a PyPI index, but a directory or a file with links to wheels + IndexUrl::Path(_) => { + let url = match file_location { + // Okay we would have something like: + // file:///home/user/project/dist/certifi-2024.8.30-py3-none-any.whl + FileLocation::AbsoluteUrl(url) => { + // Convert to a relative path from the base path + let absolute = url + .to_url() + .to_file_path() + .map_err(|_| GetUrlOrPathError::ExpectedPath(url.to_string()))?; + // !IMPORTANT! We need to strip the base path from the absolute path + // not the path returned by the uv solver. Why? Because we need the path relative + // to the project root, **not** the path relative to the --find-links path. + // This is because during installation we do something like: `project_root.join(relative_path)` + let relative = absolute.strip_prefix(&abs_project_root).map_err(|_| { + GetUrlOrPathError::NotABase(url.to_string(), abs_project_root.to_path_buf()) + })?; + // This is just to get a `./` in the path + UrlOrPath::Path( + PathBuf::from_str(RELATIVE_BASE) + .map_err(|_| { + GetUrlOrPathError::ExpectedPath(RELATIVE_BASE.to_string()) + })? + .join(relative.to_path_buf()), + ) + } + // This happens when it is relative to the non-standard index + // Not sure when this happens, but I (@tdejager) think the best approach is to + // convert into an absolute path, and then we can strip the project root, so we keep the relative path + // from the project root + FileLocation::RelativeUrl(base, relative) => { + // Example: + // absolute: /home/user/project/dist + let absolute = PathBuf::from_str(base) + .map_err(|_| GetUrlOrPathError::ExpectedPath(base.clone()))?; + // relative: certifi-2024.8.30-py3-none-any.whl + let relative = PathBuf::from_str(relative) + .map_err(|_| GetUrlOrPathError::ExpectedPath(relative.clone()))?; + // absolute: /home/user/project/dist/certifi-2024.8.30-py3-none-any.whl + let absolute = absolute.join(relative); + // !IMPORTANT! We need to strip the base path from the absolute path + let relative = absolute.strip_prefix(abs_project_root).map_err(|_| { + GetUrlOrPathError::NotABaseDir( + absolute.clone(), + abs_project_root.to_path_buf(), + ) + })?; + UrlOrPath::Path( + PathBuf::from_str(RELATIVE_BASE) + .map_err(|_| { + GetUrlOrPathError::ExpectedPath(RELATIVE_BASE.to_string()) + })? + .join(relative.to_path_buf()), + ) + } + }; + Ok(url) + } + } +} + /// Create a vector of locked packages from a resolution async fn lock_pypi_packages<'a>( conda_python_packages: CondaPythonPackages, @@ -453,6 +587,7 @@ async fn lock_pypi_packages<'a>( resolution: Resolution, index_capabilities: &IndexCapabilities, concurrent_downloads: usize, + abs_project_root: &Path, ) -> miette::Result> { let mut locked_packages = LockedPypiPackages::with_capacity(resolution.len()); let database = DistributionDatabase::new(registry_client, build_dispatch, concurrent_downloads); @@ -472,76 +607,44 @@ async fn lock_pypi_packages<'a>( let (url_or_path, hash) = match &dist { BuiltDist::Registry(dist) => { let best_wheel = dist.best_wheel(); - let url = match &best_wheel.index { - // This is the case where the registry index is a PyPI index - // or an URL - IndexUrl::Pypi(_) | IndexUrl::Url(_) => { - let url = match &best_wheel.file.url { - // Normal case - FileLocation::AbsoluteUrl(url) => UrlOrPath::Url( - Url::from_str(url.as_ref()).expect("invalid absolute url"), - ), - // This happens when it is relative to the non-standard index - FileLocation::RelativeUrl(base, relative) => { - let base = Url::from_str(base).expect("invalid base url"); - let url = base.join(relative).expect("could not join urls"); - UrlOrPath::Url(url) - } - }; - url - } - // This is the case where the index is a `--find-links` path - IndexUrl::Path(verbatim_url) => { - let base = verbatim_url - .to_url() - .to_file_path() - .expect("expected a file path for index url"); - let url = match &best_wheel.file.url { - // Normal case - FileLocation::AbsoluteUrl(url) => { - // Convert to a relative path from the base path - let absolute = url - .to_url() - .to_file_path() - .expect("expected a file path for url"); - let relative = absolute - .strip_prefix(base) - .expect("index url is not a prefix of the base path"); - UrlOrPath::Path(relative.to_path_buf()) - } - // This happens when it is relative to the non-standard index - FileLocation::RelativeUrl(_, relative) => { - UrlOrPath::Path(PathBuf::from(relative)) - } - }; - url - } - }; - - let hash = parse_hashes_from_hash_vec(&dist.best_wheel().file.hashes); - (url, hash) + let hash = parse_hashes_from_hash_vec(&dist.best_wheel().file.hashes) + .into_diagnostic() + .context("cannot parse hashes for registry dist")?; + let url_or_path = get_url_or_path( + &best_wheel.index, + &best_wheel.file.url, + abs_project_root, + ) + .into_diagnostic() + .context("cannot convert registry dist")?; + (url_or_path, hash) } BuiltDist::DirectUrl(dist) => { let url = dist.url.to_url(); let direct_url = Url::parse(&format!("direct+{url}")) - .expect("could not create direct-url"); + .into_diagnostic() + .context("cannot create direct url")?; (UrlOrPath::Url(direct_url), None) } - BuiltDist::Path(dist) => { - (UrlOrPath::Path(process_uv_path_url(&dist.url)), None) - } + BuiltDist::Path(dist) => ( + UrlOrPath::Path(process_uv_path_url(&dist.url).into_diagnostic()?), + None, + ), }; let metadata = registry_client .wheel_metadata(dist, index_capabilities) .await - .expect("failed to get wheel metadata"); + .into_diagnostic() + .wrap_err("cannot get wheel metadata")?; PypiPackageData { name: pep508_rs::PackageName::new(metadata.name.to_string()) - .expect("cannot convert name"), + .into_diagnostic() + .context("cannot convert name")?, version: pep440_rs::Version::from_str(&metadata.version.to_string()) - .expect("cannot convert version"), + .into_diagnostic() + .context("cannot convert version")?, requires_python: metadata .requires_python .map(|r| to_version_specifiers(&r)) @@ -558,7 +661,13 @@ async fn lock_pypi_packages<'a>( // Handle new hash stuff let hash = source .file() - .and_then(|file| parse_hashes_from_hash_vec(&file.hashes)); + .and_then(|file| { + parse_hashes_from_hash_vec(&file.hashes) + .into_diagnostic() + .context("cannot parse hashes for sdist") + .transpose() + }) + .transpose()?; let metadata_response = database .get_or_build_wheel_metadata(&Dist::Source(source.clone()), HashPolicy::None) @@ -570,23 +679,17 @@ async fn lock_pypi_packages<'a>( // otherwise try to construct it from the source let (url_or_path, hash, editable) = match source { SourceDist::Registry(reg) => { - let url_or_path = match ®.file.url { - FileLocation::AbsoluteUrl(url) => UrlOrPath::Url( - Url::from_str(url.as_ref()).expect("invalid absolute url"), - ), - // This happens when it is relative to the non-standard index - FileLocation::RelativeUrl(base, relative) => { - let base = Url::from_str(base).expect("invalid base url"); - let url = base.join(relative).expect("could not join urls"); - UrlOrPath::Url(url) - } - }; + let url_or_path = + get_url_or_path(®.index, ®.file.url, abs_project_root) + .into_diagnostic() + .context("cannot convert registry sdist")?; (url_or_path, hash, false) } SourceDist::DirectUrl(direct) => { let url = direct.url.to_url(); let direct_url = Url::parse(&format!("direct+{url}")) - .expect("could not create direct-url"); + .into_diagnostic() + .context("could not create direct-url")?; (direct_url.into(), hash, false) } SourceDist::Git(git) => (git.url.to_url().into(), hash, false), @@ -604,7 +707,7 @@ async fn lock_pypi_packages<'a>( }; // process the path or url that we get back from uv - let given_path = process_uv_path_url(&path.url); + let given_path = process_uv_path_url(&path.url).into_diagnostic()?; // Create the url for the lock file. This is based on the passed in URL // instead of from the source path to copy the path that was passed in from @@ -626,7 +729,7 @@ async fn lock_pypi_packages<'a>( }; // process the path or url that we get back from uv - let given_path = process_uv_path_url(&dir.url); + let given_path = process_uv_path_url(&dir.url).into_diagnostic()?; // Create the url for the lock file. This is based on the passed in URL // instead of from the source path to copy the path that was passed in from @@ -639,7 +742,7 @@ async fn lock_pypi_packages<'a>( PypiPackageData { name: to_normalize(&metadata.name).into_diagnostic()?, version: pep440_rs::Version::from_str(&metadata.version.to_string()) - .expect("cannot convert version"), + .into_diagnostic()?, requires_python: metadata .requires_python .map(|r| to_version_specifiers(&r)) From 9c7c22c776b011bc0556e7f503b0c8def7788214 Mon Sep 17 00:00:00 2001 From: Tim de Jager Date: Tue, 5 Nov 2024 16:26:12 +0100 Subject: [PATCH 3/4] fix: better path handling when locking --- src/lock_file/resolve/pypi.rs | 47 ++++++++++++++--------------------- 1 file changed, 19 insertions(+), 28 deletions(-) diff --git a/src/lock_file/resolve/pypi.rs b/src/lock_file/resolve/pypi.rs index 0a7566cfe..2d4d644cf 100644 --- a/src/lock_file/resolve/pypi.rs +++ b/src/lock_file/resolve/pypi.rs @@ -476,10 +476,6 @@ enum GetUrlOrPathError { CannotJoin(String, String), #[error("expected path found: {0}")] ExpectedPath(String), - #[error("{base} should be the base of {0}", base = .1.display())] - NotABase(String, PathBuf), - #[error("{base} should be the base of {absolute}", absolute = .0.display(), base = .1.display())] - NotABaseDir(PathBuf, PathBuf), } /// Get the UrlOrPath from the index url and file location @@ -532,46 +528,41 @@ fn get_url_or_path( // not the path returned by the uv solver. Why? Because we need the path relative // to the project root, **not** the path relative to the --find-links path. // This is because during installation we do something like: `project_root.join(relative_path)` - let relative = absolute.strip_prefix(&abs_project_root).map_err(|_| { - GetUrlOrPathError::NotABase(url.to_string(), abs_project_root.to_path_buf()) - })?; - // This is just to get a `./` in the path - UrlOrPath::Path( - PathBuf::from_str(RELATIVE_BASE) + let relative = absolute.strip_prefix(&abs_project_root); + let path = match relative { + // Apparently, we can make it relative to the project root + Ok(relative) => PathBuf::from_str(RELATIVE_BASE) .map_err(|_| { GetUrlOrPathError::ExpectedPath(RELATIVE_BASE.to_string()) })? .join(relative.to_path_buf()), - ) + // We can't make it relative to the project root + // so we just return the absolute path + Err(_) => absolute, + }; + UrlOrPath::Path(path) } // This happens when it is relative to the non-standard index - // Not sure when this happens, but I (@tdejager) think the best approach is to - // convert into an absolute path, and then we can strip the project root, so we keep the relative path - // from the project root + // location on disk. FileLocation::RelativeUrl(base, relative) => { - // Example: - // absolute: /home/user/project/dist + // This is the same logic as the `AbsoluteUrl` case + // basically but we just make an absolute path first let absolute = PathBuf::from_str(base) .map_err(|_| GetUrlOrPathError::ExpectedPath(base.clone()))?; - // relative: certifi-2024.8.30-py3-none-any.whl let relative = PathBuf::from_str(relative) .map_err(|_| GetUrlOrPathError::ExpectedPath(relative.clone()))?; - // absolute: /home/user/project/dist/certifi-2024.8.30-py3-none-any.whl let absolute = absolute.join(relative); - // !IMPORTANT! We need to strip the base path from the absolute path - let relative = absolute.strip_prefix(abs_project_root).map_err(|_| { - GetUrlOrPathError::NotABaseDir( - absolute.clone(), - abs_project_root.to_path_buf(), - ) - })?; - UrlOrPath::Path( - PathBuf::from_str(RELATIVE_BASE) + + let relative = absolute.strip_prefix(abs_project_root); + let path = match relative { + Ok(relative) => PathBuf::from_str(RELATIVE_BASE) .map_err(|_| { GetUrlOrPathError::ExpectedPath(RELATIVE_BASE.to_string()) })? .join(relative.to_path_buf()), - ) + Err(_) => absolute, + }; + UrlOrPath::Path(path) } }; Ok(url) From 5ee03c3dcc65e42eb647f0e4986558af03573d41 Mon Sep 17 00:00:00 2001 From: Tim de Jager Date: Tue, 5 Nov 2024 16:41:29 +0100 Subject: [PATCH 4/4] fix: clippy fixes --- src/lock_file/resolve/pypi.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lock_file/resolve/pypi.rs b/src/lock_file/resolve/pypi.rs index 2d4d644cf..fa92f65ae 100644 --- a/src/lock_file/resolve/pypi.rs +++ b/src/lock_file/resolve/pypi.rs @@ -528,14 +528,14 @@ fn get_url_or_path( // not the path returned by the uv solver. Why? Because we need the path relative // to the project root, **not** the path relative to the --find-links path. // This is because during installation we do something like: `project_root.join(relative_path)` - let relative = absolute.strip_prefix(&abs_project_root); + let relative = absolute.strip_prefix(abs_project_root); let path = match relative { // Apparently, we can make it relative to the project root Ok(relative) => PathBuf::from_str(RELATIVE_BASE) .map_err(|_| { GetUrlOrPathError::ExpectedPath(RELATIVE_BASE.to_string()) })? - .join(relative.to_path_buf()), + .join(relative), // We can't make it relative to the project root // so we just return the absolute path Err(_) => absolute, @@ -559,7 +559,7 @@ fn get_url_or_path( .map_err(|_| { GetUrlOrPathError::ExpectedPath(RELATIVE_BASE.to_string()) })? - .join(relative.to_path_buf()), + .join(relative), Err(_) => absolute, }; UrlOrPath::Path(path)