Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

git: split export_some_refs() to smaller functions #2207

Merged
merged 6 commits into from
Sep 5, 2023
Merged

git: split export_some_refs() to smaller functions #2207

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9a5669a
git: remove stale TODO comment about FailedRefExportReason
yuja Sep 5, 2023
5399677
git: extract helper that exports single deleted ref
yuja Sep 5, 2023
c2c8595
git: extract helper that exports single updated ref
yuja Sep 5, 2023
a1051bd
git: do not export new ref if racy process created one with the same …
yuja Sep 5, 2023
3dbc106
git: extract calculation step from export_some_refs()
yuja Sep 5, 2023
beea985
git: rename local variables in diff_refs_to_export()
yuja Sep 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
272 changes: 153 additions & 119 deletions lib/src/git.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,13 @@ pub enum FailedRefExportReason {
FailedToSet(git2::Error),
}

#[derive(Debug)]
struct RefsToExport {
branches_to_update: BTreeMap<RefName, (Option<Oid>, Oid)>,
branches_to_delete: BTreeMap<RefName, Oid>,
failed_branches: Vec<FailedRefExport>,
}

/// Export changes to branches made in the Jujutsu repo compared to our last
/// seen view of the Git repo in `mut_repo.view().git_refs()`. Returns a list of
/// refs that failed to export.
Expand All @@ -420,7 +427,6 @@ pub enum FailedRefExportReason {
/// We do not export tags and other refs at the moment, since these aren't
/// supposed to be modified by JJ. For them, the Git state is considered
/// authoritative.
// TODO: Also indicate why we failed to export these branches
martinvonz marked this conversation as resolved.
Show resolved Hide resolved
pub fn export_refs(
mut_repo: &mut MutableRepo,
git_repo: &git2::Repository,
Expand All @@ -433,12 +439,73 @@ pub fn export_some_refs(
git_repo: &git2::Repository,
git_ref_filter: impl Fn(&RefName) -> bool,
) -> Result<Vec<FailedRefExport>, GitExportError> {
// First find the changes we want need to make without modifying mut_repo
let RefsToExport {
branches_to_update,
branches_to_delete,
mut failed_branches,
} = diff_refs_to_export(
mut_repo.view(),
mut_repo.store().root_commit_id(),
git_ref_filter,
);

// TODO: Also check other worktrees' HEAD.
if let Ok(head_ref) = git_repo.find_reference("HEAD") {
if let (Some(head_git_ref), Ok(current_git_commit)) =
(head_ref.symbolic_target(), head_ref.peel_to_commit())
{
if let Some(parsed_ref) = parse_git_ref(head_git_ref) {
let detach_head =
if let Some((_old_oid, new_oid)) = branches_to_update.get(&parsed_ref) {
*new_oid != current_git_commit.id()
} else {
branches_to_delete.contains_key(&parsed_ref)
};
if detach_head {
git_repo.set_head_detached(current_git_commit.id())?;
}
}
}
}
for (parsed_ref_name, old_oid) in branches_to_delete {
let git_ref_name = to_git_ref_name(&parsed_ref_name).unwrap();
if let Err(reason) = delete_git_ref(git_repo, &git_ref_name, old_oid) {
failed_branches.push(FailedRefExport {
name: parsed_ref_name,
reason,
});
} else {
mut_repo.set_git_ref_target(&git_ref_name, RefTarget::absent());
}
}
for (parsed_ref_name, (old_oid, new_oid)) in branches_to_update {
let git_ref_name = to_git_ref_name(&parsed_ref_name).unwrap();
if let Err(reason) = update_git_ref(git_repo, &git_ref_name, old_oid, new_oid) {
failed_branches.push(FailedRefExport {
name: parsed_ref_name,
reason,
});
} else {
mut_repo.set_git_ref_target(
&git_ref_name,
RefTarget::normal(CommitId::from_bytes(new_oid.as_bytes())),
);
}
}
failed_branches.sort_by_key(|failed| failed.name.clone());
Ok(failed_branches)
}

/// Calculates diff of branches to be exported.
fn diff_refs_to_export(
view: &View,
root_commit_id: &CommitId,
git_ref_filter: impl Fn(&RefName) -> bool,
) -> RefsToExport {
let mut branches_to_update = BTreeMap::new();
let mut branches_to_delete = BTreeMap::new();
let mut failed_branches = vec![];
let root_commit_target = RefTarget::normal(mut_repo.store().root_commit_id().clone());
let view = mut_repo.view();
let root_commit_target = RefTarget::normal(root_commit_id.clone());
let jj_repo_iter_all_branches = view.branches().iter().flat_map(|(branch, target)| {
itertools::chain(
target
Expand All @@ -454,177 +521,144 @@ pub fn export_some_refs(
}),
)
});
let jj_known_refs_passing_filter: HashSet<_> = view
let all_ref_names_passing_filter: HashSet<_> = view
.git_refs()
.keys()
.filter_map(|name| parse_git_ref(name))
.chain(jj_repo_iter_all_branches)
.filter(git_ref_filter)
.collect();
for jj_known_ref in jj_known_refs_passing_filter {
let new_branch = match &jj_known_ref {
for ref_name in all_ref_names_passing_filter {
let new_target = match &ref_name {
RefName::LocalBranch(branch) => view.get_local_branch(branch),
RefName::RemoteBranch { remote, branch } => {
// Currently, the only situation where this case occurs *and* new_branch !=
// old_branch is after a `jj branch forget`. So, in practice, for
// remote-tracking branches either `new_branch == old_branch` or
// `new_branch == None`.
// Currently, the only situation where this case occurs *and* new_target !=
// old_target is after a `jj branch forget`. So, in practice, for
// remote-tracking branches either `new_target == old_target` or
// `new_target == None`.
view.get_remote_branch(branch, remote)
}
_ => continue,
};
let old_branch = if let Some(name) = to_git_ref_name(&jj_known_ref) {
let old_target = if let Some(name) = to_git_ref_name(&ref_name) {
view.get_git_ref(&name)
} else {
// Invalid branch name in Git sense
failed_branches.push(FailedRefExport {
name: jj_known_ref,
name: ref_name,
reason: FailedRefExportReason::InvalidGitName,
});
continue;
};
if new_branch == old_branch {
if new_target == old_target {
continue;
}
if *new_branch == root_commit_target {
if *new_target == root_commit_target {
// Git doesn't have a root commit
failed_branches.push(FailedRefExport {
name: jj_known_ref,
name: ref_name,
reason: FailedRefExportReason::OnRootCommit,
});
continue;
}
let old_oid = if let Some(id) = old_branch.as_normal() {
let old_oid = if let Some(id) = old_target.as_normal() {
Some(Oid::from_bytes(id.as_bytes()).unwrap())
} else if old_branch.has_conflict() {
} else if old_target.has_conflict() {
// The old git ref should only be a conflict if there were concurrent import
// operations while the value changed. Don't overwrite these values.
failed_branches.push(FailedRefExport {
name: jj_known_ref,
name: ref_name,
reason: FailedRefExportReason::ConflictedOldState,
});
continue;
} else {
assert!(old_branch.is_absent());
assert!(old_target.is_absent());
None
};
if let Some(id) = new_branch.as_normal() {
if let Some(id) = new_target.as_normal() {
let new_oid = Oid::from_bytes(id.as_bytes());
branches_to_update.insert(jj_known_ref, (old_oid, new_oid.unwrap()));
} else if new_branch.has_conflict() {
branches_to_update.insert(ref_name, (old_oid, new_oid.unwrap()));
} else if new_target.has_conflict() {
// Skip conflicts and leave the old value in git_refs
continue;
} else {
assert!(new_branch.is_absent());
branches_to_delete.insert(jj_known_ref, old_oid.unwrap());
assert!(new_target.is_absent());
branches_to_delete.insert(ref_name, old_oid.unwrap());
}
}
// TODO: Also check other worktrees' HEAD.
if let Ok(head_ref) = git_repo.find_reference("HEAD") {
if let (Some(head_git_ref), Ok(current_git_commit)) =
(head_ref.symbolic_target(), head_ref.peel_to_commit())
{
if let Some(parsed_ref) = parse_git_ref(head_git_ref) {
let detach_head =
if let Some((_old_oid, new_oid)) = branches_to_update.get(&parsed_ref) {
*new_oid != current_git_commit.id()
} else {
branches_to_delete.contains_key(&parsed_ref)
};
if detach_head {
git_repo.set_head_detached(current_git_commit.id())?;
}
}
}

RefsToExport {
branches_to_update,
branches_to_delete,
failed_branches,
}
for (parsed_ref_name, old_oid) in branches_to_delete {
let git_ref_name = to_git_ref_name(&parsed_ref_name).unwrap();
let reason = if let Ok(mut git_repo_ref) = git_repo.find_reference(&git_ref_name) {
if git_repo_ref.target() == Some(old_oid) {
// The branch has not been updated by git, so go ahead and delete it
git_repo_ref
.delete()
.err()
.map(FailedRefExportReason::FailedToDelete)
} else {
// The branch was updated by git
Some(FailedRefExportReason::DeletedInJjModifiedInGit)
}
} else {
// The branch is already deleted
None
};
if let Some(reason) = reason {
failed_branches.push(FailedRefExport {
name: parsed_ref_name,
reason,
});
}

fn delete_git_ref(
git_repo: &git2::Repository,
git_ref_name: &str,
old_oid: Oid,
) -> Result<(), FailedRefExportReason> {
if let Ok(mut git_repo_ref) = git_repo.find_reference(git_ref_name) {
if git_repo_ref.target() == Some(old_oid) {
// The branch has not been updated by git, so go ahead and delete it
git_repo_ref
.delete()
.map_err(FailedRefExportReason::FailedToDelete)?;
} else {
mut_repo.set_git_ref_target(&git_ref_name, RefTarget::absent());
// The branch was updated by git
return Err(FailedRefExportReason::DeletedInJjModifiedInGit);
}
} else {
// The branch is already deleted
}
for (parsed_ref_name, (old_oid, new_oid)) in branches_to_update {
let git_ref_name = to_git_ref_name(&parsed_ref_name).unwrap();
let reason = match old_oid {
None => {
if let Ok(git_repo_ref) = git_repo.find_reference(&git_ref_name) {
// The branch was added in jj and in git. We're good if and only if git
// pointed it to our desired target.
if git_repo_ref.target() == Some(new_oid) {
None
} else {
Some(FailedRefExportReason::AddedInJjAddedInGit)
}
} else {
// The branch was added in jj but still doesn't exist in git, so add it
git_repo
.reference(&git_ref_name, new_oid, true, "export from jj")
.err()
.map(FailedRefExportReason::FailedToSet)
Ok(())
}

fn update_git_ref(
git_repo: &git2::Repository,
git_ref_name: &str,
old_oid: Option<Oid>,
new_oid: Oid,
) -> Result<(), FailedRefExportReason> {
match old_oid {
None => {
if let Ok(git_repo_ref) = git_repo.find_reference(git_ref_name) {
// The branch was added in jj and in git. We're good if and only if git
// pointed it to our desired target.
if git_repo_ref.target() != Some(new_oid) {
return Err(FailedRefExportReason::AddedInJjAddedInGit);
}
} else {
// The branch was added in jj but still doesn't exist in git, so add it
git_repo
.reference(git_ref_name, new_oid, false, "export from jj")
.map_err(FailedRefExportReason::FailedToSet)?;
}
Some(old_oid) => {
// The branch was modified in jj. We can use libgit2's API for updating under a
// lock.
if let Err(err) = git_repo.reference_matching(
&git_ref_name,
new_oid,
true,
old_oid,
"export from jj",
) {
// The reference was probably updated in git
if let Ok(git_repo_ref) = git_repo.find_reference(&git_ref_name) {
// We still consider this a success if it was updated to our desired target
if git_repo_ref.target() == Some(new_oid) {
None
} else {
Some(FailedRefExportReason::FailedToSet(err))
}
} else {
// The reference was deleted in git and moved in jj
Some(FailedRefExportReason::ModifiedInJjDeletedInGit)
}
Some(old_oid) => {
// The branch was modified in jj. We can use libgit2's API for updating under a
// lock.
if let Err(err) =
git_repo.reference_matching(git_ref_name, new_oid, true, old_oid, "export from jj")
{
// The reference was probably updated in git
if let Ok(git_repo_ref) = git_repo.find_reference(git_ref_name) {
// We still consider this a success if it was updated to our desired target
if git_repo_ref.target() != Some(new_oid) {
return Err(FailedRefExportReason::FailedToSet(err));
}
} else {
// Successfully updated from old_oid to new_oid (unchanged in git)
None
// The reference was deleted in git and moved in jj
return Err(FailedRefExportReason::ModifiedInJjDeletedInGit);
}
} else {
// Successfully updated from old_oid to new_oid (unchanged in
// git)
}
};
if let Some(reason) = reason {
failed_branches.push(FailedRefExport {
name: parsed_ref_name,
reason,
});
} else {
mut_repo.set_git_ref_target(
&git_ref_name,
RefTarget::normal(CommitId::from_bytes(new_oid.as_bytes())),
);
}
}
failed_branches.sort_by_key(|failed| failed.name.clone());
Ok(failed_branches)
Ok(())
}

#[derive(Debug, Error)]
Expand Down