From 034f62b24fafe4cc8be5715fa0db4fd7354eb39c Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 19 Feb 2024 19:02:26 -0500 Subject: [PATCH] Respect `--index-url` provided via requirements.txt (#1719) ## Summary When we read `--index-url` from a `requirements.txt`, we attempt to respect the `--index-url` provided by the CLI if it exists. Unfortunately, `--index-url` from the CLI has a default value... so we _never_ respect the `--index-url` in the requirements file. This PR modifies the CLI to use `None`, and moves the default into logic in the `IndexLocations `struct. Closes https://github.com/astral-sh/uv/issues/1692. --- Cargo.lock | 1 + crates/distribution-types/Cargo.toml | 1 + crates/distribution-types/src/index_url.rs | 107 ++++++++++++++------- crates/uv-client/src/registry_client.rs | 4 +- crates/uv-dev/src/resolve_cli.rs | 8 +- crates/uv/src/main.rs | 32 +++--- crates/uv/tests/pip_compile.rs | 55 +++++++++++ 7 files changed, 153 insertions(+), 55 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8b5c488f5..0eb3a458a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -894,6 +894,7 @@ dependencies = [ "data-encoding", "distribution-filename", "fs-err", + "itertools 0.12.1", "once_cell", "pep440_rs 0.4.0", "pep508_rs", diff --git a/crates/distribution-types/Cargo.toml b/crates/distribution-types/Cargo.toml index 320956052..1e60c796c 100644 --- a/crates/distribution-types/Cargo.toml +++ b/crates/distribution-types/Cargo.toml @@ -26,6 +26,7 @@ pypi-types = { path = "../pypi-types" } anyhow = { workspace = true } data-encoding = { workspace = true } fs-err = { workspace = true } +itertools = { workspace = true } once_cell = { workspace = true } rkyv = { workspace = true, features = ["strict", "validation"] } rustc-hash = { workspace = true } diff --git a/crates/distribution-types/src/index_url.rs b/crates/distribution-types/src/index_url.rs index f82f90f7b..9270f7e09 100644 --- a/crates/distribution-types/src/index_url.rs +++ b/crates/distribution-types/src/index_url.rs @@ -3,6 +3,7 @@ use std::ops::Deref; use std::path::PathBuf; use std::str::FromStr; +use itertools::Either; use once_cell::sync::Lazy; use serde::{Deserialize, Serialize}; use url::Url; @@ -120,6 +121,8 @@ impl Display for FlatIndexLocation { /// The index locations to use for fetching packages. /// +/// By default, uses the PyPI index. +/// /// "pip treats all package sources equally" (), /// and so do we, i.e., you can't rely that on any particular order of querying indices. /// @@ -132,6 +135,7 @@ pub struct IndexLocations { index: Option, extra_index: Vec, flat_index: Vec, + no_index: bool, } impl Default for IndexLocations { @@ -141,30 +145,24 @@ impl Default for IndexLocations { index: Some(IndexUrl::Pypi), extra_index: Vec::new(), flat_index: Vec::new(), + no_index: false, } } } impl IndexLocations { /// Determine the index URLs to use for fetching packages. - pub fn from_args( - index: IndexUrl, + pub fn new( + index: Option, extra_index: Vec, flat_index: Vec, no_index: bool, ) -> Self { - if no_index { - Self { - index: None, - extra_index: Vec::new(), - flat_index, - } - } else { - Self { - index: Some(index), - extra_index, - flat_index, - } + Self { + index, + extra_index, + flat_index, + no_index, } } @@ -182,36 +180,44 @@ impl IndexLocations { flat_index: Vec, no_index: bool, ) -> Self { - if no_index { - Self { - index: None, - extra_index: Vec::new(), - flat_index, - } - } else { - Self { - index: self.index.or(index), - extra_index: self.extra_index.into_iter().chain(extra_index).collect(), - flat_index: self.flat_index.into_iter().chain(flat_index).collect(), - } + Self { + index: self.index.or(index), + extra_index: self.extra_index.into_iter().chain(extra_index).collect(), + flat_index: self.flat_index.into_iter().chain(flat_index).collect(), + no_index: self.no_index || no_index, } } } impl<'a> IndexLocations { - /// Return an iterator over all [`IndexUrl`] entries. - pub fn indexes(&'a self) -> impl Iterator + 'a { - self.index.iter().chain(self.extra_index.iter()) - } - /// Return the primary [`IndexUrl`] entry. + /// + /// If `--no-index` is set, return `None`. + /// + /// If no index is provided, use the `PyPI` index. pub fn index(&'a self) -> Option<&'a IndexUrl> { - self.index.as_ref() + if self.no_index { + None + } else { + match self.index.as_ref() { + Some(index) => Some(index), + None => Some(&IndexUrl::Pypi), + } + } } /// Return an iterator over the extra [`IndexUrl`] entries. pub fn extra_index(&'a self) -> impl Iterator + 'a { - self.extra_index.iter() + if self.no_index { + Either::Left(std::iter::empty()) + } else { + Either::Right(self.extra_index.iter()) + } + } + + /// Return an iterator over all [`IndexUrl`] entries. + pub fn indexes(&'a self) -> impl Iterator + 'a { + self.index().into_iter().chain(self.extra_index()) } /// Return an iterator over the [`FlatIndexLocation`] entries. @@ -224,6 +230,7 @@ impl<'a> IndexLocations { IndexUrls { index: self.index.clone(), extra_index: self.extra_index.clone(), + no_index: self.no_index, } } } @@ -235,6 +242,7 @@ impl<'a> IndexLocations { pub struct IndexUrls { index: Option, extra_index: Vec, + no_index: bool, } impl Default for IndexUrls { @@ -243,19 +251,45 @@ impl Default for IndexUrls { Self { index: Some(IndexUrl::Pypi), extra_index: Vec::new(), + no_index: false, } } } impl<'a> IndexUrls { - /// Return an iterator over the [`IndexUrl`] entries. + /// Return the primary [`IndexUrl`] entry. + /// + /// If `--no-index` is set, return `None`. + /// + /// If no index is provided, use the `PyPI` index. + pub fn index(&'a self) -> Option<&'a IndexUrl> { + if self.no_index { + None + } else { + match self.index.as_ref() { + Some(index) => Some(index), + None => Some(&IndexUrl::Pypi), + } + } + } + + /// Return an iterator over the extra [`IndexUrl`] entries. + pub fn extra_index(&'a self) -> impl Iterator + 'a { + if self.no_index { + Either::Left(std::iter::empty()) + } else { + Either::Right(self.extra_index.iter()) + } + } + + /// Return an iterator over all [`IndexUrl`] entries. pub fn indexes(&'a self) -> impl Iterator + 'a { - self.index.iter().chain(self.extra_index.iter()) + self.index().into_iter().chain(self.extra_index()) } /// Return `true` if no index is configured. pub fn no_index(&self) -> bool { - self.index.is_none() && self.extra_index.is_empty() + self.no_index } } @@ -264,6 +298,7 @@ impl From for IndexUrls { Self { index: locations.index, extra_index: locations.extra_index, + no_index: locations.no_index, } } } diff --git a/crates/uv-client/src/registry_client.rs b/crates/uv-client/src/registry_client.rs index 514e7ea77..a510045d1 100644 --- a/crates/uv-client/src/registry_client.rs +++ b/crates/uv-client/src/registry_client.rs @@ -160,7 +160,9 @@ impl RegistryClient { Err(CachedClientError::Client(err)) => match err.into_kind() { ErrorKind::Offline(_) => continue, ErrorKind::RequestError(err) => { - if err.status() == Some(StatusCode::NOT_FOUND) { + if err.status() == Some(StatusCode::NOT_FOUND) + || err.status() == Some(StatusCode::FORBIDDEN) + { continue; } Err(ErrorKind::RequestError(err).into()) diff --git a/crates/uv-dev/src/resolve_cli.rs b/crates/uv-dev/src/resolve_cli.rs index c5a130a5d..449b5bd6c 100644 --- a/crates/uv-dev/src/resolve_cli.rs +++ b/crates/uv-dev/src/resolve_cli.rs @@ -56,8 +56,12 @@ pub(crate) async fn resolve_cli(args: ResolveCliArgs) -> Result<()> { let platform = Platform::current()?; let venv = Virtualenv::from_env(platform, &cache)?; - let index_locations = - IndexLocations::from_args(args.index_url, args.extra_index_url, args.find_links, false); + let index_locations = IndexLocations::new( + Some(args.index_url), + args.extra_index_url, + args.find_links, + false, + ); let client = RegistryClientBuilder::new(cache.clone()) .index_urls(index_locations.index_urls()) .build(); diff --git a/crates/uv/src/main.rs b/crates/uv/src/main.rs index 95e3a693f..7ae9e8fad 100644 --- a/crates/uv/src/main.rs +++ b/crates/uv/src/main.rs @@ -239,9 +239,9 @@ struct PipCompileArgs { #[clap(long)] refresh_package: Vec, - /// The URL of the Python Package Index. - #[clap(long, short, default_value = IndexUrl::Pypi.as_str(), env = "UV_INDEX_URL")] - index_url: IndexUrl, + /// The URL of the Python package index (by default: https://pypi.org/simple). + #[clap(long, short, env = "UV_INDEX_URL")] + index_url: Option, /// Extra URLs of package indexes to use, in addition to `--index-url`. #[clap(long, env = "UV_EXTRA_INDEX_URL")] @@ -363,9 +363,9 @@ struct PipSyncArgs { #[clap(long, value_enum, default_value_t = install_wheel_rs::linker::LinkMode::default())] link_mode: install_wheel_rs::linker::LinkMode, - /// The URL of the Python Package Index. - #[clap(long, short, default_value = IndexUrl::Pypi.as_str(), env = "UV_INDEX_URL")] - index_url: IndexUrl, + /// The URL of the Python package index (by default: https://pypi.org/simple). + #[clap(long, short, env = "UV_INDEX_URL")] + index_url: Option, /// Extra URLs of package indexes to use, in addition to `--index-url`. #[clap(long, env = "UV_EXTRA_INDEX_URL")] @@ -528,9 +528,9 @@ struct PipInstallArgs { #[clap(short, long)] output_file: Option, - /// The URL of the Python Package Index. - #[clap(long, short, default_value = IndexUrl::Pypi.as_str(), env = "UV_INDEX_URL")] - index_url: IndexUrl, + /// The URL of the Python package index (by default: https://pypi.org/simple). + #[clap(long, short, env = "UV_INDEX_URL")] + index_url: Option, /// Extra URLs of package indexes to use, in addition to `--index-url`. #[clap(long, env = "UV_EXTRA_INDEX_URL")] @@ -669,9 +669,9 @@ struct VenvArgs { #[clap(long, verbatim_doc_comment)] prompt: Option, - /// The URL of the Python Package Index. - #[clap(long, short, default_value = IndexUrl::Pypi.as_str(), env = "UV_INDEX_URL")] - index_url: IndexUrl, + /// The URL of the Python package index (by default: https://pypi.org/simple). + #[clap(long, short, env = "UV_INDEX_URL")] + index_url: Option, /// Extra URLs of package indexes to use, in addition to `--index-url`. #[clap(long, env = "UV_EXTRA_INDEX_URL")] @@ -825,7 +825,7 @@ async fn run() -> Result { .into_iter() .map(RequirementsSource::from_path) .collect::>(); - let index_urls = IndexLocations::from_args( + let index_urls = IndexLocations::new( args.index_url, args.extra_index_url, args.find_links, @@ -885,7 +885,7 @@ async fn run() -> Result { args.compat_args.validate()?; let cache = cache.with_refresh(Refresh::from_args(args.refresh, args.refresh_package)); - let index_urls = IndexLocations::from_args( + let index_urls = IndexLocations::new( args.index_url, args.extra_index_url, args.find_links, @@ -947,7 +947,7 @@ async fn run() -> Result { .into_iter() .map(RequirementsSource::from_path) .collect::>(); - let index_urls = IndexLocations::from_args( + let index_urls = IndexLocations::new( args.index_url, args.extra_index_url, args.find_links, @@ -1023,7 +1023,7 @@ async fn run() -> Result { Commands::Venv(args) => { args.compat_args.validate()?; - let index_locations = IndexLocations::from_args( + let index_locations = IndexLocations::new( args.index_url, args.extra_index_url, // No find links for the venv subcommand, to keep things simple diff --git a/crates/uv/tests/pip_compile.rs b/crates/uv/tests/pip_compile.rs index 31b8c9774..855524b50 100644 --- a/crates/uv/tests/pip_compile.rs +++ b/crates/uv/tests/pip_compile.rs @@ -3540,3 +3540,58 @@ fn compile_constraints_incompatible_url() -> Result<()> { Ok(()) } + +/// Resolve a package from a `requirements.in` file, respecting the `--index-url` in a +/// `requirements.in` file. The resolution should fail, since the package doesn't exist at the +#[test] +fn index_url_in_requirements() -> Result<()> { + let context = TestContext::new("3.12"); + let requirements_in = context.temp_dir.child("requirements.in"); + requirements_in.write_str("--index-url https://download.pytorch.org/whl\nanyio<4")?; + + uv_snapshot!(context.compile() + .arg("requirements.in"), @r###" + success: false + exit_code: 1 + ----- stdout ----- + + ----- stderr ----- + × No solution found when resolving dependencies: + ╰─▶ Because anyio<4 was not found in the package registry and you require + anyio<4, we can conclude that the requirements are unsatisfiable. + "### + ); + + Ok(()) +} + +/// Resolve a package from a `requirements.in` file, respecting the `--index-url` passed via the +/// command line over that in a `requirements.in` file. +#[test] +fn index_url_from_command_line() -> Result<()> { + let context = TestContext::new("3.12"); + let requirements_in = context.temp_dir.child("requirements.in"); + requirements_in.write_str("--index-url https://download.pytorch.org/whl\nanyio<4")?; + + uv_snapshot!(context.compile() + .arg("requirements.in") + .arg("--index-url") + .arg("https://pypi.org/simple"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + # This file was autogenerated by uv via the following command: + # uv pip compile --cache-dir [CACHE_DIR] --exclude-newer 2023-11-18T12:00:00Z requirements.in --index-url https://pypi.org/simple + anyio==3.7.1 + idna==3.4 + # via anyio + sniffio==1.3.0 + # via anyio + + ----- stderr ----- + Resolved 3 packages in [TIME] + "### + ); + + Ok(()) +}