Auto merge of #12015 - tedinski:fix-11999-recursive-deps, r=weihanglo

Build by PackageIdSpec, not name, to avoid ambiguity

### What does this PR try to resolve?

Fixes #11999

I previously added this code to ensure `cargo install` will build the intended package, but it turns out to cause ambiguity in the case where a package depends on prior versions of itself.

This ambiguity can be resolved by identifying the package to build by its pkg-spec, not name.

### How should we test and review this PR?

A test case is included.

I have additionally manually tested that, with this patch, the issue in #11999 is fixed and now installs correctly.

Author: bors

src/cargo/ops/cargo_install.rs

@@ -4,7 +4,9 @@ use std::sync::Arc;
 use std::{env, fs};
 
 use crate::core::compiler::{CompileKind, DefaultExecutor, Executor, UnitOutput};
-use crate::core::{Dependency, Edition, Package, PackageId, Source, SourceId, Target, Workspace};
+use crate::core::{
+    Dependency, Edition, Package, PackageId, PackageIdSpec, Source, SourceId, Target, Workspace,
+};
 use crate::ops::{common_for_install_and_uninstall::*, FilterRule};
 use crate::ops::{CompileFilter, Packages};
 use crate::sources::{GitSource, PathSource, SourceConfigMap};
@@ -168,14 +170,8 @@ impl<'cfg, 'a> InstallablePackage<'cfg, 'a> {
             }
         };
 
-        // When we build this package, we want to build the *specified* package only,
-        // and avoid building e.g. workspace default-members instead. Do so by constructing
-        // specialized compile options specific to the identified package.
-        // See test `path_install_workspace_root_despite_default_members`.
-        let mut opts = original_opts.clone();
-        opts.spec = Packages::Packages(vec![pkg.name().to_string()]);
-
-        let (ws, rustc, target) = make_ws_rustc_target(config, &opts, &source_id, pkg.clone())?;
+        let (ws, rustc, target) =
+            make_ws_rustc_target(config, &original_opts, &source_id, pkg.clone())?;
         // If we're installing in --locked mode and there's no `Cargo.lock` published
         // ie. the bin was published before https://github.com/rust-lang/cargo/pull/7026
         if config.locked() && !ws.root().join("Cargo.lock").exists() {
@@ -192,6 +188,17 @@ impl<'cfg, 'a> InstallablePackage<'cfg, 'a> {
             ws.current()?.clone()
         };
 
+        // When we build this package, we want to build the *specified* package only,
+        // and avoid building e.g. workspace default-members instead. Do so by constructing
+        // specialized compile options specific to the identified package.
+        // See test `path_install_workspace_root_despite_default_members`.
+        let mut opts = original_opts.clone();
+        // For cargo install tracking, we retain the source git url in `pkg`, but for the build spec
+        // we need to unconditionally use `ws.current()` to correctly address the path where we
+        // locally cloned that repo.
+        let pkgidspec = PackageIdSpec::from_package_id(ws.current()?.package_id());
+        opts.spec = Packages::Packages(vec![pkgidspec.to_string()]);
+
         if from_cwd {
             if pkg.manifest().edition() == Edition::Edition2015 {
                 config.shell().warn(

tests/testsuite/install.rs

@@ -1415,6 +1415,46 @@ fn path_install_workspace_root_despite_default_members() {
         .run();
 }
 
+#[cargo_test]
+fn git_install_workspace_root_despite_default_members() {
+    let p = git::repo(&paths::root().join("foo"))
+        .file(
+            "Cargo.toml",
+            r#"
+                [package]
+                name = "ws-root"
+                version = "0.1.0"
+                authors = []
+
+                [workspace]
+                members = ["ws-member"]
+                default-members = ["ws-member"]
+            "#,
+        )
+        .file("src/main.rs", "fn main() {}")
+        .file(
+            "ws-member/Cargo.toml",
+            r#"
+                [package]
+                name = "ws-member"
+                version = "0.1.0"
+                authors = []
+            "#,
+        )
+        .file("ws-member/src/main.rs", "fn main() {}")
+        .build();
+
+    cargo_process("install --git")
+        .arg(p.url().to_string())
+        .arg("ws-root")
+        .with_stderr_contains(
+            "[INSTALLED] package `ws-root v0.1.0 ([..])` (executable `ws-root[EXE]`)",
+        )
+        // Particularly avoid "Installed package `ws-root v0.1.0 ([..]])` (executable `ws-member`)":
+        .with_stderr_does_not_contain("ws-member")
+        .run();
+}
+
 #[cargo_test]
 fn dev_dependencies_no_check() {
     Package::new("foo", "1.0.0").publish();
@@ -2287,3 +2327,84 @@ fn sparse_install() {
 "#,
     );
 }
+
+#[cargo_test]
+fn self_referential() {
+    // Some packages build-dep on prior versions of themselves.
+    Package::new("foo", "0.0.1")
+        .file("src/lib.rs", "fn hello() {}")
+        .file("src/main.rs", "fn main() {}")
+        .file("build.rs", "fn main() {}")
+        .publish();
+    Package::new("foo", "0.0.2")
+        .file("src/lib.rs", "fn hello() {}")
+        .file("src/main.rs", "fn main() {}")
+        .file("build.rs", "fn main() {}")
+        .build_dep("foo", "0.0.1")
+        .publish();
+
+    cargo_process("install foo")
+        .with_stderr(
+            "\
+[UPDATING] `[..]` index
+[DOWNLOADING] crates ...
+[DOWNLOADED] foo v0.0.2 (registry [..])
+[INSTALLING] foo v0.0.2
+[DOWNLOADING] crates ...
+[DOWNLOADED] foo v0.0.1 (registry [..])
+[COMPILING] foo v0.0.1
+[COMPILING] foo v0.0.2
+[FINISHED] release [optimized] target(s) in [..]
+[INSTALLING] [CWD]/home/.cargo/bin/foo[EXE]
+[INSTALLED] package `foo v0.0.2` (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");
+}
+
+#[cargo_test]
+fn ambiguous_registry_vs_local_package() {
+    // Correctly install 'foo' from a local package, even if that package also
+    // depends on a registry dependency named 'foo'.
+    Package::new("foo", "0.0.1")
+        .file("src/lib.rs", "fn hello() {}")
+        .publish();
+
+    let p = project()
+        .file("src/main.rs", "fn main() {}")
+        .file(
+            "Cargo.toml",
+            r#"
+        [package]
+        name = "foo"
+        version = "0.1.0"
+        authors = []
+        edition = "2021"
+
+        [dependencies]
+        foo = "0.0.1"
+    "#,
+        )
+        .build();
+
+    cargo_process("install --path")
+        .arg(p.root())
+        .with_stderr(
+            "\
+[INSTALLING] foo v0.1.0 ([..])
+[UPDATING] `[..]` index
+[DOWNLOADING] crates ...
+[DOWNLOADED] foo v0.0.1 (registry [..])
+[COMPILING] foo v0.0.1
+[COMPILING] foo v0.1.0 ([..])
+[FINISHED] release [optimized] target(s) in [..]
+[INSTALLING] [CWD]/home/.cargo/bin/foo[EXE]
+[INSTALLED] package `foo v0.1.0 ([..])` (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");
+}