Skip to content

Commit

Permalink
config: use XDG_CONFIG_HOME on MacOS by default
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ckoehler committed Apr 7, 2024
1 parent 93cebcd commit ba5ffde
Show file tree
Hide file tree
Showing 7 changed files with 79 additions and 66 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
66 changes: 14 additions & 52 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down
2 changes: 1 addition & 1 deletion cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
63 changes: 56 additions & 7 deletions cli/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)]
Expand Down Expand Up @@ -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(),
}
}
Expand All @@ -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(
Expand Down Expand Up @@ -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<Option<PathBuf>, ConfigError> {
ConfigEnv::new().existing_config_path()
}
Expand Down Expand Up @@ -434,6 +464,25 @@ fn read_config_path(config_path: &Path) -> Result<config::Config, config::Config
.build()
}

// Define a closure to check if config file exists in a directory
fn config_exists_in(dir: &Path) -> 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<PathBuf> {
// 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<PathBuf> {
Some(etcetera::choose_base_strategy().ok()?.config_dir())
}

/// Command name and arguments specified by config.
#[derive(Clone, Debug, Eq, PartialEq, serde::Deserialize)]
#[serde(untagged)]
Expand Down
2 changes: 1 addition & 1 deletion cli/src/git_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ fn pinentry_get_pw(url: &str) -> Option<String> {
#[tracing::instrument]
fn get_ssh_keys(_username: &str) -> Vec<PathBuf> {
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);
Expand Down
8 changes: 4 additions & 4 deletions docs/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit ba5ffde

Please sign in to comment.