From ed6876da1773856e812c6f2294ae8ef196f0a687 Mon Sep 17 00:00:00 2001 From: Essien Ita Essien <34972+essiene@users.noreply.github.com> Date: Sat, 23 Nov 2024 21:04:47 +0000 Subject: [PATCH] jj-lib: abstract a lower-level api for fetching from git. * This allows more control over the stages of the fetch. * Implement `git::fetch` in terms of the new api. * Update tests to explicitly use `git::fetch_and_get_default_branch` where relevant. Issue: #4923 --- lib/src/git.rs | 239 +++++++++++++++++++++++++++++------------- lib/tests/test_git.rs | 37 ++++++- 2 files changed, 200 insertions(+), 76 deletions(-) diff --git a/lib/src/git.rs b/lib/src/git.rs index 6bff6faac8..cce00162b6 100644 --- a/lib/src/git.rs +++ b/lib/src/git.rs @@ -1229,6 +1229,162 @@ pub enum GitFetchError { InternalGitError(#[from] git2::Error), } +fn fetch_options( + callbacks: RemoteCallbacks<'_>, + depth: Option, +) -> 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 +} + +struct FetchedBranches { + branches: Vec, + remote: String, +} + +struct GitFetch<'a> { + mut_repo: &'a mut MutableRepo, + git_repo: &'a git2::Repository, + git_settings: &'a GitSettings, + fetch_options: git2::FetchOptions<'a>, + fetched: Vec, +} + +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, + fetched: vec![], + } + } + + /// Perform a `git fetch` on the local git repo, updating the + /// remote-tracking branches in the git repo. + /// + /// Keeps track of the {branch_names, remote_name} pair the refs can be + /// subsequently imported into the `jj` repo by calling `import_refs()`. + fn fetch( + &mut self, + branch_names: &[StringPattern], + remote_name: &str, + ) -> Result, GitFetchError> { + 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( + /* This triggered by non-glob `*`s in addition to INVALID_REFSPEC_CHARS + * because `to_glob()` escapes such `*`s as `[*]`. */ + |glob| !glob.contains(INVALID_REFSPEC_CHARS), + ) + .map(|glob| format!("+refs/heads/{glob}:refs/remotes/{remote_name}/{glob}")) + }) + .collect::>() + .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, + )?; + + self.fetched.push(FetchedBranches { + branches: branch_names.to_vec(), + remote: remote_name.to_string(), + }); + + // 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()?; + Ok(default_branch) + } + + /// Import the previously fetched 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. + /// + /// Clears all yet-to-be-imported {branch_names, remote_name} pairs after + /// the import. If `fetch()` has not been called since the last time + /// `import_refs()` was called then this will be a no-op. + pub fn import_refs(&mut self) -> Result { + tracing::debug!("import_refs"); + let import_stats = + import_some_refs( + self.mut_repo, + self.git_settings, + |ref_name| match ref_name { + RefName::LocalBranch(_) => false, + RefName::Tag(_) => true, + RefName::RemoteBranch { branch, remote } => { + self.fetched.iter().any(|fetched| { + if fetched.remote != *remote { + return false; + } + + fetched + .branches + .iter() + .any(|pattern| pattern.matches(branch)) + }) + } + }, + )?; + + self.fetched.clear(); + + Ok(import_stats) + } +} + /// Describes successful `fetch()` result. #[derive(Clone, Debug, Eq, PartialEq, Default)] pub struct GitFetchStats { @@ -1248,81 +1404,14 @@ pub fn fetch( git_settings: &GitSettings, depth: Option, ) -> Result { - // 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( - /* This triggered by non-glob `*`s in addition to INVALID_REFSPEC_CHARS - * because `to_glob()` escapes such `*`s as `[*]`. */ - |glob| !glob.contains(INVALID_REFSPEC_CHARS), - ) - .map(|glob| format!("+refs/heads/{glob}:refs/remotes/{remote_name}/{glob}")) - }) - .collect::>() - .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, + 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, diff --git a/lib/tests/test_git.rs b/lib/tests/test_git.rs index 7f5c5e3335..7f773509f3 100644 --- a/lib/tests/test_git.rs +++ b/lib/tests/test_git.rs @@ -2280,7 +2280,7 @@ fn test_fetch_empty_repo() { } #[test] -fn test_fetch_initial_commit() { +fn test_fetch_initial_commit_head_is_not_set() { let test_data = GitRepoData::create(); let git_settings = GitSettings { auto_local_bookmark: true, @@ -2330,6 +2330,41 @@ fn test_fetch_initial_commit() { ); } +#[test] +fn test_fetch_initial_commit_head_is_set() { + let test_data = GitRepoData::create(); + let git_settings = GitSettings { + auto_local_bookmark: true, + ..Default::default() + }; + let initial_git_commit = empty_git_commit(&test_data.origin_repo, "refs/heads/main", &[]); + test_data.origin_repo.set_head("refs/heads/main").unwrap(); + let new_git_commit = empty_git_commit( + &test_data.origin_repo, + "refs/heads/main", + &[&initial_git_commit], + ); + test_data + .origin_repo + .reference("refs/tags/v1.0", new_git_commit.id(), false, "") + .unwrap(); + + let mut tx = test_data.repo.start_transaction(&test_data.settings); + let stats = git::fetch( + tx.repo_mut(), + &test_data.git_repo, + "origin", + &[StringPattern::everything()], + git::RemoteCallbacks::default(), + &git_settings, + None, + ) + .unwrap(); + + assert_eq!(stats.default_branch, Some("main".to_string())); + assert!(stats.import_stats.abandoned_commits.is_empty()); +} + #[test] fn test_fetch_success() { let mut test_data = GitRepoData::create();