Review feedback

astral-sh · Oct 15, 2024 · 9e54908 · 9e54908
1 parent bcb0f3e
commit 9e54908
Show file tree

Hide file tree

Showing 17 changed files with 242 additions and 176 deletions.
diff --git a/crates/uv-cli/src/lib.rs b/crates/uv-cli/src/lib.rs
@@ -782,7 +782,6 @@ fn parse_index_url(input: &str) -> Result<Maybe<IndexUrl>, String> {
     }
 }
 
-
 /// Parse a string into an [`FlatIndexLocation`], mapping the empty string to `None`.
 fn parse_flat_index(input: &str) -> Result<Maybe<FlatIndexLocation>, String> {
     if input.is_empty() {
@@ -3861,29 +3860,25 @@ pub struct IndexArgs {
     #[arg(long, env = "UV_DEFAULT_INDEX", value_parser = parse_default_index_source, help_heading = "Index options")]
     pub default_index: Option<Maybe<Index>>,
 
-    /// The URL of the Python package index (by default: <https://pypi.org/simple>).
+    /// (Deprecated: use `--default-index` instead) The URL of the Python package index (by default: <https://pypi.org/simple>).
     ///
     /// Accepts either a repository compliant with PEP 503 (the simple repository API), or a local
     /// directory laid out in the same format.
     ///
     /// The index given by this flag is given lower priority than all other
     /// indexes specified via the `--extra-index-url` flag.
-    ///
-    /// (Deprecated: use `--default-index` instead.)
     #[arg(long, short, env = EnvVars::UV_INDEX_URL, value_parser = parse_index_url, help_heading = "Index options")]
     pub index_url: Option<Maybe<IndexUrl>>,
 
-    /// Extra URLs of package indexes to use, in addition to `--index-url`.
+    /// (Deprecated: use `--index` instead) Extra URLs of package indexes to use, in addition to `--index-url`.
     ///
     /// Accepts either a repository compliant with PEP 503 (the simple repository API), or a local
     /// directory laid out in the same format.
     ///
     /// All indexes provided via this flag take priority over the index specified by
     /// `--index-url` (which defaults to PyPI). When multiple `--extra-index-url` flags are
     /// provided, earlier values take priority.
-    ///
-    /// (Deprecated: use `--index` instead.)
-    #[arg(long, EnvVars::UV_EXTRA_INDEX_URL, value_delimiter = ' ', value_parser = parse_index_url, help_heading = "Index options")]
+    #[arg(long, env = EnvVars::UV_EXTRA_INDEX_URL, value_delimiter = ' ', value_parser = parse_index_url, help_heading = "Index options")]
     pub extra_index_url: Option<Vec<Maybe<IndexUrl>>>,
 
     /// Locations to search for candidate distributions, in addition to those found in the registry

diff --git a/crates/uv-distribution-types/src/index.rs b/crates/uv-distribution-types/src/index.rs
@@ -26,7 +26,7 @@ pub struct Index {
     pub url: IndexUrl,
     /// Mark the index as explicit.
     ///
-    /// Explicit indexes will _only_ be used when explicitly enabled via a `[tool.uv.sources]`
+    /// Explicit indexes will _only_ be used when explicitly requested via a `[tool.uv.sources]`
     /// definition, as in:
     ///
     /// ```toml

diff --git a/crates/uv-distribution-types/src/index_url.rs b/crates/uv-distribution-types/src/index_url.rs
@@ -298,8 +298,8 @@ impl From<VerbatimUrl> for FlatIndexLocation {
 
 /// The index locations to use for fetching packages. By default, uses the PyPI index.
 ///
-/// From a pip perspective, this type merges `--index-url`, `--extra-index-url`, and `--find-links`,
-/// along with the uv-specific `--index` and `--default-index` options.
+/// This type merges the legacy `--index-url`, `--extra-index-url`, and `--find-links` options,
+/// along with the uv-specific `--index` and `--default-index`.
 #[derive(Default, Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
 #[serde(rename_all = "kebab-case", deny_unknown_fields)]
 pub struct IndexLocations {
@@ -451,8 +451,8 @@ impl<'a> IndexLocations {
 
 /// The index URLs to use for fetching packages.
 ///
-/// From a pip perspective, this type merges `--index-url` and `--extra-index-url`, along with the
-/// uv-specific `--index` and `--default-index` options.
+/// This type merges the legacy `--index-url` and `--extra-index-url` options, along with the
+/// uv-specific `--index` and `--default-index`.
 #[derive(Default, Debug, Clone, PartialEq, Eq)]
 pub struct IndexUrls {
     indexes: Vec<Index>,

diff --git a/crates/uv-pypi-types/src/requirement.rs b/crates/uv-pypi-types/src/requirement.rs
@@ -318,7 +318,7 @@ pub enum RequirementSource {
     /// The requirement has a version specifier, such as `foo >1,<2`.
     Registry {
         specifier: VersionSpecifiers,
-        /// Choose a version from the index with this name.
+        /// Choose a version from the index at the given URL.
         index: Option<Url>,
     },
     // TODO(konsti): Track and verify version specifier from `project.dependencies` matches the

diff --git a/crates/uv-settings/src/settings.rs b/crates/uv-settings/src/settings.rs
@@ -292,7 +292,10 @@ pub struct ResolverInstallerOptions {
     /// (the simple repository API), or a local directory laid out in the same format.
     ///
     /// Indexes are considered in the order in which they're defined, such that the first-defined
-    /// index has the highest priority.
+    /// index has the highest priority. Further, the indexes provided by this setting are given
+    /// higher priority than any indexes specified via [`index_url`](#index-url) or
+    /// [`extra_index_url`](#extra-index-url). uv will only consider the first index that contains
+    /// a given package, unless an alternative [index strategy](#index-strategy) is specified.
     ///
     /// If an index is marked as `explicit = true`, it will be used exclusively for those
     /// dependencies that select it explicitly via `[tool.uv.sources]`, as in:
@@ -307,9 +310,9 @@ pub struct ResolverInstallerOptions {
     /// torch = { index = "pytorch" }
     /// ```
     ///
-    /// Marking an index as `default = true` will disable the PyPI default index and move the
-    /// index to the end of the prioritized list, such that it is used when a package is not found
-    /// on any other index.
+    /// If an index is marked as `default = true`, it will be moved to the end of the prioritized list, such that it is
+    /// given the lowest priority when resolving packages. Additionally, marking an index as default will disable the
+    /// PyPI default index.
     #[option(
         default = "\"[]\"",
         value_type = "dict",
@@ -734,32 +737,6 @@ pub struct PipOptions {
         "#
     )]
     pub prefix: Option<PathBuf>,
-    /// The indexes to use when resolving dependencies.
-    ///
-    /// Accepts either a repository compliant with [PEP 503](https://peps.python.org/pep-0503/)
-    /// (the simple repository API), or a local directory laid out in the same format.
-    ///
-    /// Indexes are considered in the order in which they're defined, such that the first-defined
-    /// index has the highest priority. Further, the indexes provided by this setting are given
-    /// higher priority than any indexes specified via [`index_url`](#index-url) or
-    /// [`extra_index_url`](#extra-index-url).
-    ///
-    /// If an index is marked as `explicit = true`, it will be used exclusively for those
-    /// dependencies that select it explicitly via `[tool.uv.sources]`, as in:
-    ///
-    /// ```toml
-    /// [[tool.uv.index]]
-    /// name = "pytorch"
-    /// url = "https://download.pytorch.org/whl/cu121"
-    /// explicit = true
-    ///
-    /// [tool.uv.sources]
-    /// torch = { index = "pytorch" }
-    /// ```
-    ///
-    /// If an index is marked as `default = true`, it will be moved to the front of the list of
-    /// the list of indexes, such that it is given the highest priority when resolving packages.
-    /// Additionally, marking an index as default will disable the PyPI default index.
     #[serde(skip)]
     #[cfg_attr(feature = "schemars", schemars(skip))]
     pub index: Option<Vec<Index>>,

diff --git a/crates/uv-workspace/src/pyproject.rs b/crates/uv-workspace/src/pyproject.rs
@@ -163,7 +163,8 @@ pub struct ToolUv {
     /// Indexes are considered in the order in which they're defined, such that the first-defined
     /// index has the highest priority. Further, the indexes provided by this setting are given
     /// higher priority than any indexes specified via [`index_url`](#index-url) or
-    /// [`extra_index_url`](#extra-index-url).
+    /// [`extra_index_url`](#extra-index-url). uv will only consider the first index that contains
+    /// a given package, unless an alternative [index strategy](#index-strategy) is specified.
     ///
     /// If an index is marked as `explicit = true`, it will be used exclusively for those
     /// dependencies that select it explicitly via `[tool.uv.sources]`, as in:
@@ -178,9 +179,9 @@ pub struct ToolUv {
     /// torch = { index = "pytorch" }
     /// ```
     ///
-    /// If an index is marked as `default = true`, it will be moved to the front of the list of
-    /// the list of indexes, such that it is given the highest priority when resolving packages.
-    /// Additionally, marking an index as default will disable the PyPI default index.
+    /// If an index is marked as `default = true`, it will be moved to the end of the prioritized list, such that it is
+    /// given the lowest priority when resolving packages. Additionally, marking an index as default will disable the
+    /// PyPI default index.
     #[option(
         default = "\"[]\"",
         value_type = "dict",

diff --git a/crates/uv-workspace/src/workspace.rs b/crates/uv-workspace/src/workspace.rs
@@ -1,7 +1,5 @@
 //! Resolve the current [`ProjectWorkspace`] or [`Workspace`].
 
-mod tests;
-
 use either::Either;
 use glob::{glob, GlobError, PatternError};
 use rustc_hash::FxHashSet;
@@ -1524,3 +1522,7 @@ impl<'env> From<&'env VirtualProject> for InstallTarget<'env> {
         }
     }
 }
+
+#[cfg(test)]
+#[cfg(unix)] // Avoid path escaping for the unit tests
+mod tests;
diff --git a/crates/uv/src/settings.rs b/crates/uv/src/settings.rs
@@ -24,8 +24,8 @@ use uv_configuration::{
     NoBinary, NoBuild, PreviewMode, Reinstall, SourceStrategy, TargetTriple, TrustedHost,
     TrustedPublishing, Upgrade, VersionControlSystem,
 };
-use uv_install_wheel::linker::LinkMode;
 use uv_distribution_types::{DependencyMetadata, Index, IndexLocations};
+use uv_install_wheel::linker::LinkMode;
 use uv_normalize::PackageName;
 use uv_pep508::{ExtraName, RequirementOrigin};
 use uv_pypi_types::{Requirement, SupportedEnvironments};

diff --git a/crates/uv/tests/it/lock.rs b/crates/uv/tests/it/lock.rs
@@ -12200,7 +12200,7 @@ fn lock_default_index() -> Result<()> {
 }
 
 #[test]
-fn lock_explicit_index_cli() -> Result<()> {
+fn lock_named_index_cli() -> Result<()> {
     let context = TestContext::new("3.12");
 
     let pyproject_toml = context.temp_dir.child("pyproject.toml");
@@ -12247,8 +12247,8 @@ fn lock_explicit_index_cli() -> Result<()> {
 /// If a name is reused, the higher-priority index should "overwrite" the lower-priority index.
 /// In other words, the lower-priority index should be ignored entirely during implicit resolution.
 ///
-/// In this test, we should use PyPI (the default index) rather than falling back to Test PyPI,
-/// which should be ignored.
+/// In this test, we should use PyPI (the default index) and ignore `https://example.com` entirely.
+/// (Querying `https://example.com` would fail with a 500.)
 #[test]
 fn lock_repeat_named_index() -> Result<()> {
     let context = TestContext::new("3.12");
@@ -12268,10 +12268,11 @@ fn lock_repeat_named_index() -> Result<()> {
 
         [[tool.uv.index]]
         name = "pytorch"
-        url = "https://test.pypi.org/simple"
+        url = "https://example.com"
         "#,
     )?;
 
+    // Fall back to PyPI, since `iniconfig` doesn't exist on the PyTorch index.
     uv_snapshot!(context.filters(), context.lock(), @r###"
     success: true
     exit_code: 0
@@ -12320,6 +12321,153 @@ fn lock_repeat_named_index() -> Result<()> {
     Ok(())
 }
 
+/// If a name is reused, the higher-priority index should "overwrite" the lower-priority index.
+/// This includes names passed in via the CLI.
+#[test]
+fn lock_repeat_named_index_cli() -> Result<()> {
+    let context = TestContext::new("3.12");
+
+    let pyproject_toml = context.temp_dir.child("pyproject.toml");
+    pyproject_toml.write_str(
+        r#"
+        [project]
+        name = "project"
+        version = "0.1.0"
+        requires-python = ">=3.12"
+        dependencies = ["jinja2==3.1.2"]
+
+        [[tool.uv.index]]
+        name = "pytorch"
+        url = "https://download.pytorch.org/whl/cu121"
+        "#,
+    )?;
+
+    // Resolve to the PyTorch index.
+    uv_snapshot!(context.filters(), context.lock().env_remove("UV_EXCLUDE_NEWER"), @r###"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+
+    ----- stderr -----
+    Resolved 3 packages in [TIME]
+    "###);
+
+    let lock = fs_err::read_to_string(context.temp_dir.join("uv.lock")).unwrap();
+
+    insta::with_settings!({
+        filters => context.filters(),
+    }, {
+        assert_snapshot!(
+            lock, @r###"
+        version = 1
+        requires-python = ">=3.12"
+
+        [[package]]
+        name = "jinja2"
+        version = "3.1.2"
+        source = { registry = "https://download.pytorch.org/whl/cu121" }
+        dependencies = [
+            { name = "markupsafe" },
+        ]
+        wheels = [
+            { url = "https://download.pytorch.org/whl/Jinja2-3.1.2-py3-none-any.whl", hash = "sha256:6088930bfe239f0e6710546ab9c19c9ef35e29792895fed6e6e31a023a182a61" },
+        ]
+
+        [[package]]
+        name = "markupsafe"
+        version = "2.1.5"
+        source = { registry = "https://download.pytorch.org/whl/cu121" }
+        wheels = [
+            { url = "https://download.pytorch.org/whl/MarkupSafe-2.1.5-cp312-cp312-macosx_10_9_universal2.whl", hash = "sha256:8dec4936e9c3100156f8a2dc89c4b88d5c435175ff03413b443469c7c8c5f4d1" },
+            { url = "https://download.pytorch.org/whl/MarkupSafe-2.1.5-cp312-cp312-macosx_10_9_x86_64.whl", hash = "sha256:3c6b973f22eb18a789b1460b4b91bf04ae3f0c4234a0a6aa6b0a92f6f7b951d4" },
+            { url = "https://download.pytorch.org/whl/MarkupSafe-2.1.5-cp312-cp312-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:ac07bad82163452a6884fe8fa0963fb98c2346ba78d779ec06bd7a6262132aee" },
+            { url = "https://download.pytorch.org/whl/MarkupSafe-2.1.5-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:f5dfb42c4604dddc8e4305050aa6deb084540643ed5804d7455b5df8fe16f5e5" },
+            { url = "https://download.pytorch.org/whl/MarkupSafe-2.1.5-cp312-cp312-manylinux_2_5_i686.manylinux1_i686.manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:ea3d8a3d18833cf4304cd2fc9cbb1efe188ca9b5efef2bdac7adc20594a0e46b" },
+            { url = "https://download.pytorch.org/whl/MarkupSafe-2.1.5-cp312-cp312-win_amd64.whl", hash = "sha256:823b65d8706e32ad2df51ed89496147a42a2a6e01c13cfb6ffb8b1e92bc910bb" },
+        ]
+
+        [[package]]
+        name = "project"
+        version = "0.1.0"
+        source = { virtual = "." }
+        dependencies = [
+            { name = "jinja2" },
+        ]
+
+        [package.metadata]
+        requires-dist = [{ name = "jinja2", specifier = "==3.1.2" }]
+        "###
+        );
+    });
+
+    // Resolve to PyPI, since the PyTorch index is replaced by the Packse index, which doesn't
+    // include `jinja2`.
+    uv_snapshot!(context.filters(), context.lock().arg("--index").arg(format!("pytorch={}", packse_index_url())).env_remove("UV_EXCLUDE_NEWER"), @r###"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+
+    ----- stderr -----
+    Resolved 3 packages in [TIME]
+    "###);
+
+    let lock = fs_err::read_to_string(context.temp_dir.join("uv.lock")).unwrap();
+
+    insta::with_settings!({
+        filters => context.filters(),
+    }, {
+        assert_snapshot!(
+            lock, @r###"
+        version = 1
+        requires-python = ">=3.12"
+
+        [[package]]
+        name = "jinja2"
+        version = "3.1.2"
+        source = { registry = "https://pypi.org/simple" }
+        dependencies = [
+            { name = "markupsafe" },
+        ]
+        sdist = { url = "https://files.pythonhosted.org/packages/7a/ff/75c28576a1d900e87eb6335b063fab47a8ef3c8b4d88524c4bf78f670cce/Jinja2-3.1.2.tar.gz", hash = "sha256:31351a702a408a9e7595a8fc6150fc3f43bb6bf7e319770cbc0db9df9437e852", size = 268239 }
+        wheels = [
+            { url = "https://files.pythonhosted.org/packages/bc/c3/f068337a370801f372f2f8f6bad74a5c140f6fda3d9de154052708dd3c65/Jinja2-3.1.2-py3-none-any.whl", hash = "sha256:6088930bfe239f0e6710546ab9c19c9ef35e29792895fed6e6e31a023a182a61", size = 133101 },
+        ]
+
+        [[package]]
+        name = "markupsafe"
+        version = "2.1.5"
+        source = { registry = "https://pypi.org/simple" }
+        sdist = { url = "https://files.pythonhosted.org/packages/87/5b/aae44c6655f3801e81aa3eef09dbbf012431987ba564d7231722f68df02d/MarkupSafe-2.1.5.tar.gz", hash = "sha256:d283d37a890ba4c1ae73ffadf8046435c76e7bc2247bbb63c00bd1a709c6544b", size = 19384 }
+        wheels = [
+            { url = "https://files.pythonhosted.org/packages/53/bd/583bf3e4c8d6a321938c13f49d44024dbe5ed63e0a7ba127e454a66da974/MarkupSafe-2.1.5-cp312-cp312-macosx_10_9_universal2.whl", hash = "sha256:8dec4936e9c3100156f8a2dc89c4b88d5c435175ff03413b443469c7c8c5f4d1", size = 18215 },
+            { url = "https://files.pythonhosted.org/packages/48/d6/e7cd795fc710292c3af3a06d80868ce4b02bfbbf370b7cee11d282815a2a/MarkupSafe-2.1.5-cp312-cp312-macosx_10_9_x86_64.whl", hash = "sha256:3c6b973f22eb18a789b1460b4b91bf04ae3f0c4234a0a6aa6b0a92f6f7b951d4", size = 14069 },
+            { url = "https://files.pythonhosted.org/packages/51/b5/5d8ec796e2a08fc814a2c7d2584b55f889a55cf17dd1a90f2beb70744e5c/MarkupSafe-2.1.5-cp312-cp312-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:ac07bad82163452a6884fe8fa0963fb98c2346ba78d779ec06bd7a6262132aee", size = 29452 },
+            { url = "https://files.pythonhosted.org/packages/0a/0d/2454f072fae3b5a137c119abf15465d1771319dfe9e4acbb31722a0fff91/MarkupSafe-2.1.5-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:f5dfb42c4604dddc8e4305050aa6deb084540643ed5804d7455b5df8fe16f5e5", size = 28462 },
+            { url = "https://files.pythonhosted.org/packages/2d/75/fd6cb2e68780f72d47e6671840ca517bda5ef663d30ada7616b0462ad1e3/MarkupSafe-2.1.5-cp312-cp312-manylinux_2_5_i686.manylinux1_i686.manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:ea3d8a3d18833cf4304cd2fc9cbb1efe188ca9b5efef2bdac7adc20594a0e46b", size = 27869 },
+            { url = "https://files.pythonhosted.org/packages/b0/81/147c477391c2750e8fc7705829f7351cf1cd3be64406edcf900dc633feb2/MarkupSafe-2.1.5-cp312-cp312-musllinux_1_1_aarch64.whl", hash = "sha256:d050b3361367a06d752db6ead6e7edeb0009be66bc3bae0ee9d97fb326badc2a", size = 33906 },
+            { url = "https://files.pythonhosted.org/packages/8b/ff/9a52b71839d7a256b563e85d11050e307121000dcebc97df120176b3ad93/MarkupSafe-2.1.5-cp312-cp312-musllinux_1_1_i686.whl", hash = "sha256:bec0a414d016ac1a18862a519e54b2fd0fc8bbfd6890376898a6c0891dd82e9f", size = 32296 },
+            { url = "https://files.pythonhosted.org/packages/88/07/2dc76aa51b481eb96a4c3198894f38b480490e834479611a4053fbf08623/MarkupSafe-2.1.5-cp312-cp312-musllinux_1_1_x86_64.whl", hash = "sha256:58c98fee265677f63a4385256a6d7683ab1832f3ddd1e66fe948d5880c21a169", size = 33038 },
+            { url = "https://files.pythonhosted.org/packages/96/0c/620c1fb3661858c0e37eb3cbffd8c6f732a67cd97296f725789679801b31/MarkupSafe-2.1.5-cp312-cp312-win32.whl", hash = "sha256:8590b4ae07a35970728874632fed7bd57b26b0102df2d2b233b6d9d82f6c62ad", size = 16572 },
+            { url = "https://files.pythonhosted.org/packages/3f/14/c3554d512d5f9100a95e737502f4a2323a1959f6d0d01e0d0997b35f7b10/MarkupSafe-2.1.5-cp312-cp312-win_amd64.whl", hash = "sha256:823b65d8706e32ad2df51ed89496147a42a2a6e01c13cfb6ffb8b1e92bc910bb", size = 17127 },
+        ]
+
+        [[package]]
+        name = "project"
+        version = "0.1.0"
+        source = { virtual = "." }
+        dependencies = [
+            { name = "jinja2" },
+        ]
+
+        [package.metadata]
+        requires-dist = [{ name = "jinja2", specifier = "==3.1.2" }]
+        "###
+        );
+    });
+
+    Ok(())
+}
+
 /// Lock a project with `package = false`, making it a virtual project.
 #[test]
 fn lock_explicit_virtual_project() -> Result<()> {