Skip to content

Commit

Permalink
Symlinking 'config' to 'config.toml' should not produce a warning.
Browse files Browse the repository at this point in the history
Also true cor symlinking 'credentials.toml' to 'credentials'.

This will allow users to run multiple versions of Cargo without generating a warning if they want to use the .toml versions.

Note that Windows symlinks require special permission, so in the case the user doesn't have that permission, we don't run the symlink tests. This matches behavior in Rust libstd.
  • Loading branch information
zachlute committed Aug 29, 2019
1 parent 25e3fee commit 7119683
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 6 deletions.
25 changes: 19 additions & 6 deletions src/cargo/util/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -705,12 +705,25 @@ impl Config {

if fs::metadata(&possible).is_ok() {
if warn && fs::metadata(&possible_with_extension).is_ok() {
self.shell().warn(format!(
"Both `{}` and `{}` exist. Using `{}`",
possible.display(),
possible_with_extension.display(),
possible.display()
))?;
// We don't want to print a warning if the version
// without the extension is just a symlink to the version
// WITH an extension, which people may want to do to
// support multiple Cargo versions at once and not
// get a warning.
let skip_warning = if let Ok(target_path) = fs::read_link(&possible) {
target_path == possible_with_extension
} else {
false
};

if !skip_warning {
self.shell().warn(format!(
"Both `{}` and `{}` exist. Using `{}`",
possible.display(),
possible_with_extension.display(),
possible.display()
))?;
}
}

Ok(Some(possible))
Expand Down
77 changes: 77 additions & 0 deletions tests/testsuite/config.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
use std::borrow::Borrow;
use std::collections;
use std::fs;
use std::io;
use std::os;
use std::path::Path;

use crate::support::{paths, project};
use cargo::core::{enable_nightly_features, Shell};
Expand Down Expand Up @@ -54,6 +57,44 @@ fn write_config_toml(config: &str) {
fs::write(path, config).unwrap();
}

// Several test fail on windows if the user does not have permission to
// create symlinks (the `SeCreateSymbolicLinkPrivilege`). Instead of
// disabling these test on Windows, use this function to test whether we
// have permission, and return otherwise. This way, we still don't run these
// tests most of the time, but at least we do if the user has the right
// permissions.
// This function is derived from libstd fs tests.
pub fn got_symlink_permission() -> bool {
if cfg!(unix) {
return true;
}
let link = paths::root().join("some_hopefully_unique_link_name");
let target = paths::root().join("nonexisting_target");

match symlink_file(&target, &link) {
Ok(_) => true,
// ERROR_PRIVILEGE_NOT_HELD = 1314
Err(ref err) if err.raw_os_error() == Some(1314) => false,
Err(_) => true,
}
}

#[cfg(unix)]
fn symlink_file(target: &Path, link: &Path) -> io::Result<()> {
os::unix::fs::symlink(target, link)
}

#[cfg(windows)]
fn symlink_file(target: &Path, link: &Path) -> io::Result<()> {
os::windows::fs::symlink_file(target, link)
}

fn symlink_config_to_config_toml() {
let toml_path = paths::root().join(".cargo/config.toml");
let symlink_path = paths::root().join(".cargo/config");
t!(symlink_file(&toml_path, &symlink_path));
}

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 @@ -132,6 +173,42 @@ f1 = 1
assert_eq!(config.get::<Option<i32>>("foo.f1").unwrap(), Some(1));
}

#[cargo_test]
fn config_ambiguous_filename_symlink_doesnt_warn() {
// Windows requires special permissions to create symlinks.
// If we don't have permission, just skip this test.
if !got_symlink_permission() {
return;
};

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

symlink_config_to_config_toml();

let config = new_config(&[]);

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

// It should NOT have warned for the symlink.
drop(config); // Paranoid about flushing the file.
let path = paths::root().join("shell.out");
let output = fs::read_to_string(path).unwrap();
let unexpected = "\
warning: Both `[..]/.cargo/config` and `[..]/.cargo/config.toml` exist. Using `[..]/.cargo/config`
";
if lines_match(unexpected, &output) {
panic!(
"Found unexpected:\n{}\nActual error:\n{}\n",
unexpected, output
);
}
}

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

0 comments on commit 7119683

Please sign in to comment.