Skip to content

Commit

Permalink
Auto merge of #9299 - ehuss:fix-config-include, r=alexcrichton
Browse files Browse the repository at this point in the history
Fix config includes not working.

The config-include feature wasn't working because the config values were being loaded before the call to `configure`, so the unstable flag was always false.

I had to remove the unstable warning because it was a bit awkward to support, and I figure it's not that important.
  • Loading branch information
bors committed Mar 25, 2021
2 parents 58a9613 + 1130bc0 commit 56b3497
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 21 deletions.
17 changes: 12 additions & 5 deletions src/cargo/util/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -804,6 +804,15 @@ impl Config {
self.cli_config = Some(cli_config.iter().map(|s| s.to_string()).collect());
self.merge_cli_args()?;
}
if self.unstable_flags.config_include {
// If the config was already loaded (like when fetching the
// `[alias]` table), it was loaded with includes disabled because
// the `unstable_flags` hadn't been set up, yet. Any values
// fetched before this step will not process includes, but that
// should be fine (`[alias]` is one of the only things loaded
// before configure). This can be removed when stabilized.
self.reload_rooted_at(self.cwd.clone())?;
}
let extra_verbose = verbose >= 2;
let verbose = verbose != 0;

Expand Down Expand Up @@ -956,10 +965,10 @@ impl Config {
/// `seen` is used to check for cyclic includes.
fn load_includes(&self, mut value: CV, seen: &mut HashSet<PathBuf>) -> CargoResult<CV> {
// Get the list of files to load.
let (includes, def) = match &mut value {
let includes = match &mut value {
CV::Table(table, _def) => match table.remove("include") {
Some(CV::String(s, def)) => (vec![(s, def.clone())], def),
Some(CV::List(list, def)) => (list, def),
Some(CV::String(s, def)) => vec![(s, def.clone())],
Some(CV::List(list, _def)) => list,
Some(other) => bail!(
"`include` expected a string or list, but found {} in `{}`",
other.desc(),
Expand All @@ -973,8 +982,6 @@ impl Config {
};
// Check unstable.
if !self.cli_unstable().config_include {
self.shell().warn(format!("config `include` in `{}` ignored, the -Zconfig-include command-line flag is required",
def))?;
return Ok(value);
}
// Accumulate all values here.
Expand Down
78 changes: 62 additions & 16 deletions tests/testsuite/config_include.rs
Original file line number Diff line number Diff line change
@@ -1,22 +1,23 @@
//! Tests for `include` config field.

use super::config::{
assert_error, assert_match, read_output, write_config, write_config_at, ConfigBuilder,
};
use cargo_test_support::{no_such_file_err_msg, paths};
use super::config::{assert_error, write_config, write_config_at, ConfigBuilder};
use cargo_test_support::{no_such_file_err_msg, paths, project};
use std::fs;

#[cargo_test]
fn gated() {
// Requires -Z flag.
write_config("include='other'");
write_config_at(
".cargo/other",
"
othervalue = 1
",
);
let config = ConfigBuilder::new().build();
let output = read_output(config);
let expected = "\
warning: config `include` in `[..]/.cargo/config` ignored, \
the -Zconfig-include command-line flag is required
";
assert_match(expected, &output);
assert_eq!(config.get::<Option<i32>>("othervalue").unwrap(), None);
let config = ConfigBuilder::new().unstable_flag("config-include").build();
assert_eq!(config.get::<i32>("othervalue").unwrap(), 1);
}

#[cargo_test]
Expand All @@ -43,6 +44,45 @@ fn simple() {
assert_eq!(config.get::<i32>("key3").unwrap(), 4);
}

#[cargo_test]
fn works_with_cli() {
write_config_at(
".cargo/config.toml",
"
include = 'other.toml'
[build]
rustflags = ['-W', 'unused']
",
);
write_config_at(
".cargo/other.toml",
"
[build]
rustflags = ['-W', 'unsafe-code']
",
);
let p = project().file("src/lib.rs", "").build();
p.cargo("build -v")
.with_stderr(
"\
[COMPILING] foo v0.0.1 [..]
[RUNNING] `rustc [..]-W unused`
[FINISHED] [..]
",
)
.run();
p.cargo("build -v -Z config-include")
.masquerade_as_nightly_cargo()
.with_stderr(
"\
[COMPILING] foo v0.0.1 [..]
[RUNNING] `rustc [..]-W unsafe-code -W unused`
[FINISHED] [..]
",
)
.run();
}

#[cargo_test]
fn left_to_right() {
// How it merges multiple includes.
Expand Down Expand Up @@ -77,9 +117,11 @@ fn left_to_right() {
fn missing_file() {
// Error when there's a missing file.
write_config("include='missing'");
let config = ConfigBuilder::new().unstable_flag("config-include").build();
let config = ConfigBuilder::new()
.unstable_flag("config-include")
.build_err();
assert_error(
config.get::<i32>("whatever").unwrap_err(),
config.unwrap_err(),
&format!(
"\
could not load Cargo configuration
Expand All @@ -103,9 +145,11 @@ fn cycle() {
write_config_at(".cargo/config", "include='one'");
write_config_at(".cargo/one", "include='two'");
write_config_at(".cargo/two", "include='config'");
let config = ConfigBuilder::new().unstable_flag("config-include").build();
let config = ConfigBuilder::new()
.unstable_flag("config-include")
.build_err();
assert_error(
config.get::<i32>("whatever").unwrap_err(),
config.unwrap_err(),
"\
could not load Cargo configuration
Expand Down Expand Up @@ -147,9 +191,11 @@ fn cli_include() {
fn bad_format() {
// Not a valid format.
write_config("include = 1");
let config = ConfigBuilder::new().unstable_flag("config-include").build();
let config = ConfigBuilder::new()
.unstable_flag("config-include")
.build_err();
assert_error(
config.get::<i32>("whatever").unwrap_err(),
config.unwrap_err(),
"\
could not load Cargo configuration
Expand Down

0 comments on commit 56b3497

Please sign in to comment.