Skip to content

Commit

Permalink
Allow using 'config.toml' instead of just 'config' files.
Browse files Browse the repository at this point in the history
Note that this change only makes 'config.toml' optional to use instead of 'config'. If both exist, we will print a warning and prefer 'config', since that would be the existing behavior if both existed.

We should also consider a separate change to make config.toml the default and update docs, etc.
  • Loading branch information
zachlute committed Aug 24, 2019
1 parent 475a23e commit c530720
Show file tree
Hide file tree
Showing 2 changed files with 113 additions and 26 deletions.
82 changes: 56 additions & 26 deletions src/cargo/util/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -662,7 +662,7 @@ impl Config {
let mut cfg = CV::Table(HashMap::new(), PathBuf::from("."));
let home = self.home_path.clone().into_path_unlocked();

walk_tree(path, &home, |path| {
self.walk_tree(path, &home, |path| {
let mut contents = String::new();
let mut file = File::open(&path)?;
file.read_to_string(&mut contents)
Expand All @@ -689,6 +689,61 @@ impl Config {
}
}

fn walk_tree<F>(&self, pwd: &Path, home: &Path, mut walk: F) -> CargoResult<()>
where
F: FnMut(&Path) -> CargoResult<()>,
{
let mut stash: HashSet<PathBuf> = HashSet::new();

for current in paths::ancestors(pwd) {
let possible = current.join(".cargo").join("config");
let possible_with_extension = current.join(".cargo").join("config.toml");

// If both 'config' and 'config.toml' exist, we should use 'config'
// for backward compatibility, but we should warn the user.
if fs::metadata(&possible).is_ok() {
if fs::metadata(&possible_with_extension).is_ok() {
self.shell().warn(format!(
"Both `{}` and `{}` exist. Using `{}`",
possible.display(),
possible_with_extension.display(),
possible.display()
))?;
}

walk(&possible)?;
stash.insert(possible);
} else if fs::metadata(&possible_with_extension).is_ok() {
walk(&possible_with_extension)?;
stash.insert(possible);
}
}

// Once we're done, also be sure to walk the home directory even if it's not
// in our history to be sure we pick up that standard location for
// information.
let config = home.join("config");
let config_with_extension = home.join("config.toml");
if !stash.contains(&config) && fs::metadata(&config).is_ok() {
if fs::metadata(&config_with_extension).is_ok() {
self.shell().warn(format!(
"Both `{}` and `{}` exist. Using `{}`",
config.display(),
config_with_extension.display(),
config.display()
))?;
}

walk(&config)?;
} else if !stash.contains(&config_with_extension)
&& fs::metadata(&config_with_extension).is_ok()
{
walk(&config_with_extension)?;
}

Ok(())
}

/// Gets the index for a registry.
pub fn get_registry_index(&self, registry: &str) -> CargoResult<Url> {
validate_package_name(registry, "registry name", "")?;
Expand Down Expand Up @@ -1673,31 +1728,6 @@ pub fn homedir(cwd: &Path) -> Option<PathBuf> {
::home::cargo_home_with_cwd(cwd).ok()
}

fn walk_tree<F>(pwd: &Path, home: &Path, mut walk: F) -> CargoResult<()>
where
F: FnMut(&Path) -> CargoResult<()>,
{
let mut stash: HashSet<PathBuf> = HashSet::new();

for current in paths::ancestors(pwd) {
let possible = current.join(".cargo").join("config");
if fs::metadata(&possible).is_ok() {
walk(&possible)?;
stash.insert(possible);
}
}

// Once we're done, also be sure to walk the home directory even if it's not
// in our history to be sure we pick up that standard location for
// information.
let config = home.join("config");
if !stash.contains(&config) && fs::metadata(&config).is_ok() {
walk(&config)?;
}

Ok(())
}

pub fn save_credentials(cfg: &Config, token: String, registry: Option<String>) -> CargoResult<()> {
let mut file = {
cfg.home_path.create_dir()?;
Expand Down
57 changes: 57 additions & 0 deletions tests/testsuite/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,12 @@ fn write_config(config: &str) {
fs::write(path, config).unwrap();
}

fn write_config_toml(config: &str) {
let path = paths::root().join(".cargo/config.toml");
fs::create_dir_all(path.parent().unwrap()).unwrap();
fs::write(path, config).unwrap();
}

fn new_config(env: &[(&str, &str)]) -> Config {
enable_nightly_features(); // -Z advanced-env
let output = Box::new(fs::File::create(paths::root().join("shell.out")).unwrap());
Expand Down Expand Up @@ -112,6 +118,57 @@ f1 = 123
assert_eq!(s, S { f1: Some(456) });
}

#[cargo_test]
fn config_works_with_extension() {
write_config_toml(
"\
[foo]
f1 = 1
",
);

let config = new_config(&[]);

assert_eq!(config.get::<Option<i32>>("foo.f1").unwrap(), Some(1));
}

#[cargo_test]
fn config_ambiguous_filename() {
write_config(
"\
[foo]
f1 = 1
",
);

write_config_toml(
"\
[foo]
f1 = 2
",
);

let config = new_config(&[]);

// It should use the value from the one without the extension for
// backwards compatibility.
assert_eq!(config.get::<Option<i32>>("foo.f1").unwrap(), Some(1));

// But it also should have warned.
drop(config); // Paranoid about flushing the file.
let path = paths::root().join("shell.out");
let output = fs::read_to_string(path).unwrap();
let expected = "\
warning: Both `[..]/.cargo/config` and `[..]/.cargo/config.toml` exist. Using `[..]/.cargo/config`
";
if !lines_match(expected, &output) {
panic!(
"Did not find expected:\n{}\nActual error:\n{}\n",
expected, output
);
}
}

#[cargo_test]
fn config_unused_fields() {
write_config(
Expand Down

0 comments on commit c530720

Please sign in to comment.