From e620865633c3842297fe791cc4fedbda01ca348d Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Wed, 8 Aug 2018 10:09:54 -0500 Subject: [PATCH 1/6] make `cargo install` ignore .cargo/config closes #5850 --- src/bin/cargo/commands/install.rs | 2 +- src/cargo/core/compiler/build_config.rs | 11 +++++++-- src/cargo/core/profiles.rs | 1 + src/cargo/ops/cargo_compile.rs | 7 ++++-- tests/testsuite/install.rs | 30 +++++++++++++++++++++++++ 5 files changed, 46 insertions(+), 5 deletions(-) diff --git a/src/bin/cargo/commands/install.rs b/src/bin/cargo/commands/install.rs index 94f9f8c5791..2042b957f16 100644 --- a/src/bin/cargo/commands/install.rs +++ b/src/bin/cargo/commands/install.rs @@ -74,7 +74,7 @@ continuous integration systems.", } pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { - let mut compile_opts = args.compile_options(config, CompileMode::Build)?; + let mut compile_opts = args.compile_options(config, CompileMode::Install)?; compile_opts.build_config.release = !args.is_present("debug"); let krates = args.values_of("crate") diff --git a/src/cargo/core/compiler/build_config.rs b/src/cargo/core/compiler/build_config.rs index 77ed087eee3..39c29feac82 100644 --- a/src/cargo/core/compiler/build_config.rs +++ b/src/cargo/core/compiler/build_config.rs @@ -60,8 +60,13 @@ impl BuildConfig { bail!("target was empty") } } - let cfg_target = config.get_string("build.target")?.map(|s| s.val); - let target = requested_target.clone().or(cfg_target); + let target = if mode == CompileMode::Install { + // ignore `.cargo/config` when compiling for `cargo install` + requested_target + } else { + let cfg_target = config.get_string("build.target")?.map(|s| s.val); + requested_target.clone().or(cfg_target) + }; if jobs == Some(0) { bail!("jobs must be at least 1") @@ -131,6 +136,8 @@ pub enum CompileMode { Doc { deps: bool }, /// A target that will be tested with `rustdoc`. Doctest, + // Like `Build` but we are compiling something that will be installed + Install, /// A marker for Units that represent the execution of a `build.rs` /// script. RunCustomBuild, diff --git a/src/cargo/core/profiles.rs b/src/cargo/core/profiles.rs index 2a822022af4..1056e4dd11e 100644 --- a/src/cargo/core/profiles.rs +++ b/src/cargo/core/profiles.rs @@ -84,6 +84,7 @@ impl Profiles { CompileMode::Build | CompileMode::Check { .. } | CompileMode::Doctest + | CompileMode::Install | CompileMode::RunCustomBuild => { // Note: RunCustomBuild doesn't normally use this code path. // `build_unit_profiles` normally ensures that it selects the diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index fe5623ce62a..73417a3eaca 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -386,7 +386,10 @@ impl CompileFilter { pub fn need_dev_deps(&self, mode: CompileMode) -> bool { match mode { CompileMode::Test | CompileMode::Doctest | CompileMode::Bench => true, - CompileMode::Build | CompileMode::Doc { .. } | CompileMode::Check { .. } => match *self + CompileMode::Build + | CompileMode::Doc { .. } + | CompileMode::Check { .. } + | CompileMode::Install => match *self { CompileFilter::Default { .. } => false, CompileFilter::Only { @@ -707,7 +710,7 @@ fn filter_default_targets(targets: &[Target], mode: CompileMode) -> Vec<&Target> .iter() .filter(|t| t.tested() || t.is_example()) .collect(), - CompileMode::Build | CompileMode::Check { .. } => targets + CompileMode::Build | CompileMode::Check { .. } | CompileMode::Install => targets .iter() .filter(|t| t.is_bin() || t.is_lib()) .collect(), diff --git a/tests/testsuite/install.rs b/tests/testsuite/install.rs index 0c5cb3f076f..192ff89a314 100644 --- a/tests/testsuite/install.rs +++ b/tests/testsuite/install.rs @@ -1237,3 +1237,33 @@ warning: be sure to add `[..]` to your PATH to be able to run the installed bina ", ).run(); } + +fn install_ignores_cargo_config() { + pkg("bar", "0.0.1"); + + let p = project("foo") + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + authors = [] + "#, + ) + .file( + ".cargo/config", + r#" + [build] + target = "non-existing-target" + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + assert_that( + cargo_process("install").arg("bar").cwd(p.root()), + execs().with_status(0), + ); + assert_that(cargo_home(), has_installed_exe("bar")); +} From 01421ef2572009b70a6a532dec6bfb50bee2c43c Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Wed, 8 Aug 2018 12:29:27 -0500 Subject: [PATCH 2/6] add a note to the reference --- src/doc/src/reference/config.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/doc/src/reference/config.md b/src/doc/src/reference/config.md index 1eaf4511e09..3bb4c68b3ff 100644 --- a/src/doc/src/reference/config.md +++ b/src/doc/src/reference/config.md @@ -100,7 +100,7 @@ check-revoke = true # Indicates whether SSL certs are checked for revocation jobs = 1 # number of parallel jobs, defaults to # of CPUs rustc = "rustc" # the rust compiler tool rustdoc = "rustdoc" # the doc generator tool -target = "triple" # build for the target triple +target = "triple" # build for the target triple (ignored by `cargo install`) target-dir = "target" # path of where to place all generated artifacts rustflags = ["..", ".."] # custom flags to pass to all compiler invocations incremental = true # whether or not to enable incremental compilation From e53d31ed9180fff12d001d72c50b94378e274105 Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Thu, 9 Aug 2018 13:26:11 -0500 Subject: [PATCH 3/6] fix test source --- tests/testsuite/install.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/testsuite/install.rs b/tests/testsuite/install.rs index 192ff89a314..f29a38fec26 100644 --- a/tests/testsuite/install.rs +++ b/tests/testsuite/install.rs @@ -1238,10 +1238,11 @@ warning: be sure to add `[..]` to your PATH to be able to run the installed bina ).run(); } +#[test] fn install_ignores_cargo_config() { pkg("bar", "0.0.1"); - let p = project("foo") + let p = project() .file( "Cargo.toml", r#" From 74605928147e98bff8d52916be1aadee1ec48b23 Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Thu, 9 Aug 2018 14:43:23 -0500 Subject: [PATCH 4/6] add Install to all_modes --- src/cargo/core/compiler/build_config.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/cargo/core/compiler/build_config.rs b/src/cargo/core/compiler/build_config.rs index 39c29feac82..91937408e9e 100644 --- a/src/cargo/core/compiler/build_config.rs +++ b/src/cargo/core/compiler/build_config.rs @@ -182,7 +182,7 @@ impl CompileMode { /// List of all modes (currently used by `cargo clean -p` for computing /// all possible outputs). pub fn all_modes() -> &'static [CompileMode] { - static ALL: [CompileMode; 9] = [ + static ALL: [CompileMode; 10] = [ CompileMode::Test, CompileMode::Build, CompileMode::Check { test: true }, @@ -191,6 +191,7 @@ impl CompileMode { CompileMode::Doc { deps: true }, CompileMode::Doc { deps: false }, CompileMode::Doctest, + CompileMode::Install, CompileMode::RunCustomBuild, ]; &ALL From 493919c313f1bded2a9d160cd17b91cfaecbf490 Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Sat, 1 Sep 2018 16:07:15 +0200 Subject: [PATCH 5/6] drop CompileMode::Install; override target in `exec` --- src/bin/cargo/commands/install.rs | 7 ++++++- src/cargo/core/compiler/build_config.rs | 14 +++----------- src/cargo/core/profiles.rs | 1 - src/cargo/ops/cargo_compile.rs | 7 ++----- 4 files changed, 11 insertions(+), 18 deletions(-) diff --git a/src/bin/cargo/commands/install.rs b/src/bin/cargo/commands/install.rs index 2042b957f16..6f6fd627648 100644 --- a/src/bin/cargo/commands/install.rs +++ b/src/bin/cargo/commands/install.rs @@ -74,7 +74,12 @@ continuous integration systems.", } pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { - let mut compile_opts = args.compile_options(config, CompileMode::Install)?; + let mut compile_opts = args.compile_options(config, CompileMode::Build)?; + + // for `cargo-install` we want to use what the user specified via `--target` and ignore what's + // in `.cargo/config` and what the environment says + compile_opts.build_config.requested_target = args.target(); + compile_opts.build_config.release = !args.is_present("debug"); let krates = args.values_of("crate") diff --git a/src/cargo/core/compiler/build_config.rs b/src/cargo/core/compiler/build_config.rs index 91937408e9e..77ed087eee3 100644 --- a/src/cargo/core/compiler/build_config.rs +++ b/src/cargo/core/compiler/build_config.rs @@ -60,13 +60,8 @@ impl BuildConfig { bail!("target was empty") } } - let target = if mode == CompileMode::Install { - // ignore `.cargo/config` when compiling for `cargo install` - requested_target - } else { - let cfg_target = config.get_string("build.target")?.map(|s| s.val); - requested_target.clone().or(cfg_target) - }; + let cfg_target = config.get_string("build.target")?.map(|s| s.val); + let target = requested_target.clone().or(cfg_target); if jobs == Some(0) { bail!("jobs must be at least 1") @@ -136,8 +131,6 @@ pub enum CompileMode { Doc { deps: bool }, /// A target that will be tested with `rustdoc`. Doctest, - // Like `Build` but we are compiling something that will be installed - Install, /// A marker for Units that represent the execution of a `build.rs` /// script. RunCustomBuild, @@ -182,7 +175,7 @@ impl CompileMode { /// List of all modes (currently used by `cargo clean -p` for computing /// all possible outputs). pub fn all_modes() -> &'static [CompileMode] { - static ALL: [CompileMode; 10] = [ + static ALL: [CompileMode; 9] = [ CompileMode::Test, CompileMode::Build, CompileMode::Check { test: true }, @@ -191,7 +184,6 @@ impl CompileMode { CompileMode::Doc { deps: true }, CompileMode::Doc { deps: false }, CompileMode::Doctest, - CompileMode::Install, CompileMode::RunCustomBuild, ]; &ALL diff --git a/src/cargo/core/profiles.rs b/src/cargo/core/profiles.rs index 1056e4dd11e..2a822022af4 100644 --- a/src/cargo/core/profiles.rs +++ b/src/cargo/core/profiles.rs @@ -84,7 +84,6 @@ impl Profiles { CompileMode::Build | CompileMode::Check { .. } | CompileMode::Doctest - | CompileMode::Install | CompileMode::RunCustomBuild => { // Note: RunCustomBuild doesn't normally use this code path. // `build_unit_profiles` normally ensures that it selects the diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 73417a3eaca..fe5623ce62a 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -386,10 +386,7 @@ impl CompileFilter { pub fn need_dev_deps(&self, mode: CompileMode) -> bool { match mode { CompileMode::Test | CompileMode::Doctest | CompileMode::Bench => true, - CompileMode::Build - | CompileMode::Doc { .. } - | CompileMode::Check { .. } - | CompileMode::Install => match *self + CompileMode::Build | CompileMode::Doc { .. } | CompileMode::Check { .. } => match *self { CompileFilter::Default { .. } => false, CompileFilter::Only { @@ -710,7 +707,7 @@ fn filter_default_targets(targets: &[Target], mode: CompileMode) -> Vec<&Target> .iter() .filter(|t| t.tested() || t.is_example()) .collect(), - CompileMode::Build | CompileMode::Check { .. } | CompileMode::Install => targets + CompileMode::Build | CompileMode::Check { .. } => targets .iter() .filter(|t| t.is_bin() || t.is_lib()) .collect(), From 942e3672a6038cd8a64432cca13103aef3f10aa2 Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Sat, 1 Sep 2018 18:16:49 +0200 Subject: [PATCH 6/6] fix unit test --- tests/testsuite/install.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/tests/testsuite/install.rs b/tests/testsuite/install.rs index f29a38fec26..1c3bed5a5c3 100644 --- a/tests/testsuite/install.rs +++ b/tests/testsuite/install.rs @@ -1262,9 +1262,6 @@ fn install_ignores_cargo_config() { .file("src/main.rs", "fn main() {}") .build(); - assert_that( - cargo_process("install").arg("bar").cwd(p.root()), - execs().with_status(0), - ); - assert_that(cargo_home(), has_installed_exe("bar")); + cargo_process("install bar").cwd(p.root()).with_status(0).run(); + assert_has_installed_exe(cargo_home(), "bar"); }