From b1e4bc779ced87ce883d52ad68bd8386fc1f7935 Mon Sep 17 00:00:00 2001 From: Ilan Godik Date: Wed, 22 Jan 2025 21:44:35 +0200 Subject: [PATCH] [uv-settings]: Correct behavior for relative find-links paths when run from a subdir (#10827) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## One-liner Relative find-links configuration to local path from a pyproject.toml or uv.toml is now relative to the config file ## Summary ### Background One can configure find-links in a `pyproject.toml` or `uv.toml` file, which are located from the cli arg, system directory, user directory, or by traversing parent directories until one is encountered. This PR addresses the following scenario: - A project directory which includes a `pyproject.toml` or `uv.toml` file - The config file includes a `find-links` option. (eg under `[tool.uv]` for `pyproject.toml`) - The `find-links` option is configured to point to a local subdirectory in the project: `packages/` - There is a subdirectory called `subdir`, which is the current working directory - I run `uv run my_script.py`. This will locate the `pyproject.toml` in the parent directory ### Current Behavior - uv tries to use the path `subdir/packages/` to find packages, and fails. ### New Behavior - uv tries to use the path `packages/` to find the packages, and succeeds - Specifically, any relative local find-links path will resolve to be relative to the configuration file. ### Why is this behavior change OK? - I believe no one depends on the behavior that a relative find-links when running in a subdir will refer to different directories each time - Thus this change only allows a more common use case which didn't work previously. ## Test Plan - I re-created the setup mentioned above: ``` UvTest/ ├── packages/ │ ├── colorama-0.4.6-py2.py3-none-any.whl │ └── tqdm-4.67.1-py3-none-any.whl ├── subdir/ │ └── my_script.py └── pyproject.toml ``` ```toml # pyproject.toml [project] name = "uvtest" version = "0.1.0" description = "Add your description here" readme = "README.md" requires-python = ">=3.12" dependencies = [ "tqdm>=4.67.1", ] [tool.uv] offline = true no-index = true find-links = ["packages/"] ``` - With working directory under `subdir`, previously, running `uv sync --offline` would fail resolving the tdqm package, and after the change it succeeds. - Additionally, one can use `uv sync --show-settings` to show the actually-resolved settings - now having the desired path in `flat_index.url.path` ## Alternative designs considered - I considered modifying the `impl Deserialize for IndexUrl` to parse ahead of time directly with a base directory by having a custom `Deserializer` with a base dir field, but it seems to contradict the design of the serde `Deserialize` trait - which should work with all `Deserializer`s ## Future work - Support for adjusting all other local-relative paths in `Options` would be desired, but is out of scope for the current PR. --------- Co-authored-by: Charlie Marsh --- crates/uv-distribution-types/src/index.rs | 11 +++ crates/uv-distribution-types/src/index_url.rs | 57 ++++++++---- crates/uv-distribution-types/src/pip_index.rs | 7 ++ crates/uv-settings/src/lib.rs | 19 +++- crates/uv-settings/src/settings.rs | 93 ++++++++++++++++++- crates/uv/tests/it/sync.rs | 46 +++++++++ 6 files changed, 208 insertions(+), 25 deletions(-) diff --git a/crates/uv-distribution-types/src/index.rs b/crates/uv-distribution-types/src/index.rs index 7660072b3..3fd439827 100644 --- a/crates/uv-distribution-types/src/index.rs +++ b/crates/uv-distribution-types/src/index.rs @@ -1,3 +1,4 @@ +use std::path::Path; use std::str::FromStr; use thiserror::Error; @@ -166,6 +167,16 @@ impl Index { // Otherwise, extract the credentials from the URL. Credentials::from_url(self.url.url()) } + + /// Resolve the index relative to the given root directory. + pub fn relative_to(mut self, root_dir: &Path) -> Result { + if let IndexUrl::Path(ref url) = self.url { + if let Some(given) = url.given() { + self.url = IndexUrl::parse(given, Some(root_dir))?; + } + } + Ok(self) + } } impl FromStr for Index { diff --git a/crates/uv-distribution-types/src/index_url.rs b/crates/uv-distribution-types/src/index_url.rs index cd075331e..81c4f8e4f 100644 --- a/crates/uv-distribution-types/src/index_url.rs +++ b/crates/uv-distribution-types/src/index_url.rs @@ -1,6 +1,7 @@ use std::borrow::Cow; use std::fmt::{Display, Formatter}; use std::ops::Deref; +use std::path::Path; use std::str::FromStr; use std::sync::{Arc, LazyLock, RwLock}; @@ -27,6 +28,42 @@ pub enum IndexUrl { Path(VerbatimUrl), } +impl IndexUrl { + /// Parse an [`IndexUrl`] from a string, relative to an optional root directory. + /// + /// If no root directory is provided, relative paths are resolved against the current working + /// directory. + pub fn parse(path: &str, root_dir: Option<&Path>) -> Result { + let url = match split_scheme(path) { + Some((scheme, ..)) => { + match Scheme::parse(scheme) { + Some(_) => { + // Ex) `https://pypi.org/simple` + VerbatimUrl::parse_url(path)? + } + None => { + // Ex) `C:\Users\user\index` + if let Some(root_dir) = root_dir { + VerbatimUrl::from_path(path, root_dir)? + } else { + VerbatimUrl::from_absolute_path(std::path::absolute(path)?)? + } + } + } + } + None => { + // Ex) `/Users/user/index` + if let Some(root_dir) = root_dir { + VerbatimUrl::from_path(path, root_dir)? + } else { + VerbatimUrl::from_absolute_path(std::path::absolute(path)?)? + } + } + }; + Ok(Self::from(url.with_given(path))) + } +} + #[cfg(feature = "schemars")] impl schemars::JsonSchema for IndexUrl { fn schema_name() -> String { @@ -114,25 +151,7 @@ impl FromStr for IndexUrl { type Err = IndexUrlError; fn from_str(s: &str) -> Result { - let url = match split_scheme(s) { - Some((scheme, ..)) => { - match Scheme::parse(scheme) { - Some(_) => { - // Ex) `https://pypi.org/simple` - VerbatimUrl::parse_url(s)? - } - None => { - // Ex) `C:\Users\user\index` - VerbatimUrl::from_absolute_path(std::path::absolute(s)?)? - } - } - } - None => { - // Ex) `/Users/user/index` - VerbatimUrl::from_absolute_path(std::path::absolute(s)?)? - } - }; - Ok(Self::from(url.with_given(s))) + Self::parse(s, None) } } diff --git a/crates/uv-distribution-types/src/pip_index.rs b/crates/uv-distribution-types/src/pip_index.rs index 9d5de50ea..922f6521c 100644 --- a/crates/uv-distribution-types/src/pip_index.rs +++ b/crates/uv-distribution-types/src/pip_index.rs @@ -3,6 +3,7 @@ //! flags set. use serde::{Deserialize, Deserializer, Serialize}; +use std::path::Path; use crate::{Index, IndexUrl}; @@ -11,6 +12,12 @@ macro_rules! impl_index { #[derive(Debug, Clone, Eq, PartialEq)] pub struct $name(Index); + impl $name { + pub fn relative_to(self, root_dir: &Path) -> Result { + Ok(Self(self.0.relative_to(root_dir)?)) + } + } + impl From<$name> for Index { fn from(value: $name) -> Self { value.0 diff --git a/crates/uv-settings/src/lib.rs b/crates/uv-settings/src/lib.rs index 31de316c7..e4bc10840 100644 --- a/crates/uv-settings/src/lib.rs +++ b/crates/uv-settings/src/lib.rs @@ -108,8 +108,9 @@ impl FilesystemOptions { let path = dir.join("uv.toml"); match fs_err::read_to_string(&path) { Ok(content) => { - let options: Options = toml::from_str(&content) - .map_err(|err| Error::UvToml(path.clone(), Box::new(err)))?; + let options = toml::from_str::(&content) + .map_err(|err| Error::UvToml(path.clone(), Box::new(err)))? + .relative_to(&std::path::absolute(dir)?)?; // If the directory also contains a `[tool.uv]` table in a `pyproject.toml` file, // warn. @@ -155,6 +156,8 @@ impl FilesystemOptions { return Ok(None); }; + let options = options.relative_to(&std::path::absolute(dir)?)?; + tracing::debug!("Found workspace configuration at `{}`", path.display()); return Ok(Some(Self(options))); } @@ -252,8 +255,13 @@ fn system_config_file() -> Option { /// Load [`Options`] from a `uv.toml` file. fn read_file(path: &Path) -> Result { let content = fs_err::read_to_string(path)?; - let options: Options = - toml::from_str(&content).map_err(|err| Error::UvToml(path.to_path_buf(), Box::new(err)))?; + let options = toml::from_str::(&content) + .map_err(|err| Error::UvToml(path.to_path_buf(), Box::new(err)))?; + let options = if let Some(parent) = std::path::absolute(path)?.parent() { + options.relative_to(parent)? + } else { + options + }; Ok(options) } @@ -294,6 +302,9 @@ pub enum Error { #[error(transparent)] Io(#[from] std::io::Error), + #[error(transparent)] + Index(#[from] uv_distribution_types::IndexUrlError), + #[error("Failed to parse: `{}`", _0.user_display())] PyprojectToml(PathBuf, #[source] Box), diff --git a/crates/uv-settings/src/settings.rs b/crates/uv-settings/src/settings.rs index 6dd61ea38..17a43c6a0 100644 --- a/crates/uv-settings/src/settings.rs +++ b/crates/uv-settings/src/settings.rs @@ -1,4 +1,4 @@ -use std::{fmt::Debug, num::NonZeroUsize, path::PathBuf}; +use std::{fmt::Debug, num::NonZeroUsize, path::Path, path::PathBuf}; use serde::{Deserialize, Serialize}; use url::Url; @@ -9,7 +9,7 @@ use uv_configuration::{ TargetTriple, TrustedHost, TrustedPublishing, }; use uv_distribution_types::{ - Index, IndexUrl, PipExtraIndex, PipFindLinks, PipIndex, StaticMetadata, + Index, IndexUrl, IndexUrlError, PipExtraIndex, PipFindLinks, PipIndex, StaticMetadata, }; use uv_install_wheel::linker::LinkMode; use uv_macros::{CombineOptions, OptionsMetadata}; @@ -144,6 +144,15 @@ impl Options { ..Default::default() } } + + /// Resolve the [`Options`] relative to the given root directory. + pub fn relative_to(self, root_dir: &Path) -> Result { + Ok(Self { + top_level: self.top_level.relative_to(root_dir)?, + pip: self.pip.map(|pip| pip.relative_to(root_dir)).transpose()?, + ..self + }) + } } /// Global settings, relevant to all invocations. @@ -723,6 +732,46 @@ pub struct ResolverInstallerOptions { pub no_binary_package: Option>, } +impl ResolverInstallerOptions { + /// Resolve the [`ResolverInstallerOptions`] relative to the given root directory. + pub fn relative_to(self, root_dir: &Path) -> Result { + Ok(Self { + index: self + .index + .map(|index| { + index + .into_iter() + .map(|index| index.relative_to(root_dir)) + .collect::, _>>() + }) + .transpose()?, + index_url: self + .index_url + .map(|index_url| index_url.relative_to(root_dir)) + .transpose()?, + extra_index_url: self + .extra_index_url + .map(|extra_index_url| { + extra_index_url + .into_iter() + .map(|extra_index_url| extra_index_url.relative_to(root_dir)) + .collect::, _>>() + }) + .transpose()?, + find_links: self + .find_links + .map(|find_links| { + find_links + .into_iter() + .map(|find_link| find_link.relative_to(root_dir)) + .collect::, _>>() + }) + .transpose()?, + ..self + }) + } +} + /// Shared settings, relevant to all operations that might create managed python installations. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, CombineOptions, OptionsMetadata)] #[serde(rename_all = "kebab-case")] @@ -1480,6 +1529,46 @@ pub struct PipOptions { pub reinstall_package: Option>, } +impl PipOptions { + /// Resolve the [`PipOptions`] relative to the given root directory. + pub fn relative_to(self, root_dir: &Path) -> Result { + Ok(Self { + index: self + .index + .map(|index| { + index + .into_iter() + .map(|index| index.relative_to(root_dir)) + .collect::, _>>() + }) + .transpose()?, + index_url: self + .index_url + .map(|index_url| index_url.relative_to(root_dir)) + .transpose()?, + extra_index_url: self + .extra_index_url + .map(|extra_index_url| { + extra_index_url + .into_iter() + .map(|extra_index_url| extra_index_url.relative_to(root_dir)) + .collect::, _>>() + }) + .transpose()?, + find_links: self + .find_links + .map(|find_links| { + find_links + .into_iter() + .map(|find_link| find_link.relative_to(root_dir)) + .collect::, _>>() + }) + .transpose()?, + ..self + }) + } +} + impl From for ResolverOptions { fn from(value: ResolverInstallerOptions) -> Self { Self { diff --git a/crates/uv/tests/it/sync.rs b/crates/uv/tests/it/sync.rs index 025bb503b..5fa32c8d6 100644 --- a/crates/uv/tests/it/sync.rs +++ b/crates/uv/tests/it/sync.rs @@ -6115,3 +6115,49 @@ fn path_hash_mismatch() -> Result<()> { Ok(()) } + +#[test] +fn find_links_relative_in_config_works_from_subdir() -> Result<()> { + let context = TestContext::new("3.12"); + + let pyproject_toml = context.temp_dir.child("pyproject.toml"); + pyproject_toml.write_str(indoc! {r#" + [project] + name = "subdir_test" + version = "0.1.0" + requires-python = ">=3.12" + dependencies = ["ok==1.0.0"] + + [tool.uv] + find-links = ["packages/"] + "#})?; + + // Create packages/ subdirectory and copy our "offline" tqdm wheel there + let packages = context.temp_dir.child("packages"); + packages.create_dir_all()?; + + let wheel_src = context + .workspace_root + .join("scripts/links/ok-1.0.0-py3-none-any.whl"); + let wheel_dst = packages.child("ok-1.0.0-py3-none-any.whl"); + fs_err::copy(&wheel_src, &wheel_dst)?; + + // Create a separate subdir, which will become our working directory + let subdir = context.temp_dir.child("subdir"); + subdir.create_dir_all()?; + + // Run `uv sync --offline` from subdir. We expect it to find the local wheel in ../packages/. + uv_snapshot!(context.filters(), context.sync().current_dir(&subdir).arg("--offline"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 2 packages in [TIME] + Prepared 1 package in [TIME] + Installed 1 package in [TIME] + + ok==1.0.0 + "###); + + Ok(()) +}