From d3ffb1824185a8f1474bc581da6182b3feaace99 Mon Sep 17 00:00:00 2001 From: Simon Ochsenreither Date: Tue, 13 Mar 2018 10:33:57 +0100 Subject: [PATCH] Add support for platform-defined standard directories This change stops cargo from violating the operating system rules regarding the placement of config, cache, ... directories on Linux, macOS and Windows. Existing directories and overrides are retained. The precedence is as follows: 1) use the `CARGO_HOME` environment variable if it exists (legacy) 2) use `CARGO_CACHE_DIR`, `CARGO_CONFIG_DIR` etc. env vars if they exist 3) use the ~/.cargo directory if it exists (legacy) 4) follow operating system standards A new cargo command, `dirs`, is added, which can provide path information to other command line tools. Fixes: https://github.com/rust-lang/cargo/issues/1734 https://github.com/rust-lang/cargo/issues/1976 https://github.com/rust-lang/rust/issues/12725 Addresses: https://github.com/rust-lang/rfcs/pull/1615 https://github.com/rust-lang/cargo/pull/148, https://github.com/rust-lang/cargo/pull/3981 --- Cargo.toml | 1 + appveyor.yml | 1 + src/bin/cargo.rs | 2 +- src/bin/commands/dirs.rs | 16 +++ src/bin/commands/mod.rs | 3 + src/cargo/core/workspace.rs | 2 +- src/cargo/lib.rs | 1 + src/cargo/ops/cargo_install.rs | 64 ++++++++--- src/cargo/util/config.rs | 195 ++++++++++++++++++++++++--------- src/cargo/util/mod.rs | 2 +- tests/testsuite/login.rs | 6 +- 11 files changed, 219 insertions(+), 74 deletions(-) create mode 100644 src/bin/commands/dirs.rs diff --git a/Cargo.toml b/Cargo.toml index 6a37ad3e77d..ee624d7f9e3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,6 +22,7 @@ crates-io = { path = "src/crates-io", version = "0.16" } crossbeam = "0.3" crypto-hash = "0.3" curl = "0.4.6" +directories = "0.8.5" env_logger = "0.5" failure = "0.1.1" filetime = "0.1" diff --git a/appveyor.yml b/appveyor.yml index d901dbe1860..6be0cad6dd6 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -18,4 +18,5 @@ clone_depth: 1 build: false test_script: + - set RUST_BACKTRACE=1 - cargo test diff --git a/src/bin/cargo.rs b/src/bin/cargo.rs index 7d7c7db6e4c..5d014bdad9c 100644 --- a/src/bin/cargo.rs +++ b/src/bin/cargo.rs @@ -174,7 +174,7 @@ fn is_executable>(path: P) -> bool { } fn search_directories(config: &Config) -> Vec { - let mut dirs = vec![config.home().clone().into_path_unlocked().join("bin")]; + let mut dirs = vec![config.bin_path()]; if let Some(val) = env::var_os("PATH") { dirs.extend(env::split_paths(&val)); } diff --git a/src/bin/commands/dirs.rs b/src/bin/commands/dirs.rs new file mode 100644 index 00000000000..111b7bb92d0 --- /dev/null +++ b/src/bin/commands/dirs.rs @@ -0,0 +1,16 @@ +use command_prelude::*; + +pub fn cli() -> App { + subcommand("dirs") + .about("Display directories (cache, config, ...) used by cargo") + .after_help("\ +") +} + +pub fn exec(config: &mut Config, _args: &ArgMatches) -> CliResult { + println!("CARGO_CACHE_DIR: {:?}", config.cache_path().into_path_unlocked()); + println!("CARGO_CONFIG_DIR: {:?}", config.config_path().into_path_unlocked()); + println!("CARGO_DATA_DIR: {:?}", config.data_path()); + println!("CARGO_BIN_DIR: {:?}", config.bin_path()); + Ok(()) +} diff --git a/src/bin/commands/mod.rs b/src/bin/commands/mod.rs index fc829a85543..85773ca246e 100644 --- a/src/bin/commands/mod.rs +++ b/src/bin/commands/mod.rs @@ -6,6 +6,7 @@ pub fn builtin() -> Vec { build::cli(), check::cli(), clean::cli(), + dirs::cli(), doc::cli(), fetch::cli(), generate_lockfile::cli(), @@ -40,6 +41,7 @@ pub fn builtin_exec(cmd: &str) -> Option CliResu "build" => build::exec, "check" => check::exec, "clean" => clean::exec, + "dirs" => dirs::exec, "doc" => doc::exec, "fetch" => fetch::exec, "generate-lockfile" => generate_lockfile::exec, @@ -74,6 +76,7 @@ pub mod bench; pub mod build; pub mod check; pub mod clean; +pub mod dirs; pub mod doc; pub mod fetch; pub mod generate_lockfile; diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index 7bb9be948b0..69a009ca848 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -383,7 +383,7 @@ impl<'cfg> Workspace<'cfg> { // `CARGO_HOME` pointing inside of the workspace root or in the // current project, but we don't want to mistakenly try to put // crates.io crates into the workspace by accident. - if self.config.home() == path { + if &self.config.cache_path() == path { break; } } diff --git a/src/cargo/lib.rs b/src/cargo/lib.rs index 07b2cd408f1..58d52478299 100644 --- a/src/cargo/lib.rs +++ b/src/cargo/lib.rs @@ -11,6 +11,7 @@ extern crate core_foundation; extern crate crates_io as registry; extern crate crossbeam; extern crate curl; +extern crate directories; #[macro_use] extern crate failure; extern crate filetime; diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index 232c804e8f5..8bbb5ffd571 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -53,6 +53,27 @@ impl Drop for Transaction { } } +#[derive(Clone)] +struct CargoInstallDirs { + config_dir: PathBuf, + bin_dir: PathBuf, +} + +impl CargoInstallDirs { + fn from_root(root: PathBuf) -> CargoInstallDirs { + CargoInstallDirs { + bin_dir: root.join("bin"), + config_dir: root, + } + } + fn from_config(config: &Config) -> CargoInstallDirs { + CargoInstallDirs { + bin_dir: config.bin_path(), + config_dir: config.config_path().into_path_unlocked(), + } + } +} + pub fn install( root: Option<&str>, krates: Vec<&str>, @@ -61,7 +82,7 @@ pub fn install( opts: &ops::CompileOptions, force: bool, ) -> CargoResult<()> { - let root = resolve_root(root, opts.config)?; + let root = resolve_install_dirs(root, opts.config)?; let map = SourceConfigMap::new(opts.config)?; let (installed_anything, scheduled_error) = if krates.len() <= 1 { @@ -122,7 +143,7 @@ pub fn install( if installed_anything { // Print a warning that if this directory isn't in PATH that they won't be // able to run these commands. - let dst = metadata(opts.config, &root)?.parent().join("bin"); + let dst = root.bin_dir; let path = env::var_os("PATH").unwrap_or_default(); for path in env::split_paths(&path) { if path == dst { @@ -145,7 +166,7 @@ pub fn install( } fn install_one( - root: &Filesystem, + dirs: &CargoInstallDirs, map: &SourceConfigMap, krate: Option<&str>, source_id: &SourceId, @@ -235,9 +256,9 @@ fn install_one( // We have to check this again afterwards, but may as well avoid building // anything if we're gonna throw it away anyway. { - let metadata = metadata(config, root)?; + let metadata = metadata(config, &Filesystem::new(dirs.config_dir.clone()))?; let list = read_crate_list(&metadata)?; - let dst = metadata.parent().join("bin"); + let dst = config.bin_path(); check_overwrites(&dst, pkg, &opts.filter, &list, force)?; } @@ -274,7 +295,7 @@ fn install_one( ); } - let metadata = metadata(config, root)?; + let metadata = metadata(config, &Filesystem::new(dirs.config_dir.clone()))?; let mut list = read_crate_list(&metadata)?; let dst = metadata.parent().join("bin"); let duplicates = check_overwrites(&dst, pkg, &opts.filter, &list, force)?; @@ -655,8 +676,8 @@ fn write_crate_list(file: &FileLock, listing: CrateListingV1) -> CargoResult<()> } pub fn install_list(dst: Option<&str>, config: &Config) -> CargoResult<()> { - let dst = resolve_root(dst, config)?; - let dst = metadata(config, &dst)?; + let dst = resolve_install_dirs(dst, config)?; + let dst = metadata(config, &Filesystem::new(dst.config_dir))?; let list = read_crate_list(&dst)?; for (k, v) in list.v1.iter() { println!("{}:", k); @@ -677,7 +698,7 @@ pub fn uninstall( bail!("A binary can only be associated with a single installed package, specifying multiple specs with --bin is redundant."); } - let root = resolve_root(root, config)?; + let root = resolve_install_dirs(root, config)?; let scheduled_error = if specs.len() == 1 { uninstall_one(&root, specs[0], bins, config)?; false @@ -723,13 +744,13 @@ pub fn uninstall( Ok(()) } -pub fn uninstall_one( - root: &Filesystem, +fn uninstall_one( + dirs: &CargoInstallDirs, spec: &str, bins: &[String], config: &Config, ) -> CargoResult<()> { - let crate_metadata = metadata(config, root)?; + let crate_metadata = metadata(config, &Filesystem::new(dirs.config_dir.clone()))?; let mut metadata = read_crate_list(&crate_metadata)?; let mut to_remove = Vec::new(); { @@ -738,7 +759,7 @@ pub fn uninstall_one( Entry::Occupied(e) => e, Entry::Vacant(..) => panic!("entry not found: {}", result), }; - let dst = crate_metadata.parent().join("bin"); + let dst = &dirs.bin_dir; for bin in installed.get() { let bin = dst.join(bin); if fs::metadata(&bin).is_err() { @@ -787,15 +808,22 @@ pub fn uninstall_one( Ok(()) } +/// Return a file lock for the .crates.toml file at the given root. +/// The config argument is only used for logging to the shell. fn metadata(config: &Config, root: &Filesystem) -> CargoResult { root.open_rw(Path::new(".crates.toml"), config, "crate metadata") } -fn resolve_root(flag: Option<&str>, config: &Config) -> CargoResult { +// Determine cargo directories by first checking whether an argument was given +// on the command line, if not checking whether the environment variable +// CARGO_INSTALL_ROOT is set, and if not using the paths of the configuration. +fn resolve_install_dirs( + root: Option<&str>, + config: &Config, +) -> CargoResult { let config_root = config.get_path("install.root")?; - Ok(flag.map(PathBuf::from) + Ok(root.map(PathBuf::from) .or_else(|| env::var_os("CARGO_INSTALL_ROOT").map(PathBuf::from)) - .or_else(move || config_root.map(|v| v.val)) - .map(Filesystem::new) - .unwrap_or_else(|| config.home().clone())) + .or_else(move || config_root.map(|v| v.val)).map(CargoInstallDirs::from_root) + .unwrap_or_else(|| CargoInstallDirs::from_config(config))) } diff --git a/src/cargo/util/config.rs b/src/cargo/util/config.rs index 281165ec209..0b5313ab4b5 100644 --- a/src/cargo/util/config.rs +++ b/src/cargo/util/config.rs @@ -14,6 +14,7 @@ use std::sync::{Once, ONCE_INIT}; use std::time::Instant; use curl::easy::Easy; +use directories::ProjectDirs; use jobserver; use serde::{Serialize, Serializer}; use toml; @@ -32,22 +33,125 @@ use util::Filesystem; use self::ConfigValue as CV; +#[derive(Clone, Debug)] +pub struct CargoDirs { + pub current_dir: PathBuf, + pub cache_dir: Filesystem, + pub config_dir: Filesystem, + pub data_dir: PathBuf, + pub bin_dir: PathBuf, +} + +impl CargoDirs { + /// Computes the paths to directories used by cargo to retrieve and store + /// cache, config, ... files. + // This is written in the most straight-forward way possible, because it is + // hard as-is to understand all the different options, without trying to + // save lines of code. + pub fn new() -> CargoResult { + let current_dir = env::current_dir().chain_err(|| { + "couldn't get the current directory of the process" + })?; + + let mut cache_dir = PathBuf::default(); + let mut config_dir = PathBuf::default(); + let mut data_dir = PathBuf::default(); + let mut bin_dir = PathBuf::default(); + + // 1. CARGO_HOME set + let cargo_home_env = env::var_os("CARGO_HOME").map(|home| current_dir.join(home)); + if let Some(cargo_home) = cargo_home_env.clone() { + cache_dir = cargo_home.clone(); + config_dir = cargo_home.clone(); + data_dir = cargo_home.clone(); + bin_dir = cargo_home.join("bin"); + } + // 2. CARGO_CACHE_DIR, CARGO_CONFIG_DIR, CARGO_BIN_DIR, ... set + let cargo_cache_env = env::var_os("CARGO_CACHE_DIR").map(|home| current_dir.join(home)); + let cargo_config_env = env::var_os("CARGO_CONFIG_DIR").map(|home| current_dir.join(home)); + let cargo_data_env = env::var_os("CARGO_DATA_DIR").map(|home| current_dir.join(home)); + let cargo_bin_env = env::var_os("CARGO_BIN_DIR").map(|home| current_dir.join(home)); + if let Some(cargo_cache) = cargo_cache_env.clone() { + cache_dir = cargo_cache.clone(); + } + if let Some(cargo_config) = cargo_config_env.clone() { + config_dir = cargo_config.clone(); + } + if let Some(cargo_data) = cargo_data_env.clone() { + data_dir = cargo_data.clone(); + } + if let Some(cargo_bin) = cargo_bin_env.clone() { + bin_dir = cargo_bin.clone(); + } + + // no env vars are set ... + if cargo_home_env.is_none() + && cargo_cache_env.is_none() + && cargo_config_env.is_none() + && cargo_data_env.is_none() + && cargo_bin_env.is_none() { + let home_dir = ::home::home_dir().ok_or_else(|| { + format_err!("Cargo couldn't find your home directory. \ + This probably means that $HOME was not set.") + })?; + let legacy_cargo_dir = home_dir.join(".cargo"); + // 3. ... and .cargo exist + if legacy_cargo_dir.exists() { + cache_dir = legacy_cargo_dir.clone(); + config_dir = legacy_cargo_dir.clone(); + data_dir = legacy_cargo_dir.clone(); + bin_dir = legacy_cargo_dir.join("bin"); + // 4. ... otherwise follow platform conventions + } else { + let cargo_dirs = ProjectDirs::from("", "", "Cargo"); + cache_dir = cargo_dirs.cache_dir().to_path_buf(); + config_dir = cargo_dirs.config_dir().to_path_buf(); + data_dir = cargo_dirs.data_dir().to_path_buf(); + bin_dir = CargoDirs::find_bin_dir(&cargo_dirs).ok_or_else(|| { + format_err!("couldn't find the directory in which executables are placed") + })?.to_path_buf(); + } + } + + Ok(CargoDirs { + current_dir: current_dir, + cache_dir: Filesystem::new(cache_dir), + config_dir: Filesystem::new(config_dir), + data_dir: data_dir, + bin_dir: bin_dir, + }) + } + + #[cfg(target_os = "linux")] + fn find_bin_dir(_dirs: &ProjectDirs) -> Option { + use directories::BaseDirs; + let base_dir = BaseDirs::new(); + base_dir.executable_dir().map(|p| p.to_path_buf()) + } + #[cfg(target_os = "macos")] + fn find_bin_dir(dirs: &ProjectDirs) -> Option { + dirs.data_dir().parent().map(|p| p.join("bin")).map(|p| p.to_path_buf()) + } + #[cfg(target_os = "windows")] + fn find_bin_dir(dirs: &ProjectDirs) -> Option { + dirs.data_dir().parent().map(|p| p.join("bin")).map(|p| p.to_path_buf()) + } +} + /// Configuration information for cargo. This is not specific to a build, it is information /// relating to cargo itself. /// /// This struct implements `Default`: all fields can be inferred. #[derive(Debug)] pub struct Config { - /// The location of the users's 'home' directory. OS-dependent. - home_path: Filesystem, + /// The location of directories crucial for cargo. OS-dependent. + dirs: CargoDirs, /// Information about how to write messages to the shell shell: RefCell, /// Information on how to invoke the compiler (rustc) rustc: LazyCell, /// A collection of configuration options values: LazyCell>, - /// The current working directory of cargo - cwd: PathBuf, /// The location of the cargo executable (path to current process) cargo_exe: LazyCell, /// The location of the rustdoc executable @@ -70,7 +174,7 @@ pub struct Config { } impl Config { - pub fn new(shell: Shell, cwd: PathBuf, homedir: PathBuf) -> Config { + pub fn new(shell: Shell, dirs: CargoDirs) -> Config { static mut GLOBAL_JOBSERVER: *mut jobserver::Client = 0 as *mut _; static INIT: Once = ONCE_INIT; @@ -83,10 +187,9 @@ impl Config { }); Config { - home_path: Filesystem::new(homedir), + dirs: dirs, shell: RefCell::new(shell), rustc: LazyCell::new(), - cwd, values: LazyCell::new(), cargo_exe: LazyCell::new(), rustdoc: LazyCell::new(), @@ -109,40 +212,44 @@ impl Config { pub fn default() -> CargoResult { let shell = Shell::new(); - let cwd = - env::current_dir().chain_err(|| "couldn't get the current directory of the process")?; - let homedir = homedir(&cwd).ok_or_else(|| { - format_err!( - "Cargo couldn't find your home directory. \ - This probably means that $HOME was not set." - ) - })?; - Ok(Config::new(shell, cwd, homedir)) + let cargo_dirs = CargoDirs::new()?; + Ok(Config::new(shell, cargo_dirs)) + } + + pub fn cache_path(&self) -> Filesystem { + self.dirs.cache_dir.clone() + } + + pub fn config_path(&self) -> Filesystem { + self.dirs.config_dir.clone() } - /// The user's cargo home directory (OS-dependent) - pub fn home(&self) -> &Filesystem { - &self.home_path + pub fn data_path(&self) -> PathBuf { + self.dirs.data_dir.clone() + } + + pub fn bin_path(&self) -> PathBuf { + self.dirs.bin_dir.clone() } /// The cargo git directory (`/git`) pub fn git_path(&self) -> Filesystem { - self.home_path.join("git") + self.dirs.cache_dir.join("git") } /// The cargo registry index directory (`/registry/index`) pub fn registry_index_path(&self) -> Filesystem { - self.home_path.join("registry").join("index") + self.dirs.cache_dir.join("registry").join("index") } /// The cargo registry cache directory (`/registry/path`) pub fn registry_cache_path(&self) -> Filesystem { - self.home_path.join("registry").join("cache") + self.dirs.cache_dir.join("registry").join("cache") } /// The cargo registry source directory (`/registry/src`) pub fn registry_source_path(&self) -> Filesystem { - self.home_path.join("registry").join("src") + self.dirs.cache_dir.join("registry").join("src") } /// Get a reference to the shell, for e.g. writing error messages @@ -237,14 +344,14 @@ impl Config { } pub fn cwd(&self) -> &Path { - &self.cwd + &self.dirs.current_dir } pub fn target_dir(&self) -> CargoResult> { if let Some(dir) = env::var_os("CARGO_TARGET_DIR") { - Ok(Some(Filesystem::new(self.cwd.join(dir)))) + Ok(Some(Filesystem::new(self.dirs.current_dir.join(dir)))) } else if let Some(val) = self.get_path("build.target-dir")? { - let val = self.cwd.join(val.val); + let val = self.cwd().join(val.val); Ok(Some(Filesystem::new(val))) } else { Ok(None) @@ -529,7 +636,7 @@ impl Config { pub fn load_values(&self) -> CargoResult> { let mut cfg = CV::Table(HashMap::new(), PathBuf::from(".")); - walk_tree(&self.cwd, |path| { + walk_tree(&self.dirs, |path| { let mut contents = String::new(); let mut file = File::open(&path)?; file.read_to_string(&mut contents) @@ -573,8 +680,8 @@ impl Config { /// Loads credentials config from the credentials file into the ConfigValue object, if present. fn load_credentials(&self, cfg: &mut ConfigValue) -> CargoResult<()> { - let home_path = self.home_path.clone().into_path_unlocked(); - let credentials = home_path.join("credentials"); + let config_path = self.dirs.config_dir.clone().into_path_unlocked(); + let credentials = config_path.join("credentials"); if !fs::metadata(&credentials).is_ok() { return Ok(()); } @@ -589,10 +696,7 @@ impl Config { })?; let toml = cargo_toml::parse(&contents, &credentials, self).chain_err(|| { - format!( - "could not parse TOML configuration in `{}`", - credentials.display() - ) + format!("could not parse TOML configuration in `{}`", credentials.display()) })?; let mut value = CV::from_toml(&credentials, toml).chain_err(|| { @@ -638,7 +742,7 @@ impl Config { None => false, }; let path = if maybe_relative { - self.cwd.join(tool_path) + self.cwd().join(tool_path) } else { PathBuf::from(tool_path) }; @@ -924,17 +1028,13 @@ impl fmt::Display for Definition { } } -pub fn homedir(cwd: &Path) -> Option { - ::home::cargo_home_with_cwd(cwd).ok() -} - -fn walk_tree(pwd: &Path, mut walk: F) -> CargoResult<()> +fn walk_tree(dirs: &CargoDirs, mut walk: F) -> CargoResult<()> where F: FnMut(&Path) -> CargoResult<()>, { let mut stash: HashSet = HashSet::new(); - for current in paths::ancestors(pwd) { + for current in paths::ancestors(&dirs.current_dir) { let possible = current.join(".cargo").join("config"); if fs::metadata(&possible).is_ok() { walk(&possible)?; @@ -943,15 +1043,9 @@ where } // 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 + // in our path to be sure we pick up that standard location for // information. - let home = homedir(pwd).ok_or_else(|| { - format_err!( - "Cargo couldn't find your home directory. \ - This probably means that $HOME was not set." - ) - })?; - let config = home.join("config"); + let config = dirs.config_dir.clone().into_path_unlocked().join("config"); if !stash.contains(&config) && fs::metadata(&config).is_ok() { walk(&config)?; } @@ -961,9 +1055,8 @@ where pub fn save_credentials(cfg: &Config, token: String, registry: Option) -> CargoResult<()> { let mut file = { - cfg.home_path.create_dir()?; - cfg.home_path - .open_rw(Path::new("credentials"), cfg, "credentials' config file")? + cfg.dirs.config_dir.create_dir()?; + cfg.dirs.config_dir.open_rw(Path::new("credentials"), cfg, "credentials' config file")? }; let (key, value) = { diff --git a/src/cargo/util/mod.rs b/src/cargo/util/mod.rs index 2d9505d9a75..a1cec806023 100644 --- a/src/cargo/util/mod.rs +++ b/src/cargo/util/mod.rs @@ -1,5 +1,5 @@ pub use self::cfg::{Cfg, CfgExpr}; -pub use self::config::{homedir, Config, ConfigValue}; +pub use self::config::{Config, ConfigValue}; pub use self::dependency_queue::{DependencyQueue, Dirty, Fresh, Freshness}; pub use self::errors::{CargoError, CargoResult, CargoResultExt, CliResult, Test}; pub use self::errors::{CargoTestError, CliError, ProcessError}; diff --git a/tests/testsuite/login.rs b/tests/testsuite/login.rs index 7ae9f970520..a2795abe6db 100644 --- a/tests/testsuite/login.rs +++ b/tests/testsuite/login.rs @@ -1,3 +1,4 @@ +use std::env; use std::io::prelude::*; use std::fs::{self, File}; @@ -6,7 +7,7 @@ use cargotest::{cargo_process, ChannelChanger}; use cargotest::support::execs; use cargotest::support::registry::registry; use cargotest::install::cargo_home; -use cargo::util::config::Config; +use cargo::util::config::{CargoDirs, Config}; use cargo::core::Shell; use hamcrest::{assert_that, existing_file, is_not}; @@ -163,7 +164,8 @@ fn new_credentials_is_used_instead_old() { execs().with_status(0), ); - let config = Config::new(Shell::new(), cargo_home(), cargo_home()); + env::set_var("CARGO_HOME", cargo_home()); + let config = Config::new(Shell::new(), CargoDirs::new().unwrap()); let token = config.get_string("registry.token").unwrap().map(|p| p.val); assert_eq!(token.unwrap(), TOKEN);