Skip to content

Commit

Permalink
GitFetch API: Refactor jj git clone to use new GitFetch api directly.
Browse files Browse the repository at this point in the history
* Make the new `GitFetch` api public.
* Rewrite all tests that used `git::fetch*` to use the new API.
  One interesting artifact of the new API is that it holds onto a reference
  of the `tx.repo_mut()`, so the GitFetch object needs to fall go of scope
  completely before we can make any other mutable calls to `tx`.
* Delete the `git::fetch_and_get_default_branch`
  • Loading branch information
essiene committed Nov 30, 2024
1 parent c2e4cb0 commit a846163
Show file tree
Hide file tree
Showing 3 changed files with 175 additions and 188 deletions.
44 changes: 26 additions & 18 deletions cli/src/commands/git/clone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use std::path::PathBuf;

use jj_lib::config::ConfigNamePathBuf;
use jj_lib::git;
use jj_lib::git::GitFetch;
use jj_lib::git::GitFetchError;
use jj_lib::git::GitFetchStats;
use jj_lib::repo::Repo;
Expand Down Expand Up @@ -220,27 +221,34 @@ fn do_git_clone(
git_repo.remote(remote_name, source).unwrap();
let mut fetch_tx = workspace_command.start_transaction();

let stats = with_remote_git_callbacks(ui, None, |cb| {
git::fetch_and_get_default_branch(
let stats = with_remote_git_callbacks(ui, None, |cb| -> Result<GitFetchStats, CommandError> {
let git_settings = command.settings().git_settings();
let mut git_fetch = GitFetch::new(
fetch_tx.repo_mut(),
&git_repo,
remote_name,
&[StringPattern::everything()],
cb,
&command.settings().git_settings(),
depth,
)
})
.map_err(|err| match err {
GitFetchError::NoSuchRemote(_) => {
panic!("shouldn't happen as we just created the git remote")
}
GitFetchError::GitImportError(err) => CommandError::from(err),
GitFetchError::InternalGitError(err) => map_git_error(err),
GitFetchError::InvalidBranchPattern => {
unreachable!("we didn't provide any globs")
}
&git_settings,
git::fetch_options(cb, depth),
);

let default_branch = git_fetch
.fetch(&[StringPattern::everything()], remote_name)
.map_err(|err| match err {
GitFetchError::NoSuchRemote(_) => {
panic!("shouldn't happen as we just created the git remote")
}
GitFetchError::GitImportError(err) => CommandError::from(err),
GitFetchError::InternalGitError(err) => map_git_error(err),
GitFetchError::InvalidBranchPattern => {
unreachable!("we didn't provide any globs")
}
})?;
let import_stats = git_fetch.import_refs()?;
Ok(GitFetchStats {
default_branch,
import_stats,
})
})?;

print_git_import_stats(ui, fetch_tx.repo(), &stats.import_stats, true)?;
fetch_tx.finish(ui, "fetch from git remote into empty repo")?;
Ok((workspace_command, stats))
Expand Down
57 changes: 4 additions & 53 deletions lib/src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1229,7 +1229,7 @@ pub enum GitFetchError {
InternalGitError(#[from] git2::Error),
}

fn fetch_options(
pub fn fetch_options(
callbacks: RemoteCallbacks<'_>,
depth: Option<NonZeroU32>,
) -> git2::FetchOptions<'_> {
Expand All @@ -1251,7 +1251,7 @@ struct FetchedBranches {
remote: String,
}

struct GitFetch<'a> {
pub struct GitFetch<'a> {
mut_repo: &'a mut MutableRepo,
git_repo: &'a git2::Repository,
git_settings: &'a GitSettings,
Expand All @@ -1260,7 +1260,7 @@ struct GitFetch<'a> {
}

impl<'a> GitFetch<'a> {
fn new(
pub fn new(
mut_repo: &'a mut MutableRepo,
git_repo: &'a git2::Repository,
git_settings: &'a GitSettings,
Expand All @@ -1275,7 +1275,7 @@ impl<'a> GitFetch<'a> {
}
}

fn fetch(
pub fn fetch(
&mut self,
branch_names: &[StringPattern],
remote_name: &str,
Expand Down Expand Up @@ -1378,55 +1378,6 @@ pub struct GitFetchStats {
pub import_stats: GitImportStats,
}

#[tracing::instrument(skip(mut_repo, git_repo, callbacks))]
pub fn fetch_and_get_default_branch(
mut_repo: &mut MutableRepo,
git_repo: &git2::Repository,
remote_name: &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,
fetch_options(callbacks, depth),
);
let default_branch = git_fetch.fetch(branch_names, remote_name)?;
let import_stats = git_fetch.import_refs()?;
let stats = GitFetchStats {
default_branch,
import_stats,
};
Ok(stats)
}

pub fn fetch(
mut_repo: &mut MutableRepo,
git_repo: &git2::Repository,
remote_name: &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,
fetch_options(callbacks, depth),
);
git_fetch.fetch(branch_names, remote_name)?;
let import_stats = git_fetch.import_refs()?;
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 a846163

Please sign in to comment.