Skip to content

Commit

Permalink
high priority config first
Browse files Browse the repository at this point in the history
  • Loading branch information
@arlosi
arlosi committed Aug 16, 2023
1 parent 7c3904d commit b1522d7
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 43 deletions.
13 changes: 10 additions & 3 deletions src/cargo/util/config/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -938,6 +938,7 @@ impl Config {
.map(|s| (s.to_string(), def.clone())),
);
}
output.sort_by(|a, b| a.1.cmp(&b.1));
Ok(())
}

Expand Down Expand Up @@ -2106,16 +2107,22 @@ impl ConfigValue {

/// Merge the given value into self.
///
/// If `force` is true, primitive (non-container) types will override existing values.
/// If false, the original will be kept and the new value ignored.
/// If `force` is true, primitive (non-container) types will override existing values
/// of equal priority. For arrays, incoming values of equal priority will be placed first.
///
/// Container types (tables and arrays) are merged with existing values.
///
/// Container and non-container types cannot be mixed.
fn merge(&mut self, from: ConfigValue, force: bool) -> CargoResult<()> {
match (self, from) {
(&mut CV::List(ref mut old, _), CV::List(ref mut new, _)) => {
old.extend(mem::take(new).into_iter());
if force {
new.append(old);
mem::swap(new, old);
} else {
old.append(new);
}
old.sort_by(|a, b| a.1.cmp(&b.1));
}
(&mut CV::Table(ref mut old, _), CV::Table(ref mut new, _)) => {
for (key, value) in mem::take(new) {
Expand Down
19 changes: 19 additions & 0 deletions src/cargo/util/config/value.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use crate::util::config::Config;
use serde::de;
use std::cmp::Ordering;
use std::fmt;
use std::marker;
use std::mem;
Expand Down Expand Up @@ -63,6 +64,24 @@ pub enum Definition {
Cli(Option<PathBuf>),
}

impl PartialOrd for Definition {
fn partial_cmp(&self, other: &Definition) -> Option<Ordering> {
Some(self.cmp(other))
}
}

impl Ord for Definition {
fn cmp(&self, other: &Definition) -> Ordering {
if mem::discriminant(self) == mem::discriminant(other) {
Ordering::Equal
} else if self.is_higher_priority(other) {
Ordering::Less
} else {
Ordering::Greater
}
}
}

impl Definition {
/// Root directory where this is defined.
///
Expand Down
12 changes: 6 additions & 6 deletions tests/testsuite/cargo_config/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -280,10 +280,10 @@ target.\"cfg(target_os = \\\"linux\\\")\".runner = \"runme\" # [ROOT]/home/.carg
.with_stdout(
"\
build.rustflags = [
\"--flag-directory\", # [ROOT]/foo/.cargo/config.toml
\"--flag-global\", # [ROOT]/home/.cargo/config.toml
\"env1\", # environment variable `CARGO_BUILD_RUSTFLAGS`
\"env2\", # environment variable `CARGO_BUILD_RUSTFLAGS`
\"--flag-directory\", # [ROOT]/foo/.cargo/config.toml
\"--flag-global\", # [ROOT]/home/.cargo/config.toml
]
",
)
Expand All @@ -310,12 +310,12 @@ fn show_origin_toml_cli() {
.with_stdout(
"\
build.rustflags = [
\"--flag-directory\", # [ROOT]/foo/.cargo/config.toml
\"--flag-global\", # [ROOT]/home/.cargo/config.toml
\"cli1\", # --config cli option
\"cli2\", # --config cli option
\"env1\", # environment variable `CARGO_BUILD_RUSTFLAGS`
\"env2\", # environment variable `CARGO_BUILD_RUSTFLAGS`
\"--flag-directory\", # [ROOT]/foo/.cargo/config.toml
\"--flag-global\", # [ROOT]/home/.cargo/config.toml
]
",
)
Expand Down Expand Up @@ -471,7 +471,7 @@ fn includes() {
cargo_process("config get build.rustflags -Zunstable-options -Zconfig-include")
.cwd(&sub_folder.parent().unwrap())
.masquerade_as_nightly_cargo(&["cargo-config", "config-include"])
.with_stdout(r#"build.rustflags = ["--flag-other", "--flag-directory", "--flag-global"]"#)
.with_stdout(r#"build.rustflags = ["--flag-directory", "--flag-other", "--flag-global"]"#)
.with_stderr("")
.run();

Expand All @@ -481,8 +481,8 @@ fn includes() {
.with_stdout(
"\
build.rustflags = [
\"--flag-other\", # [ROOT]/foo/.cargo/other.toml
\"--flag-directory\", # [ROOT]/foo/.cargo/config.toml
\"--flag-other\", # [ROOT]/foo/.cargo/other.toml
\"--flag-global\", # [ROOT]/home/.cargo/config.toml
]
",
Expand Down
40 changes: 20 additions & 20 deletions tests/testsuite/config.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,7 @@ c = ['c']
);
assert_eq!(
config.get::<VSOB>("c").unwrap(),
VSOB::VecString(vec!["c".to_string(), "d".to_string()])
VSOB::VecString(vec!["d".to_string(), "c".to_string()])
);
assert_eq!(config.get::<VSOB>("envb").unwrap(), VSOB::Bool(false));
assert_eq!(
Expand All @@ -508,11 +508,11 @@ Caused by:
.build();
assert_eq!(
config.get::<VSOB>("b").unwrap(),
VSOB::VecString(vec!["b".to_string(), "d".to_string(), "e".to_string()])
VSOB::VecString(vec!["d".to_string(), "e".to_string(), "b".to_string()])
);
assert_eq!(
config.get::<VSOB>("c").unwrap(),
VSOB::VecString(vec!["c".to_string(), "f".to_string(), "g".to_string()])
VSOB::VecString(vec!["f".to_string(), "g".to_string(), "c".to_string()])
);

// config-cli
Expand Down Expand Up @@ -540,19 +540,19 @@ expected boolean, but found array",
assert_eq!(
config.get::<VSOB>("b").unwrap(),
VSOB::VecString(vec![
"b".to_string(),
"clib".to_string(),
"env1".to_string(),
"env2".to_string()
"env2".to_string(),
"b".to_string(),
])
);
assert_eq!(
config.get::<VSOB>("c").unwrap(),
VSOB::VecString(vec![
"c".to_string(),
"clic".to_string(),
"e1".to_string(),
"e2".to_string()
"e2".to_string(),
"c".to_string(),
])
);
}
Expand Down Expand Up @@ -780,7 +780,7 @@ expected a list, but found a integer for `l3` in [..]/.cargo/config",
);
assert_eq!(
config.get::<L>("l4").unwrap(),
vec!["one", "two", "three", "four"]
vec!["three", "four", "one", "two"]
);
assert_eq!(config.get::<L>("l5").unwrap(), vec!["a"]);
assert_eq!(config.get::<L>("env-empty").unwrap(), vec![] as Vec<String>);
Expand Down Expand Up @@ -814,10 +814,10 @@ expected `]`
.get::<(String, String, String, String)>("l4")
.unwrap(),
(
"one".to_string(),
"two".to_string(),
"three".to_string(),
"four".to_string()
"four".to_string(),
"one".to_string(),
"two".to_string()
)
);
assert_eq!(config.get::<(String,)>("l5").unwrap(), ("a".to_string(),));
Expand Down Expand Up @@ -845,7 +845,7 @@ expected `]`
assert_eq!(
config.get::<S>("nested2").unwrap(),
S {
l: Some(vec!["y".to_string(), "z".to_string()]),
l: Some(vec!["z".to_string(), "y".to_string()]),
}
);
assert_eq!(
Expand Down Expand Up @@ -1582,17 +1582,17 @@ known-hosts = [
.as_ref()
.unwrap();
assert_eq!(kh.len(), 4);
assert_eq!(kh[0].val, "example.org ...");
assert_eq!(kh[0].definition, Definition::Path(foo_path.clone()));
assert_eq!(kh[1].val, "example.com ...");
assert_eq!(kh[1].definition, Definition::Path(root_path.clone()));
assert_eq!(kh[2].val, "example.net ...");
assert_eq!(kh[2].definition, Definition::Path(root_path.clone()));
assert_eq!(kh[3].val, "env-example");
assert_eq!(kh[0].val, "env-example");
assert_eq!(
kh[3].definition,
kh[0].definition,
Definition::Environment("CARGO_NET_SSH_KNOWN_HOSTS".to_string())
);
assert_eq!(kh[1].val, "example.org ...");
assert_eq!(kh[1].definition, Definition::Path(foo_path.clone()));
assert_eq!(kh[2].val, "example.com ...");
assert_eq!(kh[2].definition, Definition::Path(root_path.clone()));
assert_eq!(kh[3].val, "example.net ...");
assert_eq!(kh[3].definition, Definition::Path(root_path.clone()));
}

#[cargo_test]
Expand Down
25 changes: 12 additions & 13 deletions tests/testsuite/config_cli.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,19 +135,18 @@ fn merges_array() {
.build();
assert_eq!(
config.get::<Vec<String>>("build.rustflags").unwrap(),
["--file", "--cli"]
["--cli", "--file"]
);

// With normal env.
let config = ConfigBuilder::new()
.env("CARGO_BUILD_RUSTFLAGS", "--env1 --env2")
.config_arg("build.rustflags = ['--cli']")
.build();
// The order of cli/env is a little questionable here, but would require
// much more complex merging logic.

assert_eq!(
config.get::<Vec<String>>("build.rustflags").unwrap(),
["--file", "--cli", "--env1", "--env2"]
["--cli", "--env1", "--env2", "--file"]
);

// With advanced-env.
Expand All @@ -158,7 +157,7 @@ fn merges_array() {
.build();
assert_eq!(
config.get::<Vec<String>>("build.rustflags").unwrap(),
["--file", "--cli", "--env"]
["--cli", "--env", "--file"]
);

// Merges multiple instances.
Expand All @@ -168,7 +167,7 @@ fn merges_array() {
.build();
assert_eq!(
config.get::<Vec<String>>("build.rustflags").unwrap(),
["--file", "--one", "--two"]
["--two", "--one", "--file"]
);
}

Expand All @@ -189,7 +188,7 @@ fn string_list_array() {
.get::<cargo::util::config::StringList>("build.rustflags")
.unwrap()
.as_slice(),
["--file", "--cli"]
["--cli", "--file"]
);

// With normal env.
Expand All @@ -202,7 +201,7 @@ fn string_list_array() {
.get::<cargo::util::config::StringList>("build.rustflags")
.unwrap()
.as_slice(),
["--file", "--cli", "--env1", "--env2"]
["--cli", "--env1", "--env2", "--file"]
);

// With advanced-env.
Expand All @@ -216,7 +215,7 @@ fn string_list_array() {
.get::<cargo::util::config::StringList>("build.rustflags")
.unwrap()
.as_slice(),
["--file", "--cli", "--env"]
["--cli", "--env", "--file"]
);
}

Expand Down Expand Up @@ -278,10 +277,10 @@ fn merge_array_mixed_def_paths() {
// The definition for the root value is somewhat arbitrary, but currently starts with the file because that is what is loaded first.
assert_eq!(paths.definition, Definition::Path(paths::root()));
assert_eq!(paths.val.len(), 2);
assert_eq!(paths.val[0].0, "file");
assert_eq!(paths.val[0].1.root(&config), paths::root());
assert_eq!(paths.val[1].0, "cli");
assert_eq!(paths.val[1].1.root(&config), somedir);
assert_eq!(paths.val[0].0, "cli");
assert_eq!(paths.val[0].1.root(&config), somedir);
assert_eq!(paths.val[1].0, "file");
assert_eq!(paths.val[1].1.root(&config), paths::root());
}

#[cargo_test]
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/config_include.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ fn works_with_cli() {
"\
[DIRTY] foo v0.0.1 ([..]): the rustflags changed
[CHECKING] foo v0.0.1 [..]
[RUNNING] `rustc [..]-W unsafe-code -W unused`
[RUNNING] `rustc [..]-W unused -W unsafe-code`
[FINISHED] [..]
",
)
Expand Down

0 comments on commit b1522d7

Please sign in to comment.