From ba5ffdec9020077bf7e98e63ac04b07456efad3d Mon Sep 17 00:00:00 2001 From: Christoph Koehler Date: Sat, 6 Apr 2024 18:41:44 -0600 Subject: [PATCH] config: use XDG_CONFIG_HOME on MacOS by default This replaces the `dirs` crate with the more lightweight and flexible `etcetera` crate, which also lets us use XDG config on MacOS. Also errors if is finds config in the old place in `Application Support`, so this is a BREAKING change. Fixes #3434 --- CHANGELOG.md | 2 ++ Cargo.lock | 66 ++++++++++----------------------------------- Cargo.toml | 2 +- cli/Cargo.toml | 2 +- cli/src/config.rs | 63 ++++++++++++++++++++++++++++++++++++++----- cli/src/git_util.rs | 2 +- docs/config.md | 8 +++--- 7 files changed, 79 insertions(+), 66 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d78ff840e..b7d99dc8a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Breaking changes +* On MacOS, jj will now look for its config in `$XDG_CONFIG_HOME` and error if config is found in `~/Library/Application Support/jj/` + ### New features * The list of conflicted paths is printed whenever the working copy changes. diff --git a/Cargo.lock b/Cargo.lock index cdd75cb06d..9620fe82c7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -672,27 +672,6 @@ dependencies = [ "subtle", ] -[[package]] -name = "dirs" -version = "5.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "44c45a9d03d6676652bcb5e724c7e988de1acad23a711b5217ab9cbecbec2225" -dependencies = [ - "dirs-sys", -] - -[[package]] -name = "dirs-sys" -version = "0.4.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "520f05a5cbd335fae5a99ff7a6ab8627577660ee5cfd6a94a6a929b52ff0321c" -dependencies = [ - "libc", - "option-ext", - "redox_users", - "windows-sys 0.48.0", -] - [[package]] name = "doc-comment" version = "0.3.3" @@ -743,6 +722,17 @@ dependencies = [ "itertools 0.10.5", ] +[[package]] +name = "etcetera" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "136d1b5283a1ab77bd9257427ffd09d8667ced0570b6f938942bc7568ed5b943" +dependencies = [ + "cfg-if", + "home", + "windows-sys 0.48.0", +] + [[package]] name = "faster-hex" version = "0.9.0" @@ -1680,8 +1670,8 @@ dependencies = [ "config", "criterion", "crossterm", - "dirs", "esl01-renderdag", + "etcetera", "futures 0.3.30", "git2", "gix", @@ -1837,17 +1827,6 @@ dependencies = [ "pkg-config", ] -[[package]] -name = "libredox" -version = "0.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "85c833ca1e66078851dba29046874e38f08b2c883700aa29a03ddd3b23814ee8" -dependencies = [ - "bitflags 2.5.0", - "libc", - "redox_syscall", -] - [[package]] name = "libssh2-sys" version = "0.3.0" @@ -2086,12 +2065,6 @@ dependencies = [ "vcpkg", ] -[[package]] -name = "option-ext" -version = "0.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "04744f49eae99ab78e0d5c0b603ab218f515ea8cfe5a456d7629ad883a3b6e7d" - [[package]] name = "overload" version = "0.1.1" @@ -2338,7 +2311,7 @@ checksum = "80b776a1b2dc779f5ee0641f8ade0125bc1298dd41a9a0c16d8bd57b42d222b1" dependencies = [ "bytes 1.6.0", "heck 0.5.0", - "itertools 0.10.5", + "itertools 0.12.1", "log", "multimap", "once_cell", @@ -2358,7 +2331,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "19de2de2a00075bf566bee3bd4db014b11587e84184d3f7a791bc17f1a8e9e48" dependencies = [ "anyhow", - "itertools 0.10.5", + "itertools 0.12.1", "proc-macro2", "quote", "syn", @@ -2458,17 +2431,6 @@ dependencies = [ "bitflags 1.3.2", ] -[[package]] -name = "redox_users" -version = "0.4.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a18479200779601e498ada4e8c1e1f50e3ee19deb0259c25825a98b5603b2cb4" -dependencies = [ - "getrandom", - "libredox", - "thiserror", -] - [[package]] name = "ref-cast" version = "1.0.22" diff --git a/Cargo.toml b/Cargo.toml index 51040d8212..3a673cef35 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -43,9 +43,9 @@ config = { version = "0.13.4", default-features = false, features = ["toml"] } criterion = "0.5.1" crossterm = { version = "0.27", default-features = false } digest = "0.10.7" -dirs = "5.0.1" either = "1.10.0" esl01-renderdag = "0.3.0" +etcetera = "0.8.0" futures = "0.3.30" git2 = "0.18.3" gix = { version = "0.61.0", default-features = false, features = [ diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 1444902a56..640931428f 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -43,8 +43,8 @@ clap_mangen = { workspace = true } config = { workspace = true } criterion = { workspace = true, optional = true } crossterm = { workspace = true } -dirs = { workspace = true } esl01-renderdag = { workspace = true } +etcetera = { workspace = true } futures = { workspace = true } git2 = { workspace = true } gix = { workspace = true } diff --git a/cli/src/config.rs b/cli/src/config.rs index 453d7dabed..e8a86d6f20 100644 --- a/cli/src/config.rs +++ b/cli/src/config.rs @@ -19,6 +19,7 @@ use std::process::Command; use std::{env, fmt}; use config::Source; +use etcetera::BaseStrategy; use itertools::Itertools; use jj_lib::settings::ConfigResultExt as _; use thiserror::Error; @@ -32,6 +33,13 @@ pub enum ConfigError { AmbiguousSource(PathBuf, PathBuf), #[error(transparent)] ConfigCreateError(#[from] std::io::Error), + #[error( + "Storing config in {0} is deprecated. Please move your config to {1} and delete the old \ + location: \n`mkdir -p {1}jj && mv \"{0}jj/config.toml\" \"{1}jj/\" && rm -rf \"{0}jj\"`" + )] + DeprecatedConfig(PathBuf, PathBuf), + #[error(transparent)] + HomeDirError(etcetera::HomeDirError), } #[derive(Clone, Debug, PartialEq, Eq)] @@ -251,8 +259,8 @@ struct ConfigEnv { impl ConfigEnv { fn new() -> Self { ConfigEnv { - config_dir: dirs::config_dir(), - home_dir: dirs::home_dir(), + config_dir: config_dir(), + home_dir: etcetera::home_dir().ok(), jj_config: env::var("JJ_CONFIG").ok(), } } @@ -262,17 +270,22 @@ impl ConfigEnv { // TODO: We should probably support colon-separated (std::env::split_paths) return Ok(ConfigPath::new(Some(PathBuf::from(path)))); } + // TODO: Should we drop the final `/config.toml` and read all files in the // directory? - let platform_config_path = ConfigPath::new(self.config_dir.map(|mut config_dir| { - config_dir.push("jj"); - config_dir.push("config.toml"); - config_dir - })); + let platform_config_path = + ConfigPath::new(self.config_dir.clone().map(|mut config_dir| { + config_dir.push("jj"); + config_dir.push("config.toml"); + config_dir + })); let home_config_path = ConfigPath::new(self.home_dir.map(|mut home_dir| { home_dir.push(".jjconfig.toml"); home_dir })); + + check_deprecated_macos_config_dir()?; + use ConfigPath::*; match (platform_config_path, home_config_path) { (Existing(platform_config_path), Existing(home_config_path)) => Err( @@ -303,6 +316,23 @@ impl ConfigEnv { } } +fn check_deprecated_macos_config_dir() -> Result<(), ConfigError> { + let apple_config_dir = etcetera::base_strategy::Apple::new() + .map_err(ConfigError::HomeDirError)? + .data_dir(); + let xdg_config_dir = etcetera::base_strategy::Xdg::new() + .map_err(ConfigError::HomeDirError)? + .config_dir(); + if config_exists_in(&apple_config_dir) { + Err(ConfigError::DeprecatedConfig( + apple_config_dir, + xdg_config_dir, + )) + } else { + Ok(()) + } +} + pub fn existing_config_path() -> Result, ConfigError> { ConfigEnv::new().existing_config_path() } @@ -434,6 +464,25 @@ fn read_config_path(config_path: &Path) -> Result bool { + let mut config_file = dir.to_owned(); + config_file.push("jj/config.toml"); + config_file.exists() +} + +#[cfg(target_os = "macos")] +fn config_dir() -> Option { + // Try getting config directory from Xdg base strategy first. + Some(etcetera::base_strategy::Xdg::new().ok()?.config_dir()) +} + +#[cfg(not(target_os = "macos"))] +/// For anything but MacOS, use the default config dir for the platform +fn config_dir() -> Option { + Some(etcetera::choose_base_strategy().ok()?.config_dir()) +} + /// Command name and arguments specified by config. #[derive(Clone, Debug, Eq, PartialEq, serde::Deserialize)] #[serde(untagged)] diff --git a/cli/src/git_util.rs b/cli/src/git_util.rs index 05f4ba6192..df54804c30 100644 --- a/cli/src/git_util.rs +++ b/cli/src/git_util.rs @@ -121,7 +121,7 @@ fn pinentry_get_pw(url: &str) -> Option { #[tracing::instrument] fn get_ssh_keys(_username: &str) -> Vec { let mut paths = vec![]; - if let Some(home_dir) = dirs::home_dir() { + if let Ok(home_dir) = etcetera::home_dir() { let ssh_dir = Path::new(&home_dir).join(".ssh"); for filename in ["id_ed25519_sk", "id_ed25519", "id_rsa"] { let key_path = ssh_dir.join(filename); diff --git a/docs/config.md b/docs/config.md index 7429a833e3..4f64435007 100644 --- a/docs/config.md +++ b/docs/config.md @@ -745,10 +745,10 @@ platform-specific location is recommended for better integration with platform services. It is an error for both of these files to exist. | Platform | Value | Example | -| :------- | :------------------------------------------------- | :-------------------------------------------------------- | -| Linux | `$XDG_CONFIG_HOME/jj/config.toml` | `/home/alice/.config/jj/config.toml` | -| macOS | `$HOME/Library/Application Support/jj/config.toml` | `/Users/Alice/Library/Application Support/jj/config.toml` | -| Windows | `{FOLDERID_RoamingAppData}\jj\config.toml` | `C:\Users\Alice\AppData\Roaming\jj\config.toml` | +| :------- | -------------------------------------------------- | --------------------------------------------------------- | +| Linux | `$XDG_CONFIG_HOME/jj/config.toml` | `/home/alice/.config/jj/config.toml` | +| macOS | `$HOME/Library/Application Support/jj/config.toml` or `$XDG_CONFIG_HOME/jj/config.toml` as fallback | `/Users/Alice/Library/Application Support/jj/config.toml` or `/Users/Alice/.config/jj/config.toml` | +| Windows | `{FOLDERID_RoamingAppData}\jj\config.toml` | `C:\Users\Alice\AppData\Roaming\jj\config.toml` | The location of the `jj` config file can also be overridden with the `JJ_CONFIG` environment variable. If it is not empty, it should contain the path