Factor out duplicated initialisation code in cli

Author: christian-schilling

josh-cli/src/bin/josh.rs

@@ -290,15 +290,7 @@ fn main() {
     env_logger::init();
     let cli = Cli::parse();
 
-    let result = match &cli.command {
-        Command::Clone(args) => handle_clone(args),
-        Command::Fetch(args) => handle_fetch(args),
-        Command::Pull(args) => handle_pull(args),
-        Command::Push(args) => handle_push(args),
-        Command::Remote(args) => handle_remote(args),
-        Command::Filter(args) => handle_filter(args),
-        Command::Link(args) => handle_link(args),
-    };
+    let result = run_command(&cli);
 
     if let Err(e) = result {
         eprintln!("Error: {e}");
@@ -311,16 +303,16 @@ fn main() {
     }
 }
 
-/// Apply josh filtering to all remote refs and update local refs
-fn apply_josh_filtering(
-    repo_path: &std::path::Path,
-    filter: &str,
-    remote_name: &str,
-) -> anyhow::Result<()> {
-    // Use josh API directly instead of calling josh-filter binary
-    let filterobj = josh_core::filter::parse(filter)
-        .map_err(from_josh_err)
-        .context("Failed to parse filter")?;
+fn run_command(cli: &Cli) -> anyhow::Result<()> {
+    // For clone, do the initial repo setup before creating transaction
+    let repo_path = if let Command::Clone(args) = &cli.command {
+        // For clone, we're not in a git repo initially, so clone first and use that path
+        clone_repo(args)?
+    } else {
+        // For other commands, we need to be in a git repo
+        let repo = git2::Repository::open_from_env().context("Not in a git repository")?;
+        repo.path().parent().unwrap().to_path_buf()
+    };
 
     josh_core::cache_sled::sled_load(&repo_path.join(".git"))
         .map_err(from_josh_err)
@@ -330,16 +322,40 @@ fn apply_josh_filtering(
         josh_core::cache_stack::CacheStack::new()
             .with_backend(josh_core::cache_sled::SledCacheBackend::default())
             .with_backend(
-                josh_core::cache_notes::NotesCacheBackend::new(repo_path)
+                josh_core::cache_notes::NotesCacheBackend::new(&repo_path)
                     .map_err(from_josh_err)
                     .context("Failed to create NotesCacheBackend")?,
             ),
     );
 
-    // Open Josh transaction
-    let transaction = josh_core::cache::TransactionContext::new(repo_path, cache.clone())
+    // Create transaction using the known repo path
+    let transaction = josh_core::cache::TransactionContext::new(&repo_path, cache.clone())
         .open(None)
-        .map_err(from_josh_err)?;
+        .map_err(from_josh_err)
+        .context("Failed TransactionContext::open")?;
+
+    match &cli.command {
+        Command::Clone(args) => handle_clone(args, &transaction),
+        Command::Fetch(args) => handle_fetch(args, &transaction),
+        Command::Pull(args) => handle_pull(args, &transaction),
+        Command::Push(args) => handle_push(args, &transaction),
+        Command::Remote(args) => handle_remote(args, &transaction),
+        Command::Filter(args) => handle_filter(args, &transaction),
+        Command::Link(args) => handle_link(args, &transaction),
+    }
+}
+
+/// Apply josh filtering to all remote refs and update local refs
+fn apply_josh_filtering(
+    transaction: &josh_core::cache::Transaction,
+    repo_path: &std::path::Path,
+    filter: &str,
+    remote_name: &str,
+) -> anyhow::Result<()> {
+    // Use josh API directly instead of calling josh-filter binary
+    let filterobj = josh_core::filter::parse(filter)
+        .map_err(from_josh_err)
+        .context("Failed to parse filter")?;
 
     let repo = transaction.repo();
 
@@ -419,7 +435,8 @@ fn to_absolute_remote_url(url: &str) -> anyhow::Result<String> {
     }
 }
 
-fn handle_clone(args: &CloneArgs) -> anyhow::Result<()> {
+/// Initial clone setup: create directory, init repo, add remote (no transaction needed)
+fn clone_repo(args: &CloneArgs) -> anyhow::Result<std::path::PathBuf> {
     // Use the provided output directory
     let output_dir = args.out.clone();
 
@@ -439,6 +456,16 @@ fn handle_clone(args: &CloneArgs) -> anyhow::Result<()> {
 
     handle_remote_add_repo(&remote_add_args, &output_dir)?;
 
+    Ok(output_dir)
+}
+
+fn handle_clone(
+    args: &CloneArgs,
+    transaction: &josh_core::cache::Transaction,
+) -> anyhow::Result<()> {
+    // Get the repo path from the transaction
+    let output_dir = transaction.repo().path().parent().unwrap();
+
     // Create FetchArgs from CloneArgs
     let fetch_args = FetchArgs {
         remote: "origin".to_string(),
@@ -447,13 +474,13 @@ fn handle_clone(args: &CloneArgs) -> anyhow::Result<()> {
     };
 
     // Use handle_fetch to do the actual fetching and filtering
-    handle_fetch_repo(&fetch_args, &output_dir)?;
+    handle_fetch_repo(&fetch_args, output_dir, transaction)?;
 
     // Get the default branch name from the remote HEAD symref
     let default_branch = if args.branch == "HEAD" {
         // Read the remote HEAD symref to get the default branch
         let head_ref = "refs/remotes/origin/HEAD".to_string();
-        let repo = git2::Repository::open(&output_dir).context("Failed to open repository")?;
+        let repo = transaction.repo();
 
         let head_reference = repo
             .find_reference(&head_ref)
@@ -508,7 +535,7 @@ fn handle_clone(args: &CloneArgs) -> anyhow::Result<()> {
     Ok(())
 }
 
-fn handle_pull(args: &PullArgs) -> anyhow::Result<()> {
+fn handle_pull(args: &PullArgs, transaction: &josh_core::cache::Transaction) -> anyhow::Result<()> {
     // Check if we're in a git repository
     let repo = git2::Repository::open_from_env().context("Not in a git repository")?;
     let repo_path = repo.path().parent().unwrap().to_path_buf();
@@ -521,7 +548,7 @@ fn handle_pull(args: &PullArgs) -> anyhow::Result<()> {
     };
 
     // Use handle_fetch to do the actual fetching and filtering
-    handle_fetch_repo(&fetch_args, &repo_path)?;
+    handle_fetch_repo(&fetch_args, &repo_path, transaction)?;
 
     // Get current working directory for shell commands
     let current_dir = std::env::current_dir()?;
@@ -549,12 +576,15 @@ fn handle_pull(args: &PullArgs) -> anyhow::Result<()> {
     Ok(())
 }
 
-fn handle_fetch(args: &FetchArgs) -> anyhow::Result<()> {
+fn handle_fetch(
+    args: &FetchArgs,
+    transaction: &josh_core::cache::Transaction,
+) -> anyhow::Result<()> {
     // Check if we're in a git repository
     let repo = git2::Repository::open_from_env().context("Not in a git repository")?;
     let repo_path = repo.path().parent().unwrap().to_path_buf();
 
-    handle_fetch_repo(args, &repo_path)
+    handle_fetch_repo(args, &repo_path, transaction)
 }
 
 fn try_parse_symref(remote: &str, output: &str) -> Option<(String, String)> {
@@ -567,7 +597,11 @@ fn try_parse_symref(remote: &str, output: &str) -> Option<(String, String)> {
     Some((default_branch.to_string(), default_branch_ref))
 }
 
-fn handle_fetch_repo(args: &FetchArgs, repo_path: &std::path::Path) -> anyhow::Result<()> {
+fn handle_fetch_repo(
+    args: &FetchArgs,
+    repo_path: &std::path::Path,
+    transaction: &josh_core::cache::Transaction,
+) -> anyhow::Result<()> {
     let repo = git2::Repository::open(repo_path).context("Failed to open repository")?;
 
     // Read the remote URL from josh-remote config
@@ -618,15 +652,15 @@ fn handle_fetch_repo(args: &FetchArgs, repo_path: &std::path::Path) -> anyhow::R
         remote: args.remote.clone(),
     };
 
-    handle_filter_repo(&filter_args, repo_path, false)?;
+    handle_filter_repo(&filter_args, repo_path, false, transaction)?;
 
     // Note: fetch doesn't checkout, it just updates the refs
     eprintln!("Fetched from remote: {}", args.remote);
 
     Ok(())
 }
 
-fn handle_push(args: &PushArgs) -> anyhow::Result<()> {
+fn handle_push(args: &PushArgs, transaction: &josh_core::cache::Transaction) -> anyhow::Result<()> {
     // Read filter from git config for the specific remote
     let repo = git2::Repository::open_from_env().context("Not in a git repository")?;
     let repo_path = repo.path().parent().unwrap();
@@ -643,28 +677,6 @@ fn handle_push(args: &PushArgs) -> anyhow::Result<()> {
         .map_err(from_josh_err)
         .context("Failed to parse filter")?;
 
-    josh_core::cache_sled::sled_load(repo_path)
-        .map_err(from_josh_err)
-        .context("Failed to load sled cache")?;
-
-    let cache = std::sync::Arc::new(
-        josh_core::cache_stack::CacheStack::new()
-            .with_backend(josh_core::cache_sled::SledCacheBackend::default())
-            .with_backend(
-                josh_core::cache_notes::NotesCacheBackend::new(repo_path)
-                    .map_err(from_josh_err)
-                    .context("Failed to create NotesCacheBackend")?,
-            ),
-    );
-
-    // Open Josh transaction
-    let transaction = josh_core::cache::TransactionContext::from_env(cache.clone())
-        .map_err(from_josh_err)
-        .context("Failed TransactionContext::from_env")?
-        .open(None)
-        .map_err(from_josh_err)
-        .context("Failed TransactionContext::open")?;
-
     // Get the remote URL from josh-remote config
     let remote_url = config
         .get_string(&format!("josh-remote.{}.url", args.remote))
@@ -848,14 +860,17 @@ fn handle_push(args: &PushArgs) -> anyhow::Result<()> {
     Ok(())
 }
 
-fn handle_link(args: &LinkArgs) -> anyhow::Result<()> {
+fn handle_link(args: &LinkArgs, transaction: &josh_core::cache::Transaction) -> anyhow::Result<()> {
     match &args.command {
-        LinkCommand::Add(add_args) => handle_link_add(add_args),
-        LinkCommand::Fetch(fetch_args) => handle_link_fetch(fetch_args),
+        LinkCommand::Add(add_args) => handle_link_add(add_args, transaction),
+        LinkCommand::Fetch(fetch_args) => handle_link_fetch(fetch_args, transaction),
     }
 }
 
-fn handle_link_add(args: &LinkAddArgs) -> anyhow::Result<()> {
+fn handle_link_add(
+    args: &LinkAddArgs,
+    transaction: &josh_core::cache::Transaction,
+) -> anyhow::Result<()> {
     use josh_core::filter::tree;
     use josh_core::{JoshLinkFile, Oid};
 
@@ -962,30 +977,8 @@ fn handle_link_add(args: &LinkAddArgs) -> anyhow::Result<()> {
         .map_err(from_josh_err)
         .context("Failed to parse :link filter")?;
 
-    // Load the cache and create transaction
-    let repo_path = repo.path().parent().unwrap();
-
-    josh_core::cache_sled::sled_load(&repo_path).unwrap();
-    let cache = std::sync::Arc::new(
-        josh_core::cache_stack::CacheStack::new()
-            .with_backend(josh_core::cache_sled::SledCacheBackend::default())
-            .with_backend(
-                josh_core::cache_notes::NotesCacheBackend::new(&repo_path)
-                    .map_err(from_josh_err)
-                    .context("Failed to create NotesCacheBackend")?,
-            ),
-    );
-
-    // Open Josh transaction
-    let transaction = josh_core::cache::TransactionContext::from_env(cache.clone())
-        .map_err(from_josh_err)
-        .context("Failed TransactionContext::from_env")?
-        .open(None)
-        .map_err(from_josh_err)
-        .context("Failed TransactionContext::open")?;
-
     let filtered_commit = josh_core::filter_commit(
-        &transaction,
+        transaction,
         link_filter,
         new_commit,
         josh_core::filter::empty(),
@@ -1009,7 +1002,10 @@ fn handle_link_add(args: &LinkAddArgs) -> anyhow::Result<()> {
     Ok(())
 }
 
-fn handle_link_fetch(args: &LinkFetchArgs) -> anyhow::Result<()> {
+fn handle_link_fetch(
+    args: &LinkFetchArgs,
+    transaction: &josh_core::cache::Transaction,
+) -> anyhow::Result<()> {
     use josh_core::filter::tree;
     use josh_core::{JoshLinkFile, Oid};
 
@@ -1147,28 +1143,8 @@ fn handle_link_fetch(args: &LinkFetchArgs) -> anyhow::Result<()> {
         .map_err(from_josh_err)
         .context("Failed to parse :link filter")?;
 
-    // Load the cache and create transaction
-    let repo_path = repo.path().parent().unwrap();
-    josh_core::cache_sled::sled_load(&repo_path).unwrap();
-    let cache = std::sync::Arc::new(
-        josh_core::cache_stack::CacheStack::new()
-            .with_backend(josh_core::cache_sled::SledCacheBackend::default())
-            .with_backend(
-                josh_core::cache_notes::NotesCacheBackend::new(&repo_path)
-                    .map_err(from_josh_err)
-                    .context("Failed to create NotesCacheBackend")?,
-            ),
-    );
-
-    // Open Josh transaction
-    let transaction = josh_core::cache::TransactionContext::from_env(cache.clone())
-        .map_err(from_josh_err)
-        .context("Failed TransactionContext::from_env")?
-        .open(None)
-        .map_err(from_josh_err)
-        .context("Failed TransactionContext::open")?;
     let filtered_commit = josh_core::filter_commit(
-        &transaction,
+        transaction,
         link_filter,
         new_commit,
         josh_core::filter::empty(),
@@ -1189,7 +1165,10 @@ fn handle_link_fetch(args: &LinkFetchArgs) -> anyhow::Result<()> {
     Ok(())
 }
 
-fn handle_remote(args: &RemoteArgs) -> anyhow::Result<()> {
+fn handle_remote(
+    args: &RemoteArgs,
+    _transaction: &josh_core::cache::Transaction,
+) -> anyhow::Result<()> {
     match &args.command {
         RemoteCommand::Add(add_args) => handle_remote_add(add_args),
     }
@@ -1268,18 +1247,22 @@ fn handle_remote_add_repo(args: &RemoteAddArgs, repo_path: &std::path::Path) ->
 }
 
 /// Handle the `josh filter` command - apply filtering to existing refs without fetching
-fn handle_filter(args: &FilterArgs) -> anyhow::Result<()> {
+fn handle_filter(
+    args: &FilterArgs,
+    transaction: &josh_core::cache::Transaction,
+) -> anyhow::Result<()> {
     let repo = git2::Repository::open_from_env().context("Not in a git repository")?;
     let repo_path = repo.path().parent().unwrap().to_path_buf();
 
-    handle_filter_repo(args, &repo_path, true)
+    handle_filter_repo(args, &repo_path, true, transaction)
 }
 
 /// Internal filter function that can be called from other handlers
 fn handle_filter_repo(
     args: &FilterArgs,
     repo_path: &std::path::Path,
     print_messages: bool,
+    transaction: &josh_core::cache::Transaction,
 ) -> anyhow::Result<()> {
     let repo = git2::Repository::open(repo_path).context("Failed to open repository")?;
 
@@ -1296,7 +1279,7 @@ fn handle_filter_repo(
     }
 
     // Apply josh filtering (this is the same as in handle_fetch but without the git fetch step)
-    apply_josh_filtering(repo_path, &filter, &args.remote)?;
+    apply_josh_filtering(transaction, repo_path, &filter, &args.remote)?;
 
     if print_messages {
         println!("Applied filter to remote: {}", args.remote);

tests/cli/clone-http.t

@@ -28,7 +28,7 @@ Test josh clone via HTTP (no filter)
   Fetched from remote: origin
   Already on 'master'
 
-  Cloned repository to: repo1-clone-josh
+  Cloned repository to: ${TESTTMP}/repo1-clone-josh
 
   $ cd repo1-clone-josh
   $ ls
@@ -53,7 +53,7 @@ Test josh clone via HTTP (with filter)
   Fetched from remote: origin
   Already on 'master'
 
-  Cloned repository to: repo1-clone-josh-filtered
+  Cloned repository to: ${TESTTMP}/repo1-clone-josh-filtered
 
   $ cd repo1-clone-josh-filtered
   $ ls
@@ -75,7 +75,7 @@ Test josh clone via HTTP (with explicit filter argument)
   Fetched from remote: origin
   Already on 'master'
 
-  Cloned repository to: repo1-clone-josh-explicit
+  Cloned repository to: ${TESTTMP}/repo1-clone-josh-explicit
 
   $ cd repo1-clone-josh-explicit
   $ ls