From 71196832513b4f2c7f90be16f740dbc6fd0a8d4e Mon Sep 17 00:00:00 2001 From: Zach Lute Date: Thu, 29 Aug 2019 00:39:15 -0700 Subject: [PATCH] Symlinking 'config' to 'config.toml' should not produce a warning. 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. --- src/cargo/util/config.rs | 25 ++++++++++--- tests/testsuite/config.rs | 77 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 6 deletions(-) diff --git a/src/cargo/util/config.rs b/src/cargo/util/config.rs index bdc344b83a0..6848733116b 100644 --- a/src/cargo/util/config.rs +++ b/src/cargo/util/config.rs @@ -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)) diff --git a/tests/testsuite/config.rs b/tests/testsuite/config.rs index 8f7549c4309..bd56e996e0a 100644 --- a/tests/testsuite/config.rs +++ b/tests/testsuite/config.rs @@ -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}; @@ -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()); @@ -132,6 +173,42 @@ f1 = 1 assert_eq!(config.get::>("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::>("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(