Skip to content

Commit

Permalink
config: use XDG_CONFIG_HOME on MacOS as a fallback
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.

Fixes #3434
  • Loading branch information
ckoehler committed Apr 6, 2024
1 parent 93cebcd commit 85c4e91
Show file tree
Hide file tree
Showing 7 changed files with 61 additions and 61 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* `jj status` now supports filtering by paths. For example, `jj status .` will
only list changed files that are descendants of the current directory.

* On MacOS, jj will now look for its config in `$XDG_CONFIG_HOME` first, then fall back to `~/Library/Application Support`

### Fixed bugs

## [0.16.0] - 2024-04-03
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
40 changes: 38 additions & 2 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 Down Expand Up @@ -251,8 +252,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 Down Expand Up @@ -434,6 +435,41 @@ fn read_config_path(config_path: &Path) -> Result<config::Config, config::Config
.build()
}

#[cfg(target_os = "macos")]
fn config_dir() -> Option<PathBuf> {
// Define a closure to check if config file exists in a directory
let check_config = |dir: &Path| {
let mut config_file = dir.to_owned();
config_file.push("jj/config.toml");
if config_file.exists() {
Some(dir.to_owned())
} else {
None
}
};

// Try getting config directory from Xdg base strategy first.
let path = etcetera::base_strategy::Xdg::new()
.ok()
.and_then(|strategy| check_config(&strategy.config_dir()));

// If config directory is not found from Xdg, try Apple instead. `Application
// Support` is really the data_dir, so get that.
path.or_else(|| {
etcetera::base_strategy::Apple::new()
.ok()
.and_then(|strategy| check_config(&strategy.data_dir()))
})
}

#[cfg(not(target_os = "macos"))]
/// For anything but MacOS, use the default config dir for the platform
fn config_dir() -> Option<PathBuf> {
etcetera::choose_base_strategy()
.ok()
.map(|strategy| strategy.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 85c4e91

Please sign in to comment.