Avoid rewriting [[tool.uv.index]] entries when credentials are provided (#8502)

## Summary

Instead of creating a new entry, we should reuse the existing entry (to
preserve decor); similarly, we should avoid overwriting fields that are
already "correct".

Closes https://github.com/astral-sh/uv/issues/8483.

Cargo.lock

@@ -5341,6 +5341,7 @@ dependencies = [
  "toml_edit",
  "tracing",
  "url",
+ "uv-cache-key",
  "uv-distribution-types",
  "uv-fs",
  "uv-git",

crates/uv-workspace/Cargo.toml

@@ -16,6 +16,7 @@ doctest = false
 workspace = true
 
 [dependencies]
+uv-cache-key = { workspace = true }
 uv-distribution-types = { workspace = true }
 uv-fs = { workspace = true, features = ["tokio", "schemars"] }
 uv-git = { workspace = true }

crates/uv-workspace/src/pyproject_mut.rs

@@ -1,9 +1,15 @@
-use itertools::Itertools;
 use std::path::Path;
 use std::str::FromStr;
 use std::{fmt, mem};
+
+use itertools::Itertools;
 use thiserror::Error;
-use toml_edit::{Array, ArrayOfTables, DocumentMut, Item, RawString, Table, TomlError, Value};
+use toml_edit::{
+    Array, ArrayOfTables, DocumentMut, Formatted, Item, RawString, Table, TomlError, Value,
+};
+use url::Url;
+
+use uv_cache_key::CanonicalUrl;
 use uv_distribution_types::Index;
 use uv_fs::PortablePath;
 use uv_pep440::{Version, VersionSpecifier, VersionSpecifiers};
@@ -220,70 +226,160 @@ impl PyProjectTomlMut {
             .ok_or(Error::MalformedSources)?
             .entry("index")
             .or_insert(Item::ArrayOfTables(ArrayOfTables::new()))
-            .as_array_of_tables()
+            .as_array_of_tables_mut()
             .ok_or(Error::MalformedSources)?;
 
-        let mut table = Table::new();
+        // If there's already an index with the same name or URL, update it (and move it to the top).
-        if let Some(name) = index.name.as_ref() {
+        let mut table = existing
-            table.insert("name", toml_edit::value(name.to_string()));
-        } else if let Some(name) = existing
             .iter()
             .find(|table| {
-                table
+                // If the index has the same name, reuse it.
-                    .get("url")
+                if let Some(index) = index.name.as_deref() {
-                    .is_some_and(|url| url.as_str() == Some(index.url.url().as_str()))
-            })
-            .and_then(|existing| existing.get("name"))
-        {
-            // If there's an existing index with the same URL, and a name, preserve the name.
-            table.insert("name", name.clone());
-        }
-        table.insert("url", toml_edit::value(index.url.to_string()));
-        if index.default {
-            table.insert("default", toml_edit::value(true));
-        }
-
-        // Push the item to the table.
-        let mut updated = ArrayOfTables::new();
-        updated.push(table);
-        for table in existing {
-            // If there's another index with the same name, replace it.
                     if table
                         .get("name")
-                .is_some_and(|name| name.as_str() == index.name.as_deref())
+                        .and_then(|name| name.as_str())
+                        .is_some_and(|name| name == index)
                     {
-                continue;
+                        return true;
+                    }
                 }
 
-            // If there's another default index, remove it.
+                // If the index is the default, and there's another default index, reuse it.
                 if index.default
                     && table
                         .get("default")
                         .is_some_and(|default| default.as_bool() == Some(true))
                 {
-                continue;
+                    return true;
                 }
 
-            // If there's another index with the same URL, replace it.
+                // If there's another index with the same URL, reuse it.
                 if table
                     .get("url")
-                .is_some_and(|url| url.as_str() == Some(index.url.url().as_str()))
+                    .and_then(|item| item.as_str())
+                    .and_then(|url| Url::parse(url).ok())
+                    .is_some_and(|url| {
+                        CanonicalUrl::new(&url) == CanonicalUrl::new(index.url.url())
+                    })
                 {
-                continue;
+                    return true;
                 }
 
-            updated.push(table.clone());
+                false
+            })
+            .cloned()
+            .unwrap_or_default();
+
+        // If necessary, update the name.
+        if let Some(index) = index.name.as_deref() {
+            if !table
+                .get("name")
+                .and_then(|name| name.as_str())
+                .is_some_and(|name| name == index)
+            {
+                let mut formatted = Formatted::new(index.to_string());
+                if let Some(value) = table.get("name").and_then(Item::as_value) {
+                    if let Some(prefix) = value.decor().prefix() {
+                        formatted.decor_mut().set_prefix(prefix.clone());
                     }
-        self.doc
+                    if let Some(suffix) = value.decor().suffix() {
-            .entry("tool")
+                        formatted.decor_mut().set_suffix(suffix.clone());
-            .or_insert(implicit())
+                    }
-            .as_table_mut()
+                }
-            .ok_or(Error::MalformedSources)?
+                table.insert("name", Value::String(formatted).into());
-            .entry("uv")
+            }
-            .or_insert(implicit())
+        }
-            .as_table_mut()
+
-            .ok_or(Error::MalformedSources)?
+        // If necessary, update the URL.
-            .insert("index", Item::ArrayOfTables(updated));
+        if !table
+            .get("url")
+            .and_then(|item| item.as_str())
+            .and_then(|url| Url::parse(url).ok())
+            .is_some_and(|url| CanonicalUrl::new(&url) == CanonicalUrl::new(index.url.url()))
+        {
+            let mut formatted = Formatted::new(index.url.to_string());
+            if let Some(value) = table.get("url").and_then(Item::as_value) {
+                if let Some(prefix) = value.decor().prefix() {
+                    formatted.decor_mut().set_prefix(prefix.clone());
+                }
+                if let Some(suffix) = value.decor().suffix() {
+                    formatted.decor_mut().set_suffix(suffix.clone());
+                }
+            }
+            table.insert("url", Value::String(formatted).into());
+        }
+
+        // If necessary, update the default.
+        if index.default {
+            if !table
+                .get("default")
+                .and_then(toml_edit::Item::as_bool)
+                .is_some_and(|default| default)
+            {
+                let mut formatted = Formatted::new(true);
+                if let Some(value) = table.get("default").and_then(Item::as_value) {
+                    if let Some(prefix) = value.decor().prefix() {
+                        formatted.decor_mut().set_prefix(prefix.clone());
+                    }
+                    if let Some(suffix) = value.decor().suffix() {
+                        formatted.decor_mut().set_suffix(suffix.clone());
+                    }
+                }
+                table.insert("default", Value::Boolean(formatted).into());
+            }
+        }
+
+        // Remove any replaced tables.
+        existing.retain(|table| {
+            // If the index has the same name, skip it.
+            if let Some(index) = index.name.as_deref() {
+                if table
+                    .get("name")
+                    .and_then(|name| name.as_str())
+                    .is_some_and(|name| name == index)
+                {
+                    return false;
+                }
+            }
+
+            // If there's another default index, skip it.
+            if index.default
+                && table
+                    .get("default")
+                    .is_some_and(|default| default.as_bool() == Some(true))
+            {
+                return false;
+            }
+
+            // If there's another index with the same URL, skip it.
+            if table
+                .get("url")
+                .and_then(|item| item.as_str())
+                .and_then(|url| Url::parse(url).ok())
+                .is_some_and(|url| CanonicalUrl::new(&url) == CanonicalUrl::new(index.url.url()))
+            {
+                return false;
+            }
+
+            true
+        });
+
+        // Set the position to the minimum, if it's not already the first element.
+        if let Some(min) = existing.iter().filter_map(toml_edit::Table::position).min() {
+            // if !table.position().is_some_and(|position| position < min) {
+            table.set_position(min);
+
+            // Increment the position of all existing elements.
+            for table in existing.iter_mut() {
+                if let Some(position) = table.position() {
+                    table.set_position(position + 1);
+                }
+            }
+            // }
+        }
+
+        // Push the item to the table.
+        existing.push(table);
 
         Ok(())
     }

crates/uv/tests/it/edit.rs

@@ -5507,7 +5507,7 @@ fn add_index() -> Result<()> {
         );
     });
 
-    // Adding a subsequent index should put it _below_ the existing index.
+    // Adding a subsequent index should put it _above_ the existing index.
     uv_snapshot!(context.filters(), context.add().arg("jinja2").arg("--index").arg("pytorch=https://download.pytorch.org/whl/cu121").env_remove(EnvVars::UV_EXCLUDE_NEWER), @r###"
     success: true
     exit_code: 0
@@ -5541,11 +5541,11 @@ fn add_index() -> Result<()> {
         name = "pytorch"
         url = "https://download.pytorch.org/whl/cu121"
 
-        [[tool.uv.index]]
-        url = "https://pypi.org/simple"
-
         [tool.uv.sources]
         jinja2 = { index = "pytorch" }
+
+        [[tool.uv.index]]
+        url = "https://pypi.org/simple"
         "###
         );
     });
@@ -5638,15 +5638,15 @@ fn add_index() -> Result<()> {
             "jinja2>=3.1.3",
         ]
 
+        [tool.uv.sources]
+        jinja2 = { index = "pytorch" }
+
         [[tool.uv.index]]
         name = "pytorch"
         url = "https://test.pypi.org/simple"
 
         [[tool.uv.index]]
         url = "https://pypi.org/simple"
-
-        [tool.uv.sources]
-        jinja2 = { index = "pytorch" }
         "###
         );
     });
@@ -5748,15 +5748,15 @@ fn add_index() -> Result<()> {
             "typing-extensions>=4.12.2",
         ]
 
+        [tool.uv.sources]
+        jinja2 = { index = "pytorch" }
+
         [[tool.uv.index]]
         url = "https://pypi.org/simple"
 
         [[tool.uv.index]]
         name = "pytorch"
         url = "https://test.pypi.org/simple"
-
-        [tool.uv.sources]
-        jinja2 = { index = "pytorch" }
         "###
         );
     });
@@ -5867,15 +5867,15 @@ fn add_index() -> Result<()> {
             "typing-extensions>=4.12.2",
         ]
 
+        [tool.uv.sources]
+        jinja2 = { index = "pytorch" }
+
         [[tool.uv.index]]
         name = "pytorch"
         url = "https://test.pypi.org/simple"
 
         [[tool.uv.index]]
         url = "https://pypi.org/simple"
-
-        [tool.uv.sources]
-        jinja2 = { index = "pytorch" }
         "###
         );
     });
@@ -6133,6 +6133,194 @@ fn add_default_index_url() -> Result<()> {
     Ok(())
 }
 
+#[test]
+fn add_index_credentials() -> Result<()> {
+    let context = TestContext::new("3.12");
+
+    let pyproject_toml = context.temp_dir.child("pyproject.toml");
+    pyproject_toml.write_str(indoc! {r#"
+        [project]
+        name = "project"
+        version = "0.1.0"
+        requires-python = ">=3.12"
+        dependencies = []
+
+        # Set an internal index as the default, without credentials.
+        [[tool.uv.index]]
+        name = "internal"
+        url = "https://pypi-proxy.fly.dev/basic-auth/simple"
+        default = true
+    "#})?;
+
+    // Provide credentials for the index via the environment variable.
+    uv_snapshot!(context.filters(), context.add().arg("iniconfig==2.0.0").env("UV_DEFAULT_INDEX", "https://public:heron@pypi-proxy.fly.dev/basic-auth/simple"), @r###"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+
+    ----- stderr -----
+    Resolved 2 packages in [TIME]
+    Prepared 1 package in [TIME]
+    Installed 1 package in [TIME]
+     + iniconfig==2.0.0
+    "###);
+
+    let pyproject_toml = fs_err::read_to_string(context.temp_dir.join("pyproject.toml"))?;
+
+    insta::with_settings!({
+        filters => context.filters(),
+    }, {
+        assert_snapshot!(
+            pyproject_toml, @r###"
+        [project]
+        name = "project"
+        version = "0.1.0"
+        requires-python = ">=3.12"
+        dependencies = [
+            "iniconfig==2.0.0",
+        ]
+
+        # Set an internal index as the default, without credentials.
+        [[tool.uv.index]]
+        name = "internal"
+        url = "https://pypi-proxy.fly.dev/basic-auth/simple"
+        default = true
+        "###
+        );
+    });
+
+    let lock = fs_err::read_to_string(context.temp_dir.join("uv.lock"))?;
+
+    insta::with_settings!({
+        filters => context.filters(),
+    }, {
+        assert_snapshot!(
+            lock, @r###"
+        version = 1
+        requires-python = ">=3.12"
+
+        [options]
+        exclude-newer = "2024-03-25T00:00:00Z"
+
+        [[package]]
+        name = "iniconfig"
+        version = "2.0.0"
+        source = { registry = "https://pypi-proxy.fly.dev/basic-auth/simple" }
+        sdist = { url = "https://pypi-proxy.fly.dev/basic-auth/files/packages/d7/4b/cbd8e699e64a6f16ca3a8220661b5f83792b3017d0f79807cb8708d33913/iniconfig-2.0.0.tar.gz", hash = "sha256:2d91e135bf72d31a410b17c16da610a82cb55f6b0477d1a902134b24a455b8b3", size = 4646 }
+        wheels = [
+            { url = "https://pypi-proxy.fly.dev/basic-auth/files/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl", hash = "sha256:b6a85871a79d2e3b22d2d1b94ac2824226a63c6b741c88f7ae975f18b6778374", size = 5892 },
+        ]
+
+        [[package]]
+        name = "project"
+        version = "0.1.0"
+        source = { virtual = "." }
+        dependencies = [
+            { name = "iniconfig" },
+        ]
+
+        [package.metadata]
+        requires-dist = [{ name = "iniconfig", specifier = "==2.0.0" }]
+        "###
+        );
+    });
+
+    Ok(())
+}
+
+#[test]
+fn add_index_comments() -> Result<()> {
+    let context = TestContext::new("3.12");
+
+    let pyproject_toml = context.temp_dir.child("pyproject.toml");
+    pyproject_toml.write_str(indoc! {r#"
+        [project]
+        name = "project"
+        version = "0.1.0"
+        requires-python = ">=3.12"
+        dependencies = []
+
+        [[tool.uv.index]]
+        name = "internal"
+        url = "https://test.pypi.org/simple"  # This is a test index.
+        default = true
+    "#})?;
+
+    // Preserve the comment on the index URL, despite replacing it.
+    uv_snapshot!(context.filters(), context.add().arg("iniconfig==2.0.0").env("UV_DEFAULT_INDEX", "https://pypi.org/simple"), @r###"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+
+    ----- stderr -----
+    Resolved 2 packages in [TIME]
+    Prepared 1 package in [TIME]
+    Installed 1 package in [TIME]
+     + iniconfig==2.0.0
+    "###);
+
+    let pyproject_toml = fs_err::read_to_string(context.temp_dir.join("pyproject.toml"))?;
+
+    insta::with_settings!({
+        filters => context.filters(),
+    }, {
+        assert_snapshot!(
+            pyproject_toml, @r###"
+        [project]
+        name = "project"
+        version = "0.1.0"
+        requires-python = ">=3.12"
+        dependencies = [
+            "iniconfig==2.0.0",
+        ]
+
+        [[tool.uv.index]]
+        name = "internal"
+        url = "https://pypi.org/simple"  # This is a test index.
+        default = true
+        "###
+        );
+    });
+
+    let lock = fs_err::read_to_string(context.temp_dir.join("uv.lock"))?;
+
+    insta::with_settings!({
+        filters => context.filters(),
+    }, {
+        assert_snapshot!(
+            lock, @r###"
+        version = 1
+        requires-python = ">=3.12"
+
+        [options]
+        exclude-newer = "2024-03-25T00:00:00Z"
+
+        [[package]]
+        name = "iniconfig"
+        version = "2.0.0"
+        source = { registry = "https://pypi.org/simple" }
+        sdist = { url = "https://files.pythonhosted.org/packages/d7/4b/cbd8e699e64a6f16ca3a8220661b5f83792b3017d0f79807cb8708d33913/iniconfig-2.0.0.tar.gz", hash = "sha256:2d91e135bf72d31a410b17c16da610a82cb55f6b0477d1a902134b24a455b8b3", size = 4646 }
+        wheels = [
+            { url = "https://files.pythonhosted.org/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl", hash = "sha256:b6a85871a79d2e3b22d2d1b94ac2824226a63c6b741c88f7ae975f18b6778374", size = 5892 },
+        ]
+
+        [[package]]
+        name = "project"
+        version = "0.1.0"
+        source = { virtual = "." }
+        dependencies = [
+            { name = "iniconfig" },
+        ]
+
+        [package.metadata]
+        requires-dist = [{ name = "iniconfig", specifier = "==2.0.0" }]
+        "###
+        );
+    });
+
+    Ok(())
+}
+
 /// Accidentally add a dependency on the project itself.
 #[test]
 fn add_self() -> Result<()> {