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

Ignore workspace.default-members when running cargo install on root package of a non-virtual workspace #11067

Merged
merged 1 commit into from
Jan 18, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
7 changes: 4 additions & 3 deletions src/cargo/core/compiler/build_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@ use cargo_util::ProcessBuilder;
use serde::ser;
use std::cell::RefCell;
use std::path::PathBuf;
use std::sync::Arc;
use std::thread::available_parallelism;

/// Configuration information for a rustc build.
#[derive(Debug)]
#[derive(Debug, Clone)]
pub struct BuildConfig {
/// The requested kind of compilation for this session
pub requested_kinds: Vec<CompileKind>,
Expand All @@ -33,7 +34,7 @@ pub struct BuildConfig {
pub primary_unit_rustc: Option<ProcessBuilder>,
/// A thread used by `cargo fix` to receive messages on a socket regarding
/// the success/failure of applying fixes.
pub rustfix_diagnostic_server: RefCell<Option<RustfixDiagnosticServer>>,
pub rustfix_diagnostic_server: Arc<RefCell<Option<RustfixDiagnosticServer>>>,
/// The directory to copy final artifacts to. Note that even if `out_dir` is
/// set, a copy of artifacts still could be found a `target/(debug\release)`
/// as usual.
Expand Down Expand Up @@ -100,7 +101,7 @@ impl BuildConfig {
build_plan: false,
unit_graph: false,
primary_unit_rustc: None,
rustfix_diagnostic_server: RefCell::new(None),
rustfix_diagnostic_server: Arc::new(RefCell::new(None)),
export_dir: None,
future_incompat_report: false,
timing_outputs: Vec::new(),
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/cargo_compile/compile_filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ pub enum FilterRule {
///
/// [`generate_root_units`]: super::UnitGenerator::generate_root_units
/// [`Packages`]: crate::ops::Packages
#[derive(Debug)]
#[derive(Debug, Clone)]
pub enum CompileFilter {
/// The default set of Cargo targets.
Default {
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/cargo_compile/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ pub use packages::Packages;
/// of it as `CompileOptions` are high-level settings requested on the
/// command-line, and `BuildConfig` are low-level settings for actually
/// driving `rustc`.
#[derive(Debug)]
#[derive(Debug, Clone)]
pub struct CompileOptions {
/// Configuration information for a rustc build
pub build_config: BuildConfig,
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/cargo_compile/packages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use anyhow::{bail, Context as _};
///
/// Generally, it represents the combination of all `-p` flag. When working within
/// a workspace, `--exclude` and `--workspace` flags also contribute to it.
#[derive(PartialEq, Eq, Debug)]
#[derive(PartialEq, Eq, Debug, Clone)]
pub enum Packages {
/// Packages selected by default. Usually means no flag provided.
Default,
Expand Down
25 changes: 16 additions & 9 deletions src/cargo/ops/cargo_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ use std::{env, fs};

use crate::core::compiler::{CompileKind, DefaultExecutor, Executor, UnitOutput};
use crate::core::{Dependency, Edition, Package, PackageId, Source, SourceId, Workspace};
use crate::ops::CompileFilter;
use crate::ops::{common_for_install_and_uninstall::*, FilterRule};
use crate::ops::{CompileFilter, Packages};
use crate::sources::{GitSource, PathSource, SourceConfigMap};
use crate::util::errors::CargoResult;
use crate::util::{Config, Filesystem, Rustc, ToSemver, VersionReqExt};
Expand Down Expand Up @@ -37,7 +37,7 @@ impl Drop for Transaction {

struct InstallablePackage<'cfg, 'a> {
config: &'cfg Config,
opts: &'a ops::CompileOptions,
opts: ops::CompileOptions,
root: Filesystem,
source_id: SourceId,
vers: Option<&'a str>,
Expand All @@ -60,7 +60,7 @@ impl<'cfg, 'a> InstallablePackage<'cfg, 'a> {
source_id: SourceId,
from_cwd: bool,
vers: Option<&'a str>,
opts: &'a ops::CompileOptions,
original_opts: &'a ops::CompileOptions,
force: bool,
no_track: bool,
needs_update_if_source_is_index: bool,
Expand Down Expand Up @@ -145,7 +145,7 @@ impl<'cfg, 'a> InstallablePackage<'cfg, 'a> {
dep.clone(),
&mut source,
config,
opts,
original_opts,
&root,
&dst,
force,
Expand All @@ -167,7 +167,14 @@ impl<'cfg, 'a> InstallablePackage<'cfg, 'a> {
}
};

let (ws, rustc, target) = make_ws_rustc_target(config, opts, &source_id, pkg.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();
opts.spec = Packages::Packages(vec![pkg.name().to_string()]);
Comment on lines +171 to +175
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this is the cause of #11999. Would you be interested in working on a fix for it?


let (ws, rustc, target) = make_ws_rustc_target(config, &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 Down Expand Up @@ -235,7 +242,7 @@ impl<'cfg, 'a> InstallablePackage<'cfg, 'a> {
// Check for conflicts.
ip.no_track_duplicates(&dst)?;
} else if is_installed(
&ip.pkg, config, opts, &ip.rustc, &ip.target, &ip.root, &dst, force,
&ip.pkg, config, &ip.opts, &ip.rustc, &ip.target, &ip.root, &dst, force,
)? {
let msg = format!(
"package `{}` is already installed, use --force to override",
Expand Down Expand Up @@ -297,7 +304,7 @@ impl<'cfg, 'a> InstallablePackage<'cfg, 'a> {
self.check_yanked_install()?;

let exec: Arc<dyn Executor> = Arc::new(DefaultExecutor);
let compile = ops::compile_ws(&self.ws, self.opts, &exec).with_context(|| {
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();
Expand Down Expand Up @@ -372,7 +379,7 @@ impl<'cfg, 'a> InstallablePackage<'cfg, 'a> {
&dst,
&self.pkg,
self.force,
self.opts,
&self.opts,
&self.target,
&self.rustc.verbose_version,
)?;
Expand Down Expand Up @@ -439,7 +446,7 @@ impl<'cfg, 'a> InstallablePackage<'cfg, 'a> {
&self.pkg,
&successful_bins,
self.vers.map(|s| s.to_string()),
self.opts,
&self.opts,
&self.target,
&self.rustc.verbose_version,
);
Expand Down
40 changes: 40 additions & 0 deletions tests/testsuite/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1292,6 +1292,46 @@ fn use_path_workspace() {
assert_eq!(lock, lock2, "different lockfiles");
}

#[cargo_test]
fn path_install_workspace_root_despite_default_members() {
let p = project()
.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();

p.cargo("install --path")
.arg(p.root())
.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