fixes #1884: HISTFILE can be a directory or a file (#2630)

Xonsh history import was failing (in the default xonsh configuration)
because $HISTFILE is actually a directory in that case. This change sets
up the xonsh import to check for a *directory* instead of a regular
file, and makes it clearer that other importers expect a regular file.

crates/atuin-client/src/import/bash.rs

@@ -6,7 +6,7 @@ use eyre::{Result, eyre};
 use itertools::Itertools;
 use time::{Duration, OffsetDateTime};
 
-use super::{Importer, Loader, get_histpath, unix_byte_lines};
+use super::{Importer, Loader, get_histfile_path, unix_byte_lines};
 use crate::history::History;
 use crate::import::read_to_end;
 
@@ -27,7 +27,7 @@ impl Importer for Bash {
     const NAME: &'static str = "bash";
 
     async fn new() -> Result<Self> {
-        let bytes = read_to_end(get_histpath(default_histpath)?)?;
+        let bytes = read_to_end(get_histfile_path(default_histpath)?)?;
         Ok(Self { bytes })
     }
 

crates/atuin-client/src/import/mod.rs

@@ -73,12 +73,26 @@ where
     D: FnOnce() -> Result<PathBuf>,
 {
     if let Ok(p) = std::env::var("HISTFILE") {
-        is_file(PathBuf::from(p))
+        Ok(PathBuf::from(p))
     } else {
-        is_file(def()?)
+        def()
     }
 }
 
+fn get_histfile_path<D>(def: D) -> Result<PathBuf>
+where
+    D: FnOnce() -> Result<PathBuf>,
+{
+    get_histpath(def).and_then(is_file)
+}
+
+fn get_histdir_path<D>(def: D) -> Result<PathBuf>
+where
+    D: FnOnce() -> Result<PathBuf>,
+{
+    get_histpath(def).and_then(is_dir)
+}
+
 fn read_to_end(path: PathBuf) -> Result<Vec<u8>> {
     let mut bytes = Vec::new();
     let mut f = File::open(path)?;
@@ -92,6 +106,16 @@ fn is_file(p: PathBuf) -> Result<PathBuf> {
         bail!("Could not find history file {:?}. Try setting $HISTFILE", p)
     }
 }
+fn is_dir(p: PathBuf) -> Result<PathBuf> {
+    if p.is_dir() {
+        Ok(p)
+    } else {
+        bail!(
+            "Could not find history directory {:?}. Try setting $HISTFILE",
+            p
+        )
+    }
+}
 
 #[cfg(test)]
 mod tests {

crates/atuin-client/src/import/replxx.rs

@@ -5,7 +5,7 @@ use directories::UserDirs;
 use eyre::{Result, eyre};
 use time::{OffsetDateTime, PrimitiveDateTime, macros::format_description};
 
-use super::{Importer, Loader, get_histpath, unix_byte_lines};
+use super::{Importer, Loader, get_histfile_path, unix_byte_lines};
 use crate::history::History;
 use crate::import::read_to_end;
 
@@ -28,7 +28,7 @@ impl Importer for Replxx {
     const NAME: &'static str = "replxx";
 
     async fn new() -> Result<Self> {
-        let bytes = read_to_end(get_histpath(default_histpath)?)?;
+        let bytes = read_to_end(get_histfile_path(default_histpath)?)?;
         Ok(Self { bytes })
     }
 

crates/atuin-client/src/import/resh.rs

@@ -8,7 +8,7 @@ use serde::Deserialize;
 use atuin_common::utils::uuid_v7;
 use time::OffsetDateTime;
 
-use super::{Importer, Loader, get_histpath, unix_byte_lines};
+use super::{Importer, Loader, get_histfile_path, unix_byte_lines};
 use crate::history::History;
 use crate::import::read_to_end;
 
@@ -85,7 +85,7 @@ impl Importer for Resh {
     const NAME: &'static str = "resh";
 
     async fn new() -> Result<Self> {
-        let bytes = read_to_end(get_histpath(default_histpath)?)?;
+        let bytes = read_to_end(get_histfile_path(default_histpath)?)?;
         Ok(Self { bytes })
     }
 

crates/atuin-client/src/import/xonsh.rs

@@ -10,7 +10,7 @@ use time::OffsetDateTime;
 use uuid::Uuid;
 use uuid::timestamp::{Timestamp, context::NoContext};
 
-use super::{Importer, Loader, get_histpath};
+use super::{Importer, Loader, get_histdir_path};
 use crate::history::History;
 use crate::utils::get_host_user;
 
@@ -102,7 +102,7 @@ impl Importer for Xonsh {
     async fn new() -> Result<Self> {
         // wrap xonsh-specific path resolver in general one so that it respects $HISTPATH
         let xonsh_data_dir = env::var("XONSH_DATA_DIR").ok();
-        let hist_dir = get_histpath(|| xonsh_hist_dir(xonsh_data_dir))?;
+        let hist_dir = get_histdir_path(|| xonsh_hist_dir(xonsh_data_dir))?;
         let sessions = load_sessions(&hist_dir)?;
         let hostname = get_host_user();
         Ok(Xonsh { sessions, hostname })

crates/atuin-client/src/import/xonsh_sqlite.rs

@@ -10,7 +10,7 @@ use time::OffsetDateTime;
 use uuid::Uuid;
 use uuid::timestamp::{Timestamp, context::NoContext};
 
-use super::{Importer, Loader, get_histpath};
+use super::{Importer, Loader, get_histfile_path};
 use crate::history::History;
 use crate::utils::get_host_user;
 
@@ -93,7 +93,7 @@ impl Importer for XonshSqlite {
     async fn new() -> Result<Self> {
         // wrap xonsh-specific path resolver in general one so that it respects $HISTPATH
         let xonsh_data_dir = env::var("XONSH_DATA_DIR").ok();
-        let db_path = get_histpath(|| xonsh_db_path(xonsh_data_dir))?;
+        let db_path = get_histfile_path(|| xonsh_db_path(xonsh_data_dir))?;
         let connection_str = db_path.to_str().ok_or_else(|| {
             eyre!(
                 "Invalid path for SQLite database: {}",

crates/atuin-client/src/import/zsh.rs

@@ -9,7 +9,7 @@ use directories::UserDirs;
 use eyre::{Result, eyre};
 use time::OffsetDateTime;
 
-use super::{Importer, Loader, get_histpath, unix_byte_lines};
+use super::{Importer, Loader, get_histfile_path, unix_byte_lines};
 use crate::history::History;
 use crate::import::read_to_end;
 
@@ -49,7 +49,7 @@ impl Importer for Zsh {
     const NAME: &'static str = "zsh";
 
     async fn new() -> Result<Self> {
-        let bytes = read_to_end(get_histpath(default_histpath)?)?;
+        let bytes = read_to_end(get_histfile_path(default_histpath)?)?;
         Ok(Self { bytes })
     }