Fix BAT_CONFIG_DIR pointing at system config dir causing duplicate flag errors

When BAT_CONFIG_DIR is set to the system config directory (e.g. /etc/bat),
both system_config_file() and config_file() resolve to /etc/bat/config.
The config file was read twice, causing clap errors for non-repeatable flags
like --italic-text.

Skip the user config read when it resolves to the same file as the system
config, using canonicalization to also handle symlinks.

Closes #3589

CHANGELOG.md

@@ -14,6 +14,7 @@
 - Add `--fallback-syntax`/`--fallback-language` to apply syntax highlighting only when auto-detection fails, see #1341 (@Xavrir)
 
 ## Bugfixes
+- Fix `BAT_CONFIG_DIR` pointing at system config directory causing duplicate flag errors. Closes #3589, see #3620 (@Xavrir)
 - Report error when pager is missing instead of silently falling back, see #3588 (@IMaloney)
 - Fix `--wrap=never` and `-S` flags being ignored when piping to pager, see #3592 (@IMaloney)
 - Fix crash with BusyBox `less` on Windows, see #3527 (@Anchal-T)

src/bin/bat/config.rs

@@ -2,7 +2,7 @@ use std::env;
 use std::ffi::OsString;
 use std::fs;
 use std::io::{self, Write};
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 
 use crate::directories::PROJECT_DIRS;
 
@@ -104,18 +104,32 @@ pub fn generate_config_file() -> bat::error::Result<()> {
 pub fn get_args_from_config_file() -> Result<Vec<OsString>, shell_words::ParseError> {
     let mut config = String::new();
 
-    if let Ok(c) = fs::read_to_string(system_config_file()) {
+    let system_config = system_config_file();
+    let user_config = config_file();
+
+    if let Ok(c) = fs::read_to_string(&system_config) {
         config.push_str(&c);
         config.push('\n');
     }
 
-    if let Ok(c) = fs::read_to_string(config_file()) {
-        config.push_str(&c);
+    // Skip the user config if it resolves to the same file as the system config,
+    // which can happen when BAT_CONFIG_DIR is set to e.g. "/etc/bat". See #3589.
+    if !same_file(&system_config, &user_config) {
+        if let Ok(c) = fs::read_to_string(&user_config) {
+            config.push_str(&c);
+        }
     }
 
     get_args_from_str(&config)
 }
 
+fn same_file(a: &Path, b: &Path) -> bool {
+    match (fs::canonicalize(a), fs::canonicalize(b)) {
+        (Ok(a), Ok(b)) => a == b,
+        _ => a == b,
+    }
+}
+
 pub fn get_args_from_env_opts_var() -> Option<Result<Vec<OsString>, shell_words::ParseError>> {
     env::var("BAT_OPTS").ok().map(|s| get_args_from_str(&s))
 }
@@ -214,3 +228,40 @@ fn comments() {
         get_args_from_str(config).unwrap()
     );
 }
+
+#[test]
+fn same_file_identical_paths() {
+    let dir = tempfile::tempdir().unwrap();
+    let file = dir.path().join("config");
+    fs::write(&file, "").unwrap();
+    assert!(same_file(&file, &file));
+}
+
+#[test]
+fn same_file_different_paths() {
+    let dir = tempfile::tempdir().unwrap();
+    let a = dir.path().join("a");
+    let b = dir.path().join("b");
+    fs::write(&a, "").unwrap();
+    fs::write(&b, "").unwrap();
+    assert!(!same_file(&a, &b));
+}
+
+#[test]
+fn same_file_nonexistent() {
+    let dir = tempfile::tempdir().unwrap();
+    let a = dir.path().join("a");
+    let b = dir.path().join("b");
+    assert!(!same_file(&a, &b));
+}
+
+#[cfg(unix)]
+#[test]
+fn same_file_via_symlink() {
+    let dir = tempfile::tempdir().unwrap();
+    let original = dir.path().join("config");
+    let link = dir.path().join("link");
+    fs::write(&original, "").unwrap();
+    std::os::unix::fs::symlink(&original, &link).unwrap();
+    assert!(same_file(&original, &link));
+}