From d6df47f061d631bf6536d8a83411c9e2378a857d Mon Sep 17 00:00:00 2001 From: Essien Ita Essien <34972+essiene@users.noreply.github.com> Date: Sat, 2 Nov 2024 11:04:02 +0000 Subject: [PATCH] cli: Refactor a reusable `git_fetch` to git_util.rs We will need this in the `jj git sync` command to perform a fetch that behaves the same as the `jj git fetch` command. Part of: #1039 --- cli/src/commands/git/clone.rs | 2 +- cli/src/commands/git/fetch.rs | 81 ++------------------------- cli/src/commands/git/mod.rs | 20 ------- cli/src/commands/git/push.rs | 2 +- cli/src/git_util.rs | 102 ++++++++++++++++++++++++++++++++++ 5 files changed, 110 insertions(+), 97 deletions(-) diff --git a/cli/src/commands/git/clone.rs b/cli/src/commands/git/clone.rs index 7761c7ed66..f9e1488824 100644 --- a/cli/src/commands/git/clone.rs +++ b/cli/src/commands/git/clone.rs @@ -32,11 +32,11 @@ use crate::command_error::cli_error; use crate::command_error::user_error; use crate::command_error::user_error_with_message; use crate::command_error::CommandError; -use crate::commands::git::map_git_error; use crate::commands::git::maybe_add_gitignore; use crate::config::write_config_value_to_file; use crate::config::ConfigNamePathBuf; use crate::git_util::get_git_repo; +use crate::git_util::map_git_error; use crate::git_util::print_git_import_stats; use crate::git_util::with_remote_git_callbacks; use crate::ui::Ui; diff --git a/cli/src/commands/git/fetch.rs b/cli/src/commands/git/fetch.rs index 8413895072..808cdf3c73 100644 --- a/cli/src/commands/git/fetch.rs +++ b/cli/src/commands/git/fetch.rs @@ -13,23 +13,16 @@ // limitations under the License. use itertools::Itertools; -use jj_lib::git; -use jj_lib::git::GitFetchError; use jj_lib::repo::Repo; use jj_lib::settings::ConfigResultExt as _; use jj_lib::settings::UserSettings; use jj_lib::str_util::StringPattern; use crate::cli_util::CommandHelper; -use crate::cli_util::WorkspaceCommandTransaction; -use crate::command_error::user_error; -use crate::command_error::user_error_with_hint; use crate::command_error::CommandError; use crate::commands::git::get_single_remote; -use crate::commands::git::map_git_error; use crate::git_util::get_git_repo; -use crate::git_util::print_git_import_stats; -use crate::git_util::with_remote_git_callbacks; +use crate::git_util::git_fetch; use crate::ui::Ui; /// Fetch from a Git remote @@ -69,44 +62,13 @@ pub fn cmd_git_fetch( args.remotes.clone() }; let mut tx = workspace_command.start_transaction(); - for remote in &remotes { - let stats = with_remote_git_callbacks(ui, None, |cb| { - git::fetch( - tx.repo_mut(), - &git_repo, - remote, - &args.branch, - cb, - &command.settings().git_settings(), - None, - ) - }) - .map_err(|err| match err { - GitFetchError::InvalidBranchPattern => { - if args - .branch - .iter() - .any(|pattern| pattern.as_exact().map_or(false, |s| s.contains('*'))) - { - user_error_with_hint( - err, - "Prefix the pattern with `glob:` to expand `*` as a glob", - ) - } else { - user_error(err) - } - } - GitFetchError::GitImportError(err) => err.into(), - GitFetchError::InternalGitError(err) => map_git_error(err), - _ => user_error(err), - })?; - print_git_import_stats(ui, tx.repo(), &stats.import_stats, true)?; - } - warn_if_branches_not_found( + git_fetch( ui, - &tx, + &mut tx, + command.settings(), + &git_repo, + &remotes, &args.branch, - &remotes.iter().map(StringPattern::exact).collect_vec(), )?; tx.finish( ui, @@ -148,34 +110,3 @@ fn get_all_remotes(git_repo: &git2::Repository) -> Result, CommandEr .filter_map(|x| x.map(ToOwned::to_owned)) .collect()) } - -fn warn_if_branches_not_found( - ui: &mut Ui, - tx: &WorkspaceCommandTransaction, - branches: &[StringPattern], - remotes: &[StringPattern], -) -> Result<(), CommandError> { - for branch in branches { - let matches = remotes.iter().any(|remote| { - tx.repo() - .view() - .remote_bookmarks_matching(branch, remote) - .next() - .is_some() - || tx - .base_repo() - .view() - .remote_bookmarks_matching(branch, remote) - .next() - .is_some() - }); - if !matches { - writeln!( - ui.warning_default(), - "No branch matching `{branch}` found on any specified/configured remote", - )?; - } - } - - Ok(()) -} diff --git a/cli/src/commands/git/mod.rs b/cli/src/commands/git/mod.rs index 2a418751b6..1c7fee9818 100644 --- a/cli/src/commands/git/mod.rs +++ b/cli/src/commands/git/mod.rs @@ -41,8 +41,6 @@ use self::submodule::cmd_git_submodule; use self::submodule::GitSubmoduleCommand; use crate::cli_util::CommandHelper; use crate::cli_util::WorkspaceCommandHelper; -use crate::command_error::user_error; -use crate::command_error::user_error_with_hint; use crate::command_error::user_error_with_message; use crate::command_error::CommandError; use crate::ui::Ui; @@ -82,24 +80,6 @@ pub fn cmd_git( } } -fn map_git_error(err: git2::Error) -> CommandError { - if err.class() == git2::ErrorClass::Ssh { - let hint = - if err.code() == git2::ErrorCode::Certificate && std::env::var_os("HOME").is_none() { - "The HOME environment variable is not set, and might be required for Git to \ - successfully load certificates. Try setting it to the path of a directory that \ - contains a `.ssh` directory." - } else { - "Jujutsu uses libssh2, which doesn't respect ~/.ssh/config. Does `ssh -F \ - /dev/null` to the host work?" - }; - - user_error_with_hint(err, hint) - } else { - user_error(err.to_string()) - } -} - pub fn maybe_add_gitignore(workspace_command: &WorkspaceCommandHelper) -> Result<(), CommandError> { if workspace_command.working_copy_shared_with_git() { std::fs::write( diff --git a/cli/src/commands/git/push.rs b/cli/src/commands/git/push.rs index c8295c9624..b519e9e972 100644 --- a/cli/src/commands/git/push.rs +++ b/cli/src/commands/git/push.rs @@ -46,9 +46,9 @@ use crate::command_error::user_error; use crate::command_error::user_error_with_hint; use crate::command_error::CommandError; use crate::commands::git::get_single_remote; -use crate::commands::git::map_git_error; use crate::formatter::Formatter; use crate::git_util::get_git_repo; +use crate::git_util::map_git_error; use crate::git_util::with_remote_git_callbacks; use crate::git_util::GitSidebandProgressMessageWriter; use crate::ui::Ui; diff --git a/cli/src/git_util.rs b/cli/src/git_util.rs index 1c98c6f5d8..9cd8dd5124 100644 --- a/cli/src/git_util.rs +++ b/cli/src/git_util.rs @@ -27,6 +27,7 @@ use itertools::Itertools; use jj_lib::git; use jj_lib::git::FailedRefExport; use jj_lib::git::FailedRefExportReason; +use jj_lib::git::GitFetchError; use jj_lib::git::GitImportStats; use jj_lib::git::RefName; use jj_lib::git_backend::GitBackend; @@ -34,16 +35,38 @@ use jj_lib::op_store::RefTarget; use jj_lib::op_store::RemoteRef; use jj_lib::repo::ReadonlyRepo; use jj_lib::repo::Repo; +use jj_lib::settings::UserSettings; use jj_lib::store::Store; +use jj_lib::str_util::StringPattern; use jj_lib::workspace::Workspace; use unicode_width::UnicodeWidthStr; +use crate::cli_util::WorkspaceCommandTransaction; use crate::command_error::user_error; +use crate::command_error::user_error_with_hint; use crate::command_error::CommandError; use crate::formatter::Formatter; use crate::progress::Progress; use crate::ui::Ui; +pub fn map_git_error(err: git2::Error) -> CommandError { + if err.class() == git2::ErrorClass::Ssh { + let hint = + if err.code() == git2::ErrorCode::Certificate && std::env::var_os("HOME").is_none() { + "The HOME environment variable is not set, and might be required for Git to \ + successfully load certificates. Try setting it to the path of a directory that \ + contains a `.ssh` directory." + } else { + "Jujutsu uses libssh2, which doesn't respect ~/.ssh/config. Does `ssh -F \ + /dev/null` to the host work?" + }; + + user_error_with_hint(err, hint) + } else { + user_error(err.to_string()) + } +} + pub fn get_git_repo(store: &Store) -> Result { match store.backend_impl().downcast_ref::() { None => Err(user_error("The repo is not backed by a git repo")), @@ -433,3 +456,82 @@ export or their "parent" bookmarks."#, } Ok(()) } + +pub fn git_fetch( + ui: &mut Ui, + tx: &mut WorkspaceCommandTransaction, + settings: &UserSettings, + git_repo: &git2::Repository, + remotes: &[String], + branch: &[StringPattern], +) -> Result<(), CommandError> { + for remote in remotes { + let stats = with_remote_git_callbacks(ui, None, |cb| { + git::fetch( + tx.repo_mut(), + git_repo, + remote, + branch, + cb, + &settings.git_settings(), + None, + ) + }) + .map_err(|err| match err { + GitFetchError::InvalidBranchPattern => { + if branch + .iter() + .any(|pattern| pattern.as_exact().map_or(false, |s| s.contains('*'))) + { + user_error_with_hint( + err, + "Prefix the pattern with `glob:` to expand `*` as a glob", + ) + } else { + user_error(err) + } + } + GitFetchError::GitImportError(err) => err.into(), + GitFetchError::InternalGitError(err) => map_git_error(err), + _ => user_error(err), + })?; + print_git_import_stats(ui, tx.repo(), &stats.import_stats, true)?; + } + warn_if_branches_not_found( + ui, + tx, + branch, + &remotes.iter().map(StringPattern::exact).collect_vec(), + ) +} + +fn warn_if_branches_not_found( + ui: &mut Ui, + tx: &WorkspaceCommandTransaction, + branches: &[StringPattern], + remotes: &[StringPattern], +) -> Result<(), CommandError> { + for branch in branches { + let matches = remotes.iter().any(|remote| { + tx.repo() + .view() + .remote_bookmarks_matching(branch, remote) + .next() + .is_some() + || tx + .base_repo() + .view() + .remote_bookmarks_matching(branch, remote) + .next() + .is_some() + }); + if !matches { + writeln!( + ui.warning_default(), + "No branch matching `{branch}` found on any specified/configured remote", + )?; + } + } + + Ok(()) +}