Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build by PackageIdSpec, not name, to avoid ambiguity #12015

Merged
merged 4 commits into from
May 3, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 16 additions & 9 deletions src/cargo/ops/cargo_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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() {
Expand All @@ -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(
Expand Down
121 changes: 121 additions & 0 deletions tests/testsuite/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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_workspace_package() {
// Correctly install 'foo' from a workspace, even if that workspace
weihanglo marked this conversation as resolved.
Show resolved Hide resolved
// (somewhere) also depends on a registry package 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");
}