ok/jj
1
0
Fork 0
forked from mirrors/jj
Code Pull requests Activity

git: make import_refs() return abandoned commits to caller

Browse source 
It'll be reported to user.
This commit is contained in:
Yuya Nishihara 2023-09-30 13:29:58 +09:00
parent 58de7c292b
commit 16d3bcd4c5
3 changed files with 75 additions and 32 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

15
cli/src/commands/git.rs
View file

@ -10,7 +10,9 @@ use std::{fs, io};
use clap::{ArgGroup, Subcommand};
use itertools::Itertools;
use jj_lib::backend::{CommitId, ObjectId, TreeValue};
use jj_lib::git::{self, parse_gitmodules, GitFetchError, GitPushError, GitRefUpdate};
use jj_lib::git::{
self, parse_gitmodules, GitFetchError, GitFetchStats, GitPushError, GitRefUpdate,
};
use jj_lib::git_backend::GitBackend;
use jj_lib::op_store::{BranchTarget, RefTarget};
use jj_lib::refs::{classify_branch_push_action, BranchPushAction, BranchPushUpdate};
@ -480,11 +482,12 @@ fn cmd_git_clone(
}
}
if let (mut workspace_command, git_repo, Some(default_branch)) = clone_result? {
let (mut workspace_command, git_repo, stats) = clone_result?;
if let Some(default_branch) = &stats.default_branch {
let default_branch_target = workspace_command
.repo()
.view()
.get_remote_branch(&default_branch, "origin");
.get_remote_branch(default_branch, "origin");
if let Some(commit_id) = default_branch_target.as_normal().cloned() {
let mut checkout_tx =
workspace_command.start_transaction("check out git remote's default branch");
@ -509,7 +512,7 @@ fn do_git_clone(
colocate: bool,
source: &str,
wc_path: &Path,
) -> Result<(WorkspaceCommandHelper, git2::Repository, Option<String>), CommandError> {
) -> Result<(WorkspaceCommandHelper, git2::Repository, GitFetchStats), CommandError> {
let (workspace, repo) = if colocate {
let git_repo = git2::Repository::init(wc_path)?;
add_to_git_exclude(ui, &git_repo)?;
@ -524,7 +527,7 @@ fn do_git_clone(
git_repo.remote(remote_name, source).unwrap();
let mut fetch_tx = workspace_command.start_transaction("fetch from git remote into empty repo");
let maybe_default_branch = with_remote_callbacks(ui, |cb| {
let stats = with_remote_callbacks(ui, |cb| {
git::fetch(
fetch_tx.mut_repo(),
&git_repo,
@ -545,7 +548,7 @@ fn do_git_clone(
}
})?;
fetch_tx.finish(ui)?;
Ok((workspace_command, git_repo, maybe_default_branch))
Ok((workspace_command, git_repo, stats))
}
fn with_remote_callbacks<T>(ui: &mut Ui, f: impl FnOnce(git::RemoteCallbacks<'_>) -> T) -> T {

52
lib/src/git.rs
View file

@ -59,6 +59,13 @@ pub enum GitImportError {
InternalGitError(#[from] git2::Error),
}
/// Describes changes made by `import_refs()` or `fetch()`.
#[derive(Clone, Debug, Eq, PartialEq)]
pub struct GitImportStats {
/// Commits superseded by newly imported commits.
pub abandoned_commits: Vec<CommitId>,
}
fn parse_git_ref(ref_name: &str) -> Option<RefName> {
if let Some(branch_name) = ref_name.strip_prefix("refs/heads/") {
// Git CLI says 'HEAD' is not a valid branch name
@ -180,7 +187,7 @@ pub fn import_refs(
mut_repo: &mut MutableRepo,
git_repo: &git2::Repository,
git_settings: &GitSettings,
) -> Result<(), GitImportError> {
) -> Result<GitImportStats, GitImportError> {
import_some_refs(mut_repo, git_repo, git_settings, |_| true)
}
@ -193,7 +200,7 @@ pub fn import_some_refs(
git_repo: &git2::Repository,
git_settings: &GitSettings,
git_ref_filter: impl Fn(&RefName) -> bool,
) -> Result<(), GitImportError> {
) -> Result<GitImportStats, GitImportError> {
// TODO: Should this be a separate function? We may not always want to import
// the Git HEAD (and add it to our set of heads).
let old_git_head = mut_repo.view().git_head();
@ -286,9 +293,17 @@ pub fn import_some_refs(
.cloned()
.collect_vec();
if hidable_git_heads.is_empty() {
return Ok(());
let stats = GitImportStats {
abandoned_commits: vec![],
};
return Ok(stats);
}
let pinned_heads = pinned_commit_ids(mut_repo.view()).cloned().collect_vec();
let pinned_heads = itertools::chain(
pinned_commit_ids(mut_repo.view()),
iter::once(mut_repo.store().root_commit_id()),
)
.cloned()
.collect_vec();
// We could use mut_repo.record_rewrites() here but we know we only need to care
// about abandoned commits for now. We may want to change this if we ever
// add a way of preserving change IDs across rewrites by `git` (e.g. by
@ -297,14 +312,12 @@ pub fn import_some_refs(
.unwrap()
.iter()
.collect_vec();
let root_commit_id = mut_repo.store().root_commit_id().clone();
for abandoned_commit in abandoned_commits {
if abandoned_commit != root_commit_id {
mut_repo.record_abandoned_commit(abandoned_commit);
}
for abandoned_commit in &abandoned_commits {
mut_repo.record_abandoned_commit(abandoned_commit.clone());
}
Ok(())
let stats = GitImportStats { abandoned_commits };
Ok(stats)
}
/// Calculates diff of git refs to be imported.
@ -797,6 +810,15 @@ pub enum GitFetchError {
InternalGitError(#[from] git2::Error),
}
/// Describes successful `fetch()` result.
#[derive(Clone, Debug, Eq, PartialEq)]
pub struct GitFetchStats {
/// Remote's default branch.
pub default_branch: Option<String>,
/// Changes made by the import.
pub import_stats: GitImportStats,
}
#[tracing::instrument(skip(mut_repo, git_repo, callbacks))]
pub fn fetch(
mut_repo: &mut MutableRepo,
@ -805,7 +827,7 @@ pub fn fetch(
branch_name_globs: Option<&[&str]>,
callbacks: RemoteCallbacks<'_>,
git_settings: &GitSettings,
) -> Result<Option<String>, GitFetchError> {
) -> Result<GitFetchStats, GitFetchError> {
let branch_name_filter = {
let regex = if let Some(globs) = branch_name_globs {
let result = regex::RegexSet::new(
@ -913,12 +935,16 @@ pub fn fetch(
// local branches. We also import local tags since remote tags should have
// been merged by Git.
tracing::debug!("import_refs");
import_some_refs(mut_repo, git_repo, git_settings, |ref_name| {
let import_stats = import_some_refs(mut_repo, git_repo, git_settings, |ref_name| {
to_remote_branch(ref_name, remote_name)
.map(&branch_name_filter)
.unwrap_or_else(|| matches!(ref_name, RefName::Tag(_)))
})?;
Ok(default_branch)
let stats = GitFetchStats {
default_branch,
import_stats,
};
Ok(stats)
}
#[derive(Error, Debug, PartialEq)]

40
lib/tests/test_git.rs
View file

@ -105,11 +105,12 @@ fn test_import_refs() {
let git_repo = get_git_repo(repo);
let mut tx = repo.start_transaction(&settings, "test");
git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap();
let stats = git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap();
tx.mut_repo().rebase_descendants(&settings).unwrap();
let repo = tx.commit();
let view = repo.view();
assert!(stats.abandoned_commits.is_empty());
let expected_heads = hashset! {
jj_id(&commit3),
jj_id(&commit4),
@ -201,10 +202,11 @@ fn test_import_refs_reimport() {
.unwrap();
let mut tx = repo.start_transaction(&settings, "test");
git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap();
let stats = git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap();
tx.mut_repo().rebase_descendants(&settings).unwrap();
let repo = tx.commit();
assert!(stats.abandoned_commits.is_empty());
let expected_heads = hashset! {
jj_id(&commit3),
jj_id(&commit4),
@ -228,10 +230,18 @@ fn test_import_refs_reimport() {
let repo = tx.commit();
let mut tx = repo.start_transaction(&settings, "test");
git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap();
let stats = git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap();
tx.mut_repo().rebase_descendants(&settings).unwrap();
let repo = tx.commit();
assert_eq!(
// The order is unstable just because we import heads from Git repo.
HashSet::from_iter(stats.abandoned_commits),
hashset! {
jj_id(&commit4),
jj_id(&commit3),
},
);
let view = repo.view();
let expected_heads = hashset! {
jj_id(&commit5),
@ -1599,7 +1609,7 @@ fn test_fetch_empty_repo() {
let mut tx = test_data
.repo
.start_transaction(&test_data.settings, "test");
let default_branch = git::fetch(
let stats = git::fetch(
tx.mut_repo(),
&test_data.git_repo,
"origin",
@ -1609,7 +1619,8 @@ fn test_fetch_empty_repo() {
)
.unwrap();
// No default branch and no refs
assert_eq!(default_branch, None);
assert_eq!(stats.default_branch, None);
assert!(stats.import_stats.abandoned_commits.is_empty());
assert_eq!(*tx.mut_repo().view().git_refs(), btreemap! {});
assert_eq!(*tx.mut_repo().view().branches(), btreemap! {});
}
@ -1623,7 +1634,7 @@ fn test_fetch_initial_commit() {
let mut tx = test_data
.repo
.start_transaction(&test_data.settings, "test");
let default_branch = git::fetch(
let stats = git::fetch(
tx.mut_repo(),
&test_data.git_repo,
"origin",
@ -1633,7 +1644,8 @@ fn test_fetch_initial_commit() {
)
.unwrap();
// No default branch because the origin repo's HEAD wasn't set
assert_eq!(default_branch, None);
assert_eq!(stats.default_branch, None);
assert!(stats.import_stats.abandoned_commits.is_empty());
let repo = tx.commit();
// The initial commit is visible after git::fetch().
let view = repo.view();
@ -1692,7 +1704,7 @@ fn test_fetch_success() {
let mut tx = test_data
.repo
.start_transaction(&test_data.settings, "test");
let default_branch = git::fetch(
let stats = git::fetch(
tx.mut_repo(),
&test_data.git_repo,
"origin",
@ -1702,7 +1714,8 @@ fn test_fetch_success() {
)
.unwrap();
// The default branch is "main"
assert_eq!(default_branch, Some("main".to_string()));
assert_eq!(stats.default_branch, Some("main".to_string()));
assert!(stats.import_stats.abandoned_commits.is_empty());
let repo = tx.commit();
// The new commit is visible after we fetch again
let view = repo.view();
@ -1738,7 +1751,7 @@ fn test_fetch_success() {
fn test_fetch_prune_deleted_ref() {
let test_data = GitRepoData::create();
let git_settings = GitSettings::default();
empty_git_commit(&test_data.origin_repo, "refs/heads/main", &[]);
let commit = empty_git_commit(&test_data.origin_repo, "refs/heads/main", &[]);
let mut tx = test_data
.repo
@ -1762,7 +1775,7 @@ fn test_fetch_prune_deleted_ref() {
.delete()
.unwrap();
// After re-fetching, the branch should be deleted
git::fetch(
let stats = git::fetch(
tx.mut_repo(),
&test_data.git_repo,
"origin",
@ -1771,6 +1784,7 @@ fn test_fetch_prune_deleted_ref() {
&git_settings,
)
.unwrap();
assert_eq!(stats.import_stats.abandoned_commits, vec![jj_id(&commit)]);
assert!(tx.mut_repo().get_branch("main").is_none());
}
@ -1806,7 +1820,7 @@ fn test_fetch_no_default_branch() {
.set_head_detached(initial_git_commit.id())
.unwrap();
let default_branch = git::fetch(
let stats = git::fetch(
tx.mut_repo(),
&test_data.git_repo,
"origin",
@ -1816,7 +1830,7 @@ fn test_fetch_no_default_branch() {
)
.unwrap();
// There is no default branch
assert_eq!(default_branch, None);
assert_eq!(stats.default_branch, None);
}
#[test]