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 #1692.
astral-sh · Feb 20, 2024 · 034f62b · 034f62b
1 parent 7b2c93f
commit 034f62b
Show file tree

Hide file tree

Showing 7 changed files with 153 additions and 55 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git 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
@@ -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" (<https://github.com/pypa/pip/issues/8606#issuecomment-788754817>),
 /// 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<IndexUrl>,
     extra_index: Vec<IndexUrl>,
     flat_index: Vec<FlatIndexLocation>,
+    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<IndexUrl>,
         extra_index: Vec<IndexUrl>,
         flat_index: Vec<FlatIndexLocation>,
         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<FlatIndexLocation>,
         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<Item = &'a IndexUrl> + '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<Item = &'a IndexUrl> + '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<Item = &'a IndexUrl> + '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<IndexUrl>,
     extra_index: Vec<IndexUrl>,
+    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<Item = &'a IndexUrl> + '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<Item = &'a IndexUrl> + '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<IndexLocations> 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
@@ -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
@@ -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
@@ -239,9 +239,9 @@ struct PipCompileArgs {
     #[clap(long)]
     refresh_package: Vec<PackageName>,
 
-    /// 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<IndexUrl>,
 
     /// 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<IndexUrl>,
 
     /// 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<PathBuf>,
 
-    /// 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<IndexUrl>,
 
     /// 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<String>,
 
-    /// 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<IndexUrl>,
 
     /// 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<ExitStatus> {
                 .into_iter()
                 .map(RequirementsSource::from_path)
                 .collect::<Vec<_>>();
-            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<ExitStatus> {
             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<ExitStatus> {
                 .into_iter()
                 .map(RequirementsSource::from_path)
                 .collect::<Vec<_>>();
-            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<ExitStatus> {
         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
@@ -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(())
+}