Skip to content

Commit

Permalink
Ignore workspace.default-members when running cargo install on ro…
Browse files Browse the repository at this point in the history
…ot package of a non-virtual workspace
  • Loading branch information
tedinski committed Jan 13, 2023
1 parent 1cd6d38 commit 7dc8be5
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 15 deletions.
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()]);

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

0 comments on commit 7dc8be5

Please sign in to comment.