Skip to content

Commit

Permalink
cli: Refactor a reusable git_fetch to git_util.rs
Browse files Browse the repository at this point in the history 
We will need this in the `jj git sync` command to perform a fetch that behaves
the same as the `jj git fetch` command.

* Refactor out `git_fetch` and move it to `git_util.rs`
* Also move `warn_if_branches_not_found`. It is only used by `git_fetch`
* Move `map_git_error` to `git_util.rs` and update other call sites to import correctly.
* Update using statements as needed.

All tests still pass as this is a noop.

Part of: #1039
  • Loading branch information
@essiene
essiene committed Nov 5, 2024
1 parent be9df56 commit 8588983
Show file tree
Hide file tree
Showing 5 changed files with 106 additions and 100 deletions.
2 changes: 1 addition & 1 deletion cli/src/commands/git/clone.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
80 changes: 2 additions & 78 deletions cli/src/commands/git/fetch.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -69,45 +62,7 @@ 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(
ui,
&tx,
&args.branch,
&remotes.iter().map(StringPattern::exact).collect_vec(),
)?;
git_fetch(ui, &mut tx, &git_repo, &remotes, &args.branch)?;
tx.finish(
ui,
format!("fetch from git remote(s) {}", remotes.iter().join(",")),
Expand Down Expand Up @@ -148,34 +103,3 @@ fn get_all_remotes(git_repo: &git2::Repository) -> Result<Vec<String>, 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(())
}
20 changes: 0 additions & 20 deletions cli/src/commands/git/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion cli/src/commands/git/push.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
102 changes: 102 additions & 0 deletions cli/src/git_util.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -35,15 +36,36 @@ use jj_lib::op_store::RemoteRef;
use jj_lib::repo::ReadonlyRepo;
use jj_lib::repo::Repo;
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<git2::Repository, CommandError> {
match store.backend_impl().downcast_ref::<GitBackend>() {
None => Err(user_error("The repo is not backed by a git repo")),
Expand Down Expand Up @@ -433,3 +455,83 @@ export or their "parent" bookmarks."#,
}
Ok(())
}

pub fn git_fetch(
ui: &mut Ui,
tx: &mut WorkspaceCommandTransaction,
git_repo: &git2::Repository,
remotes: &[String],
branch: &[StringPattern],
) -> Result<(), CommandError> {
let git_settings = tx.settings().git_settings();

for remote in remotes {
let stats = with_remote_git_callbacks(ui, None, |cb| {
git::fetch(
tx.repo_mut(),
git_repo,
remote,
branch,
cb,
&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(())
}

0 comments on commit 8588983

Please sign in to comment.