Auto merge of #11773 - weihanglo:beta-backport, r=ehuss

[beta-1.68] backport #11756

Beta backports:

- https://github.com/rust-lang/cargo/pull/11756

In order to make CI pass, the following PR are also cherry-picked:

- https://github.com/rust-lang/cargo/pull/11771

src/cargo/core/source/source_id.rs

@@ -81,6 +81,14 @@ impl SourceId {
     ///
     /// The canonical url will be calculated, but the precise field will not
     fn new(kind: SourceKind, url: Url, name: Option<&str>) -> CargoResult<SourceId> {
+        if kind == SourceKind::SparseRegistry {
+            // Sparse URLs are different because they store the kind prefix (sparse+)
+            // in the URL. This is because the prefix is necessary to differentiate
+            // from regular registries (git-based). The sparse+ prefix is included
+            // everywhere, including user-facing locations such as the `config.toml`
+            // file that defines the registry, or whenever Cargo displays it to the user.
+            assert!(url.as_str().starts_with("sparse+"));
+        }
         let source_id = SourceId::wrap(SourceIdInner {
             kind,
             canonical_url: CanonicalUrl::new(&url)?,
@@ -152,7 +160,7 @@ impl SourceId {
                     .with_precise(Some("locked".to_string())))
             }
             "sparse" => {
-                let url = url.into_url()?;
+                let url = string.into_url()?;
                 Ok(SourceId::new(SourceKind::SparseRegistry, url, None)?
                     .with_precise(Some("locked".to_string())))
             }
@@ -721,6 +729,7 @@ impl<'a> fmt::Display for SourceIdAsUrl<'a> {
                 ref url,
                 ..
             } => {
+                // Sparse registry URL already includes the `sparse+` prefix
                 write!(f, "{url}")
             }
             SourceIdInner {
@@ -864,4 +873,14 @@ mod tests {
         assert_eq!(gen_hash(source_id), 17459999773908528552);
         assert_eq!(crate::util::hex::short_hash(&source_id), "6568fe2c2fab5bfe");
     }
+
+    #[test]
+    fn serde_roundtrip() {
+        let url = "sparse+https://my-crates.io/".into_url().unwrap();
+        let source_id = SourceId::for_registry(&url).unwrap();
+        let formatted = format!("{}", source_id.as_url());
+        let deserialized = SourceId::from_url(&formatted).unwrap();
+        assert_eq!(formatted, "sparse+https://my-crates.io/");
+        assert_eq!(source_id, deserialized);
+    }
 }

src/cargo/ops/cargo_install.rs

@@ -307,7 +307,7 @@ impl<'cfg, 'a> InstallablePackage<'cfg, 'a> {
         let compile = ops::compile_ws(&self.ws, &self.opts, &exec).with_context(|| {
             if let Some(td) = td_opt.take() {
                 // preserve the temporary directory, so the user can inspect it
-                td.into_path();
+                drop(td.into_path());
             }
 
             format!(

src/cargo/util/canonical_url.rs

@@ -1,4 +1,4 @@
-use crate::util::{errors::CargoResult, IntoUrl};
+use crate::util::errors::CargoResult;
 use std::hash::{self, Hash};
 use url::Url;
 
@@ -56,17 +56,6 @@ impl CanonicalUrl {
             url.path_segments_mut().unwrap().pop().push(&last);
         }
 
-        // Ignore the protocol specifier (if any).
-        if url.scheme().starts_with("sparse+") {
-            // NOTE: it is illegal to use set_scheme to change sparse+http(s) to http(s).
-            url = url
-                .to_string()
-                .strip_prefix("sparse+")
-                .expect("we just found that prefix")
-                .into_url()
-                .expect("a valid url without a protocol specifier should still be valid");
-        }
-
         Ok(CanonicalUrl(url))
     }
 

tests/testsuite/install.rs

@@ -2,13 +2,16 @@
 
 use std::fs::{self, OpenOptions};
 use std::io::prelude::*;
+use std::path::Path;
 
+use cargo_test_support::compare;
 use cargo_test_support::cross_compile;
 use cargo_test_support::git;
 use cargo_test_support::registry::{self, registry_path, Package};
 use cargo_test_support::{
     basic_manifest, cargo_process, no_such_file_err_msg, project, project_in, symlink_supported, t,
 };
+use cargo_util::ProcessError;
 
 use cargo_test_support::install::{
     assert_has_installed_exe, assert_has_not_installed_exe, cargo_home,
@@ -2105,3 +2108,103 @@ fn no_auto_fix_note() {
         .run();
     assert_has_not_installed_exe(cargo_home(), "auto_fix");
 }
+
+#[cargo_test]
+fn failed_install_retains_temp_directory() {
+    // Verifies that the temporary directory persists after a build failure.
+    Package::new("foo", "0.0.1")
+        .file("src/main.rs", "x")
+        .publish();
+    let err = cargo_process("install foo").exec_with_output().unwrap_err();
+    let err = err.downcast::<ProcessError>().unwrap();
+    let stderr = String::from_utf8(err.stderr.unwrap()).unwrap();
+    compare::match_contains(
+        "\
+[UPDATING] `dummy-registry` index
+[DOWNLOADING] crates ...
+[DOWNLOADED] foo v0.0.1 (registry `dummy-registry`)
+[INSTALLING] foo v0.0.1
+[COMPILING] foo v0.0.1
+",
+        &stderr,
+        None,
+    )
+    .unwrap();
+    compare::match_contains(
+        "error: failed to compile `foo v0.0.1`, intermediate artifacts can be found at `[..]`",
+        &stderr,
+        None,
+    )
+    .unwrap();
+
+    // Find the path in the output.
+    let start = stderr.find("found at `").unwrap() + 10;
+    let end = stderr[start..].find('\n').unwrap() - 1;
+    let path = Path::new(&stderr[start..(end + start)]);
+    assert!(path.exists());
+    assert!(path.join("release/deps").exists());
+}
+
+#[cargo_test]
+fn sparse_install() {
+    // Checks for an issue where uninstalling something corrupted
+    // the SourceIds of sparse registries.
+    // See https://github.com/rust-lang/cargo/issues/11751
+    let _registry = registry::RegistryBuilder::new().http_index().build();
+
+    pkg("foo", "0.0.1");
+    pkg("bar", "0.0.1");
+
+    cargo_process("install foo --registry dummy-registry")
+        .with_stderr(
+            "\
+[UPDATING] `dummy-registry` index
+[DOWNLOADING] crates ...
+[DOWNLOADED] foo v0.0.1 (registry `dummy-registry`)
+[INSTALLING] foo v0.0.1 (registry `dummy-registry`)
+[UPDATING] `dummy-registry` index
+[COMPILING] foo v0.0.1 (registry `dummy-registry`)
+[FINISHED] release [optimized] target(s) in [..]
+[INSTALLING] [ROOT]/home/.cargo/bin/foo[EXE]
+[INSTALLED] package `foo v0.0.1 (registry `dummy-registry`)` (executable `foo[EXE]`)
+[WARNING] be sure to add `[..]` to your PATH to be able to run the installed binaries
+",
+        )
+        .run();
+    assert_has_installed_exe(cargo_home(), "foo");
+    let assert_v1 = |expected| {
+        let v1 = fs::read_to_string(paths::home().join(".cargo/.crates.toml")).unwrap();
+        compare::assert_match_exact(expected, &v1);
+    };
+    assert_v1(
+        r#"[v1]
+"foo 0.0.1 (sparse+http://127.0.0.1:[..]/index/)" = ["foo[EXE]"]
+"#,
+    );
+    cargo_process("install bar").run();
+    assert_has_installed_exe(cargo_home(), "bar");
+    assert_v1(
+        r#"[v1]
+"bar 0.0.1 (registry+https://github.com/rust-lang/crates.io-index)" = ["bar[EXE]"]
+"foo 0.0.1 (sparse+http://127.0.0.1:[..]/index/)" = ["foo[EXE]"]
+"#,
+    );
+
+    cargo_process("uninstall bar")
+        .with_stderr("[REMOVING] [CWD]/home/.cargo/bin/bar[EXE]")
+        .run();
+    assert_has_not_installed_exe(cargo_home(), "bar");
+    assert_v1(
+        r#"[v1]
+"foo 0.0.1 (sparse+http://127.0.0.1:[..]/index/)" = ["foo[EXE]"]
+"#,
+    );
+    cargo_process("uninstall foo")
+        .with_stderr("[REMOVING] [CWD]/home/.cargo/bin/foo[EXE]")
+        .run();
+    assert_has_not_installed_exe(cargo_home(), "foo");
+    assert_v1(
+        r#"[v1]
+"#,
+    );
+}