feat!: Use CommandInput for commands (#269)

This re-allows to use a String for `password-command` or
`warm-up-command`.

Breaking Change: The former version to give the command in an array is
no longer supported.
Instead of:
```
password-command = ["echo", "password"]
```
please use either of
```
password-command = "echo password"
password-command = { command = "echo", args = ["password"] }
```

---------

Co-authored-by: simonsan <14062932+simonsan@users.noreply.github.com>

crates/backend/Cargo.toml

@@ -70,7 +70,6 @@ merge = { version = "0.1.0", optional = true }
 
 # local backend
 aho-corasick = { workspace = true }
-shell-words = "1.1.0"
 walkdir = "2.5.0"
 
 # rest backend

crates/backend/src/error.rs

@@ -106,8 +106,6 @@ pub enum LocalBackendErrorKind {
     /// error building automaton `{0:?}`
     FromAhoCorasick(#[from] aho_corasick::BuildError),
     /// {0:?}
-    FromSplitError(#[from] shell_words::ParseError),
-    /// {0:?}
     #[error(transparent)]
     FromTryIntError(#[from] TryFromIntError),
     /// {0:?}

crates/backend/src/local.rs

@@ -9,10 +9,9 @@ use aho_corasick::AhoCorasick;
 use anyhow::Result;
 use bytes::Bytes;
 use log::{debug, trace, warn};
-use shell_words::split;
 use walkdir::WalkDir;
 
-use rustic_core::{FileType, Id, ReadBackend, WriteBackend, ALL_FILE_TYPES};
+use rustic_core::{CommandInput, FileType, Id, ReadBackend, WriteBackend, ALL_FILE_TYPES};
 
 use crate::error::LocalBackendErrorKind;
 
@@ -121,9 +120,9 @@ impl LocalBackend {
         let replace_with = &[filename.to_str().unwrap(), tpe.dirname(), id.as_str()];
         let actual_command = ac.replace_all(command, replace_with);
         debug!("calling {actual_command}...");
-        let commands = split(&actual_command).map_err(LocalBackendErrorKind::FromSplitError)?;
-        let status = Command::new(&commands[0])
-            .args(&commands[1..])
+        let command: CommandInput = actual_command.parse()?;
+        let status = Command::new(command.command())
+            .args(command.args())
             .status()
             .map_err(LocalBackendErrorKind::CommandExecutionFailed)?;
         if !status.success() {

crates/backend/src/rclone.rs

@@ -7,20 +7,21 @@ use std::{
 
 use anyhow::Result;
 use bytes::Bytes;
+use constants::DEFAULT_COMMAND;
 use log::{debug, info};
 use rand::{
     distributions::{Alphanumeric, DistString},
     thread_rng,
 };
-
 use semver::{BuildMetadata, Prerelease, Version, VersionReq};
-use shell_words::split;
 
 use crate::{error::RcloneErrorKind, rest::RestBackend};
 
-use rustic_core::{FileType, Id, ReadBackend, WriteBackend};
+use rustic_core::{CommandInput, FileType, Id, ReadBackend, WriteBackend};
 
 pub(super) mod constants {
+    /// The default command called if no other is specified
+    pub(super) const DEFAULT_COMMAND: &str = "rclone serve restic --addr localhost:0";
     /// The string to search for in the rclone output.
     pub(super) const SEARCHSTRING: &str = "Serving restic REST API on ";
 }
@@ -150,12 +151,13 @@ impl RcloneBackend {
         let user = Alphanumeric.sample_string(&mut thread_rng(), 12);
         let password = Alphanumeric.sample_string(&mut thread_rng(), 12);
 
-        let mut rclone_command =
-            split(rclone_command.map_or("rclone serve restic --addr localhost:0", String::as_str))?;
-        rclone_command.push(url.as_ref().to_string());
+        let mut rclone_command = rclone_command.map_or(DEFAULT_COMMAND.to_string(), Clone::clone);
+        rclone_command.push(' ');
+        rclone_command.push_str(url.as_ref());
+        let rclone_command: CommandInput = rclone_command.parse()?;
         debug!("starting rclone via {rclone_command:?}");
 
-        let mut command = Command::new(&rclone_command[0]);
+        let mut command = Command::new(rclone_command.command());
 
         if use_password {
             // TODO: We should handle errors here
@@ -165,7 +167,7 @@ impl RcloneBackend {
         }
 
         let mut child = command
-            .args(&rclone_command[1..])
+            .args(rclone_command.args())
             .stderr(Stdio::piped())
             .spawn()
             .map_err(RcloneErrorKind::FromIoError)?;

crates/core/src/repository.rs

@@ -1,6 +1,3 @@
-// Note: we need a fully qualified Vec here for clap, see https://github.com/clap-rs/clap/issues/4481
-#![allow(unused_qualifications)]
-
 mod command_input;
 mod warm_up;
 
@@ -18,7 +15,7 @@ use std::{
 use bytes::Bytes;
 use derive_setters::Setters;
 use log::{debug, error, info};
-use serde_with::{serde_as, DisplayFromStr, OneOrMany};
+use serde_with::{serde_as, DisplayFromStr};
 
 use crate::{
     backend::{
@@ -119,13 +116,8 @@ pub struct RepositoryOptions {
         global = true,
         env = "RUSTIC_PASSWORD_COMMAND",
         conflicts_with_all = &["password", "password_file"],
-        value_parser = clap::builder::ValueParser::new(shell_words::split),
-        default_value = "",
     ))]
-    #[cfg_attr(feature = "merge", merge(strategy = merge::vec::overwrite_empty))]
-    #[serde_as(as = "OneOrMany<_>")]
-    // Note: we need a fully qualified Vec here for clap, see https://github.com/clap-rs/clap/issues/4481
-    pub password_command: std::vec::Vec<String>,
+    pub password_command: Option<CommandInput>,
 
     /// Don't use a cache.
     #[cfg_attr(feature = "clap", clap(long, global = true, env = "RUSTIC_NO_CACHE"))]
@@ -151,18 +143,11 @@ pub struct RepositoryOptions {
     pub warm_up: bool,
 
     /// Warm up needed data pack files by running the command with %id replaced by pack id
-    #[cfg_attr(feature = "clap", clap(
-        long,
-        global = true,
-        conflicts_with = "warm_up", 
-        value_parser = clap::builder::ValueParser::new(shell_words::split),
-        action = clap::ArgAction::Set,
-        default_value = "",
-    ))]
-    #[cfg_attr(feature = "merge", merge(strategy = merge::vec::overwrite_empty))]
-    #[serde_as(as = "OneOrMany<_>")]
-    // Note: we need a fully qualified Vec here for clap, see https://github.com/clap-rs/clap/issues/4481
-    pub warm_up_command: std::vec::Vec<String>,
+    #[cfg_attr(
+        feature = "clap",
+        clap(long, global = true, conflicts_with = "warm_up",)
+    )]
+    pub warm_up_command: Option<CommandInput>,
 
     /// Duration (e.g. 10m) to wait after warm up
     #[cfg_attr(feature = "clap", clap(long, global = true, value_name = "DURATION"))]
@@ -199,10 +184,10 @@ impl RepositoryOptions {
                 );
                 Ok(Some(read_password_from_reader(&mut file)?))
             }
-            (_, _, command) if !command.is_empty() => {
+            (_, _, Some(command)) if command.is_set() => {
                 debug!("commands: {command:?}");
-                let command = Command::new(&command[0])
-                    .args(&command[1..])
+                let command = Command::new(command.command())
+                    .args(command.args())
                     .stdout(Stdio::piped())
                     .spawn();
 
@@ -356,11 +341,11 @@ impl<P> Repository<P, ()> {
         let mut be = backends.repository();
         let be_hot = backends.repo_hot();
 
-        if !opts.warm_up_command.is_empty() {
-            if opts.warm_up_command.iter().all(|c| !c.contains("%id")) {
+        if let Some(warm_up) = &opts.warm_up_command {
+            if warm_up.args().iter().all(|c| !c.contains("%id")) {
                 return Err(RepositoryErrorKind::NoIDSpecified.into());
             }
-            info!("using warm-up command {:?}", opts.warm_up_command);
+            info!("using warm-up command {warm_up}");
         }
 
         if opts.warm_up {

crates/core/src/repository/warm_up.rs

@@ -10,6 +10,7 @@ use crate::{
     id::Id,
     progress::{Progress, ProgressBars},
     repository::Repository,
+    CommandInput,
 };
 
 pub(super) mod constants {
@@ -62,8 +63,8 @@ pub(crate) fn warm_up<P: ProgressBars, S>(
     repo: &Repository<P, S>,
     packs: impl ExactSizeIterator<Item = Id>,
 ) -> RusticResult<()> {
-    if !repo.opts.warm_up_command.is_empty() {
-        warm_up_command(packs, &repo.opts.warm_up_command, &repo.pb)?;
+    if let Some(warm_up_cmd) = &repo.opts.warm_up_command {
+        warm_up_command(packs, warm_up_cmd, &repo.pb)?;
     } else if repo.be.needs_warm_up() {
         warm_up_repo(repo, packs)?;
     }
@@ -85,18 +86,19 @@ pub(crate) fn warm_up<P: ProgressBars, S>(
 /// [`RepositoryErrorKind::FromSplitError`]: crate::error::RepositoryErrorKind::FromSplitError
 fn warm_up_command<P: ProgressBars>(
     packs: impl ExactSizeIterator<Item = Id>,
-    command: &[String],
+    command: &CommandInput,
     pb: &P,
 ) -> RusticResult<()> {
     let p = pb.progress_counter("warming up packs...");
     p.set_length(packs.len() as u64);
     for pack in packs {
-        let command: Vec<_> = command
+        let args: Vec<_> = command
+            .args()
             .iter()
             .map(|c| c.replace("%id", &pack.to_hex()))
             .collect();
         debug!("calling {command:?}...");
-        let status = Command::new(&command[0]).args(&command[1..]).status()?;
+        let status = Command::new(command.command()).args(&args).status()?;
         if !status.success() {
             warn!("warm-up command was not successful for pack {pack:?}. {status}");
         }

crates/core/tests/integration.rs

@@ -34,10 +34,10 @@ use insta::{
 use pretty_assertions::assert_eq;
 use rstest::{fixture, rstest};
 use rustic_core::{
-    repofile::SnapshotFile, BackupOptions, CheckOptions, ConfigOptions, FindMatches, FindNode,
-    FullIndex, IndexedFull, IndexedStatus, KeyOptions, LimitOption, LsOptions, NoProgressBars,
-    OpenStatus, ParentOptions, PathList, Repository, RepositoryBackends, RepositoryOptions,
-    RusticResult, SnapshotGroupCriterion, SnapshotOptions, StringList,
+    repofile::SnapshotFile, BackupOptions, CheckOptions, CommandInput, ConfigOptions, FindMatches,
+    FindNode, FullIndex, IndexedFull, IndexedStatus, KeyOptions, LimitOption, LsOptions,
+    NoProgressBars, OpenStatus, ParentOptions, PathList, Repository, RepositoryBackends,
+    RepositoryOptions, RusticResult, SnapshotGroupCriterion, SnapshotOptions, StringList,
 };
 use rustic_core::{
     repofile::{Metadata, Node},
@@ -175,6 +175,22 @@ fn assert_with_win<T: Serialize>(test: &str, snap: T) {
     assert_ron_snapshot!(format!("{test}-nix"), snap);
 }
 
+#[test]
+fn repo_with_commands() -> Result<()> {
+    let be = InMemoryBackend::new();
+    let be = RepositoryBackends::new(Arc::new(be), None);
+    let command: CommandInput = "echo test".parse()?;
+    let warm_up: CommandInput = "echo %id".parse()?;
+    let options = RepositoryOptions::default()
+        .password_command(command)
+        .warm_up_command(warm_up);
+    let repo = Repository::new(&options, &be)?;
+    let key_opts = KeyOptions::default();
+    let config_opts = &ConfigOptions::default();
+    let _repo = repo.init(&key_opts, config_opts)?;
+    Ok(())
+}
+
 #[rstest]
 fn test_backup_with_tar_gz_passes(
     tar_gz_testdata: Result<TestSource>,