Skip to content

Commit

Permalink
high priority config last
Browse files Browse the repository at this point in the history
  • Loading branch information
@arlosi
arlosi committed Aug 16, 2023
1 parent 7c3904d commit c611fed
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 29 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 later.
///
/// 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 {
old.append(new);
} else {
new.append(old);
mem::swap(new, old);
}
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::Greater
} else {
Ordering::Less
}
}
}

impl Definition {
/// Root directory where this is defined.
///
Expand Down
22 changes: 11 additions & 11 deletions tests/testsuite/cargo_config/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ fn get_toml() {
alias.foo = \"abc --xyz\"
alias.sub-example = [\"sub\", \"example\"]
build.jobs = 99
build.rustflags = [\"--flag-directory\", \"--flag-global\"]
build.rustflags = [\"--flag-global\", \"--flag-directory\"]
extra-table.somekey = \"somevalue\"
profile.dev.opt-level = 3
profile.dev.package.foo.opt-level = 1
Expand All @@ -111,7 +111,7 @@ target.\"cfg(target_os = \\\"linux\\\")\".runner = \"runme\"
cargo_process("config get build.rustflags -Zunstable-options")
.cwd(&sub_folder.parent().unwrap())
.masquerade_as_nightly_cargo(&["cargo-config"])
.with_stdout("build.rustflags = [\"--flag-directory\", \"--flag-global\"]")
.with_stdout("build.rustflags = [\"--flag-global\", \"--flag-directory\"]")
.with_stderr("")
.run();

Expand Down Expand Up @@ -171,8 +171,8 @@ fn get_json() {
"build": {
"jobs": 99,
"rustflags": [
"--flag-directory",
"--flag-global"
"--flag-global",
"--flag-directory"
]
},
"extra-table": {
Expand Down Expand Up @@ -259,8 +259,8 @@ alias.sub-example = [
]
build.jobs = 99 # [ROOT]/home/.cargo/config.toml
build.rustflags = [
\"--flag-directory\", # [ROOT]/foo/.cargo/config.toml
\"--flag-global\", # [ROOT]/home/.cargo/config.toml
\"--flag-directory\", # [ROOT]/foo/.cargo/config.toml
]
extra-table.somekey = \"somevalue\" # [ROOT]/home/.cargo/config.toml
profile.dev.opt-level = 3 # [ROOT]/home/.cargo/config.toml
Expand All @@ -280,8 +280,8 @@ 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
\"--flag-directory\", # [ROOT]/foo/.cargo/config.toml
\"env1\", # environment variable `CARGO_BUILD_RUSTFLAGS`
\"env2\", # environment variable `CARGO_BUILD_RUSTFLAGS`
]
Expand Down Expand Up @@ -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
\"--flag-directory\", # [ROOT]/foo/.cargo/config.toml
\"env1\", # environment variable `CARGO_BUILD_RUSTFLAGS`
\"env2\", # environment variable `CARGO_BUILD_RUSTFLAGS`
\"cli1\", # --config cli option
\"cli2\", # --config cli option
]
",
)
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-global", "--flag-other", "--flag-directory"]"#)
.with_stderr("")
.run();

Expand All @@ -481,9 +481,9 @@ fn includes() {
.with_stdout(
"\
build.rustflags = [
\"--flag-global\", # [ROOT]/home/.cargo/config.toml
\"--flag-other\", # [ROOT]/foo/.cargo/other.toml
\"--flag-directory\", # [ROOT]/foo/.cargo/config.toml
\"--flag-global\", # [ROOT]/home/.cargo/config.toml
]
",
)
Expand Down
18 changes: 9 additions & 9 deletions tests/testsuite/config.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -541,18 +541,18 @@ expected boolean, but found array",
config.get::<VSOB>("b").unwrap(),
VSOB::VecString(vec![
"b".to_string(),
"clib".to_string(),
"env1".to_string(),
"env2".to_string()
"env2".to_string(),
"clib".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(),
"clic".to_string(),
])
);
}
Expand Down Expand Up @@ -1582,12 +1582,12 @@ 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[0].val, "example.com ...");
assert_eq!(kh[0].definition, Definition::Path(root_path.clone()));
assert_eq!(kh[1].val, "example.net ...");
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[2].val, "example.org ...");
assert_eq!(kh[2].definition, Definition::Path(foo_path.clone()));
assert_eq!(kh[3].val, "env-example");
assert_eq!(
kh[3].definition,
Expand Down
10 changes: 4 additions & 6 deletions tests/testsuite/config_cli.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,11 +143,9 @@ fn merges_array() {
.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"]
["--file", "--env1", "--env2", "--cli"]
);

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

// Merges multiple instances.
Expand Down Expand Up @@ -202,7 +200,7 @@ fn string_list_array() {
.get::<cargo::util::config::StringList>("build.rustflags")
.unwrap()
.as_slice(),
["--file", "--cli", "--env1", "--env2"]
["--file", "--env1", "--env2", "--cli"]
);

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

Expand Down

0 comments on commit c611fed

Please sign in to comment.