Skip to content

Commit

Permalink
jj-lib: abstract an api for fetching from git.
Browse files Browse the repository at this point in the history 
* This allows more control over the stages of the fetch.
* Provide thin and directed wrappers for clone and fetch
  which allow slightly different overall actions.
* Allow `git::fetch` to handle multiple remotes.
  * One important side-effect to note (as demonstrated in the changed
    cli test) is that when there are multiple remotes, all fetches
    must pass before `import_refs` starts; no branch imports if a 
    fetch from any remote fails.
* Update call sites to use new `git::clone` and `git::fetch`

Fixes: #4923
  • Loading branch information
@essiene
essiene committed Nov 24, 2024
1 parent eceb664 commit ff061c2
Show file tree
Hide file tree
Showing 5 changed files with 267 additions and 128 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 @@ -222,7 +222,7 @@ fn do_git_clone(
let mut fetch_tx = workspace_command.start_transaction();

let stats = with_remote_git_callbacks(ui, None, |cb| {
git::fetch(
git::clone(
fetch_tx.repo_mut(),
&git_repo,
remote_name,
Expand Down
62 changes: 30 additions & 32 deletions cli/src/git_util.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -479,39 +479,37 @@ pub fn git_fetch(
) -> Result<(), CommandError> {
let git_settings = tx.settings().git_settings();

for remote in args.remotes {
let stats = with_remote_git_callbacks(ui, None, |cb| {
git::fetch(
tx.repo_mut(),
repo,
remote,
args.branch,
cb,
&git_settings,
None,
)
})
.map_err(|err| match err {
GitFetchError::InvalidBranchPattern => {
if args
.branch
.iter()
.any(|pattern| pattern.as_exact().is_some_and(|s| s.contains('*')))
{
user_error_with_hint(
err,
"Prefix the pattern with `glob:` to expand `*` as a glob",
)
} else {
user_error(err)
}
let stats = with_remote_git_callbacks(ui, None, |cb| {
git::fetch(
tx.repo_mut(),
repo,
&args.remotes.iter().map(|s| s.as_str()).collect::<Vec<_>>(),
args.branch,
cb,
&git_settings,
None,
)
})
.map_err(|err| match err {
GitFetchError::InvalidBranchPattern => {
if args
.branch
.iter()
.any(|pattern| pattern.as_exact().is_some_and(|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)?;
}
}
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,
Expand Down
10 changes: 2 additions & 8 deletions cli/tests/test_git_fetch.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,10 +237,7 @@ fn test_git_fetch_nonexistent_remote() {
&repo_path,
&["git", "fetch", "--remote", "rem1", "--remote", "rem2"],
);
insta::assert_snapshot!(stderr, @r###"
bookmark: rem1@rem1 [new] untracked
Error: No git remote named 'rem2'
"###);
insta::assert_snapshot!(stderr, @"Error: No git remote named 'rem2'");
// No remote should have been fetched as part of the failing transaction
insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @"");
}
Expand All @@ -254,10 +251,7 @@ fn test_git_fetch_nonexistent_remote_from_config() {
test_env.add_config(r#"git.fetch = ["rem1", "rem2"]"#);

let stderr = &test_env.jj_cmd_failure(&repo_path, &["git", "fetch"]);
insta::assert_snapshot!(stderr, @r###"
bookmark: rem1@rem1 [new] untracked
Error: No git remote named 'rem2'
"###);
insta::assert_snapshot!(stderr, @"Error: No git remote named 'rem2'");
// No remote should have been fetched as part of the failing transaction
insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @"");
}
Expand Down
229 changes: 157 additions & 72 deletions lib/src/git.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1229,6 +1229,126 @@ pub enum GitFetchError {
InternalGitError(#[from] git2::Error),
}

struct GitFetch<'a> {
mut_repo: &'a mut MutableRepo,
git_repo: &'a git2::Repository,
git_settings: &'a GitSettings,
fetch_options: git2::FetchOptions<'a>,
}

impl<'a> GitFetch<'a> {
fn new(
mut_repo: &'a mut MutableRepo,
git_repo: &'a git2::Repository,
git_settings: &'a GitSettings,
fetch_options: git2::FetchOptions<'a>,
) -> Self {
GitFetch {
mut_repo,
git_repo,
git_settings,
fetch_options,
}
}

fn fetch_options(
callbacks: RemoteCallbacks<'_>,
depth: Option<NonZeroU32>,
) -> git2::FetchOptions<'_> {
let mut proxy_options = git2::ProxyOptions::new();
proxy_options.auto();

let mut fetch_options = git2::FetchOptions::new();
fetch_options.proxy_options(proxy_options);
fetch_options.remote_callbacks(callbacks.into_git());
if let Some(depth) = depth {
fetch_options.depth(depth.get().try_into().unwrap_or(i32::MAX));
}

fetch_options
}

fn fetch(
&mut self,
branch_names: &[StringPattern],
remote_name: &str,
) -> Result<Option<String>, GitFetchError> {
// Perform a `git fetch` on the local git repo, updating the remote-tracking
// branches in the git repo.
let mut remote = self.git_repo.find_remote(remote_name).map_err(|err| {
if is_remote_not_found_err(&err) {
GitFetchError::NoSuchRemote(remote_name.to_string())
} else {
GitFetchError::InternalGitError(err)
}
})?;
// At this point, we are only updating Git's remote tracking branches, not the
// local branches.
let refspecs: Vec<_> = branch_names
.iter()
.map(|pattern| {
pattern
.to_glob()
.filter(|glob| !glob.contains(INVALID_REFSPEC_CHARS))
.map(|glob| format!("+refs/heads/{glob}:refs/remotes/{remote_name}/{glob}"))
})
.collect::<Option<_>>()
.ok_or(GitFetchError::InvalidBranchPattern)?;
if refspecs.is_empty() {
// Don't fall back to the base refspecs.
return Ok(None);
}

tracing::debug!("remote.download");
remote.download(&refspecs, Some(&mut self.fetch_options))?;
tracing::debug!("remote.prune");
remote.prune(None)?;
tracing::debug!("remote.update_tips");
remote.update_tips(
None,
git2::RemoteUpdateFlags::empty(),
git2::AutotagOption::Unspecified,
None,
)?;

let mut default_branch = None;
if let Ok(default_ref_buf) = remote.default_branch() {
if let Some(default_ref) = default_ref_buf.as_str() {
// LocalBranch here is the local branch on the remote, so it's really the remote
// branch
if let Some(RefName::LocalBranch(branch_name)) = parse_git_ref(default_ref) {
tracing::debug!(default_branch = branch_name);
default_branch = Some(branch_name);
}
}
}
tracing::debug!("remote.disconnect");
remote.disconnect()?;
Ok(default_branch)
}

pub fn import_refs(
&mut self,
branch_names: &[StringPattern],
remote_names: &[&str],
) -> Result<GitImportStats, GitImportError> {
// Import the remote-tracking branches into the jj repo and update jj's
// local branches. We also import local tags since remote tags should have
// been merged by Git.
tracing::debug!("import_refs");
import_some_refs(self.mut_repo, self.git_settings, |ref_name| {
remote_names
.iter()
.filter_map(|remote_name| {
to_remote_branch(ref_name, remote_name)
.map(|branch| branch_names.iter().any(|pattern| pattern.matches(branch)))
})
.next()
.unwrap_or(matches!(ref_name, RefName::Tag(_)))
})
}
}

/// Describes successful `fetch()` result.
#[derive(Clone, Debug, Eq, PartialEq, Default)]
pub struct GitFetchStats {
Expand All @@ -1239,7 +1359,7 @@ pub struct GitFetchStats {
}

#[tracing::instrument(skip(mut_repo, git_repo, callbacks))]
pub fn fetch(
pub fn clone(
mut_repo: &mut MutableRepo,
git_repo: &git2::Repository,
remote_name: &str,
Expand All @@ -1248,84 +1368,49 @@ pub fn fetch(
git_settings: &GitSettings,
depth: Option<NonZeroU32>,
) -> Result<GitFetchStats, GitFetchError> {
// Perform a `git fetch` on the local git repo, updating the remote-tracking
// branches in the git repo.
let mut remote = git_repo.find_remote(remote_name).map_err(|err| {
if is_remote_not_found_err(&err) {
GitFetchError::NoSuchRemote(remote_name.to_string())
} else {
GitFetchError::InternalGitError(err)
}
})?;
let mut fetch_options = git2::FetchOptions::new();
let mut proxy_options = git2::ProxyOptions::new();
proxy_options.auto();
fetch_options.proxy_options(proxy_options);
let callbacks = callbacks.into_git();
fetch_options.remote_callbacks(callbacks);
if let Some(depth) = depth {
fetch_options.depth(depth.get().try_into().unwrap_or(i32::MAX));
}
// At this point, we are only updating Git's remote tracking branches, not the
// local branches.
let refspecs: Vec<_> = branch_names
.iter()
.map(|pattern| {
pattern
.to_glob()
.filter(|glob| !glob.contains(INVALID_REFSPEC_CHARS))
.map(|glob| format!("+refs/heads/{glob}:refs/remotes/{remote_name}/{glob}"))
})
.collect::<Option<_>>()
.ok_or(GitFetchError::InvalidBranchPattern)?;
if refspecs.is_empty() {
// Don't fall back to the base refspecs.
let stats = GitFetchStats::default();
return Ok(stats);
}
tracing::debug!("remote.download");
remote.download(&refspecs, Some(&mut fetch_options))?;
tracing::debug!("remote.prune");
remote.prune(None)?;
tracing::debug!("remote.update_tips");
remote.update_tips(
None,
git2::RemoteUpdateFlags::empty(),
git2::AutotagOption::Unspecified,
None,
)?;
// TODO: We could make it optional to get the default branch since we only care
// about it on clone.
let mut default_branch = None;
if let Ok(default_ref_buf) = remote.default_branch() {
if let Some(default_ref) = default_ref_buf.as_str() {
// LocalBranch here is the local branch on the remote, so it's really the remote
// branch
if let Some(RefName::LocalBranch(branch_name)) = parse_git_ref(default_ref) {
tracing::debug!(default_branch = branch_name);
default_branch = Some(branch_name);
}
}
}
tracing::debug!("remote.disconnect");
remote.disconnect()?;

// Import the remote-tracking branches into the jj repo and update jj's
// local branches. We also import local tags since remote tags should have
// been merged by Git.
tracing::debug!("import_refs");
let import_stats = import_some_refs(mut_repo, git_settings, |ref_name| {
to_remote_branch(ref_name, remote_name)
.map(|branch| branch_names.iter().any(|pattern| pattern.matches(branch)))
.unwrap_or_else(|| matches!(ref_name, RefName::Tag(_)))
})?;
let mut git_fetch = GitFetch::new(
mut_repo,
git_repo,
git_settings,
GitFetch::fetch_options(callbacks, depth),
);
let default_branch = git_fetch.fetch(branch_names, remote_name)?;
let import_stats = git_fetch.import_refs(branch_names, &[remote_name])?;
let stats = GitFetchStats {
default_branch,
import_stats,
};
Ok(stats)
}

#[tracing::instrument(skip(mut_repo, git_repo, callbacks))]
pub fn fetch(
mut_repo: &mut MutableRepo,
git_repo: &git2::Repository,
remote_names: &[&str],
branch_names: &[StringPattern],
callbacks: RemoteCallbacks<'_>,
git_settings: &GitSettings,
depth: Option<NonZeroU32>,
) -> Result<GitFetchStats, GitFetchError> {
let mut git_fetch = GitFetch::new(
mut_repo,
git_repo,
git_settings,
GitFetch::fetch_options(callbacks, depth),
);

for remote_name in remote_names {
git_fetch.fetch(branch_names, remote_name)?;
}
let import_stats = git_fetch.import_refs(branch_names, remote_names)?;
let stats = GitFetchStats {
default_branch: None,
import_stats,
};
Ok(stats)
}

#[derive(Error, Debug, PartialEq)]
pub enum GitPushError {
#[error("No git remote named '{0}'")]
Expand Down
Loading

0 comments on commit ff061c2

Please sign in to comment.