Skip to content

Commit

Permalink
cli: Refactor out get_fetch_remotes into git_util.rs
Browse files Browse the repository at this point in the history
Similarly to `git_fetch`, this will be used in `jj git sync`, so
moving this out to maintain the same behaviour across `fetch` command and `sync`.

* Refactor out `get_fetch_remotes` to `git_util.rs`.
* inline the different `get_*_remote*` functions into `get_fetch_remotes`;
  The resulting function is still small enough to be easily readable.
* Move `get_single_remote` into `git_util.rs` and update call sites.
* Update use references

All tests still pass.

Issue: #1039
  • Loading branch information
essiene committed Nov 4, 2024
1 parent 4a4ac40 commit e53870d
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 61 deletions.
56 changes: 4 additions & 52 deletions cli/src/commands/git/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,11 @@

use itertools::Itertools;
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::command_error::CommandError;
use crate::commands::git::get_single_remote;
use crate::git_util::get_fetch_remotes;
use crate::git_util::get_git_repo;
use crate::git_util::git_fetch;
use crate::ui::Ui;
Expand Down Expand Up @@ -54,59 +52,13 @@ pub fn cmd_git_fetch(
) -> Result<(), CommandError> {
let mut workspace_command = command.workspace_helper(ui)?;
let git_repo = get_git_repo(workspace_command.repo().store())?;
let remotes = if args.all_remotes {
get_all_remotes(&git_repo)?
} else if args.remotes.is_empty() {
get_default_fetch_remotes(ui, command.settings(), &git_repo)?
} else {
args.remotes.clone()
};
let settings = command.settings();
let remotes = get_fetch_remotes(ui, settings, &git_repo, &args.remotes, args.all_remotes)?;
let mut tx = workspace_command.start_transaction();
git_fetch(
ui,
&mut tx,
command.settings(),
&git_repo,
&remotes,
&args.branch,
)?;
git_fetch(ui, &mut tx, settings, &git_repo, &remotes, &args.branch)?;
tx.finish(
ui,
format!("fetch from git remote(s) {}", remotes.iter().join(",")),
)?;
Ok(())
}

const DEFAULT_REMOTE: &str = "origin";

fn get_default_fetch_remotes(
ui: &Ui,
settings: &UserSettings,
git_repo: &git2::Repository,
) -> Result<Vec<String>, CommandError> {
const KEY: &str = "git.fetch";
if let Ok(remotes) = settings.config().get(KEY) {
Ok(remotes)
} else if let Some(remote) = settings.config().get_string(KEY).optional()? {
Ok(vec![remote])
} else if let Some(remote) = get_single_remote(git_repo)? {
// if nothing was explicitly configured, try to guess
if remote != DEFAULT_REMOTE {
writeln!(
ui.hint_default(),
"Fetching from the only existing remote: {remote}"
)?;
}
Ok(vec![remote])
} else {
Ok(vec![DEFAULT_REMOTE.to_owned()])
}
}

fn get_all_remotes(git_repo: &git2::Repository) -> Result<Vec<String>, CommandError> {
let git_remotes = git_repo.remotes()?;
Ok(git_remotes
.iter()
.filter_map(|x| x.map(ToOwned::to_owned))
.collect())
}
8 changes: 0 additions & 8 deletions cli/src/commands/git/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,3 @@ pub fn maybe_add_gitignore(workspace_command: &WorkspaceCommandHelper) -> Result
Ok(())
}
}

fn get_single_remote(git_repo: &git2::Repository) -> Result<Option<String>, CommandError> {
let git_remotes = git_repo.remotes()?;
Ok(match git_remotes.len() {
1 => git_remotes.get(0).map(ToOwned::to_owned),
_ => None,
})
}
2 changes: 1 addition & 1 deletion cli/src/commands/git/push.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ 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::formatter::Formatter;
use crate::git_util::get_git_repo;
use crate::git_util::get_single_remote;
use crate::git_util::map_git_error;
use crate::git_util::with_remote_git_callbacks;
use crate::git_util::GitSidebandProgressMessageWriter;
Expand Down
46 changes: 46 additions & 0 deletions cli/src/git_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ 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::ConfigResultExt as _;
use jj_lib::settings::UserSettings;
use jj_lib::store::Store;
use jj_lib::str_util::StringPattern;
Expand Down Expand Up @@ -67,6 +68,14 @@ pub fn map_git_error(err: git2::Error) -> CommandError {
}
}

pub fn get_single_remote(git_repo: &git2::Repository) -> Result<Option<String>, CommandError> {
let git_remotes = git_repo.remotes()?;
Ok(match git_remotes.len() {
1 => git_remotes.get(0).map(ToOwned::to_owned),
_ => None,
})
}

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 @@ -535,3 +544,40 @@ fn warn_if_branches_not_found(

Ok(())
}

const DEFAULT_REMOTE: &str = "origin";
pub fn get_fetch_remotes(
ui: &Ui,
settings: &UserSettings,
git_repo: &git2::Repository,
remotes: &[String],
use_all_remotes: bool,
) -> Result<Vec<String>, CommandError> {
if use_all_remotes {
Ok(git_repo
.remotes()?
.iter()
.filter_map(|x| x.map(ToOwned::to_owned))
.collect())
} else if !remotes.is_empty() {
Ok(remotes.iter().map(|s| s.clone()).collect_vec())
} else {
const KEY: &str = "git.fetch";
if let Ok(remotes) = settings.config().get(KEY) {
Ok(remotes)
} else if let Some(remote) = settings.config().get_string(KEY).optional()? {
Ok(vec![remote])
} else if let Some(remote) = get_single_remote(git_repo)? {
// if nothing was explicitly configured, try to guess
if remote != DEFAULT_REMOTE {
writeln!(
ui.hint_default(),
"Fetching from the only existing remote: {remote}"
)?;
}
Ok(vec![remote])
} else {
Ok(vec![DEFAULT_REMOTE.to_owned()])
}
}
}

0 comments on commit e53870d

Please sign in to comment.