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: migrate export_refs() to gix::Repository #2683

Merged
merged 4 commits into from
Dec 9, 2023
Merged

git: migrate export_refs() to gix::Repository #2683

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a22eb83
git: have export_refs() obtain git2::Repository instance from store
yuja Nov 10, 2023
c36443a
git: unimplement PartialEq on GitExportError
yuja Nov 10, 2023
870549f
git: unimplement PartialEq on FailedRefExportReason
yuja Dec 9, 2023
3a7012e
git: migrate export_refs() to gix::Repository
yuja Nov 10, 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
5 changes: 2 additions & 3 deletions cli/src/cli_util.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1386,8 +1386,7 @@ See https://github.com/martinvonz/jj/blob/main/docs/working-copy.md#stale-workin
}

if self.working_copy_shared_with_git {
let git_repo = self.user_repo.git_backend().unwrap().open_git_repo()?;
let failed_branches = git::export_refs(mut_repo, &git_repo)?;
let failed_branches = git::export_refs(mut_repo)?;
print_failed_git_export(ui, &failed_branches)?;
}

Expand Down Expand Up @@ -1462,7 +1461,7 @@ See https://github.com/martinvonz/jj/blob/main/docs/working-copy.md#stale-workin
if let Some(wc_commit) = &maybe_new_wc_commit {
git::reset_head(tx.mut_repo(), &git_repo, wc_commit)?;
}
let failed_branches = git::export_refs(tx.mut_repo(), &git_repo)?;
let failed_branches = git::export_refs(tx.mut_repo())?;
print_failed_git_export(ui, &failed_branches)?;
}
self.user_repo = ReadonlyUserRepo::new(tx.commit());
Expand Down
4 changes: 1 addition & 3 deletions cli/src/commands/git.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1101,10 +1101,8 @@ fn cmd_git_export(
_args: &GitExportArgs,
) -> Result<(), CommandError> {
let mut workspace_command = command.workspace_helper(ui)?;
let repo = workspace_command.repo();
let git_repo = get_git_repo(repo.store())?;
let mut tx = workspace_command.start_transaction("export git refs");
let failed_branches = git::export_refs(tx.mut_repo(), &git_repo)?;
let failed_branches = git::export_refs(tx.mut_repo())?;
tx.finish(ui)?;
print_failed_git_export(ui, &failed_branches)?;
Ok(())
Expand Down
114 changes: 71 additions & 43 deletions lib/src/git.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,10 @@ fn get_git_backend(store: &Store) -> Option<&GitBackend> {
store.backend_impl().downcast_ref()
}

fn get_git_repo(store: &Store) -> Option<gix::Repository> {
get_git_backend(store).map(|backend| backend.git_repo())
}

/// Checks if `git_ref` points to a Git commit object, and returns its id.
///
/// If the ref points to the previously `known_target` (i.e. unchanged), this
Expand Down Expand Up @@ -517,21 +521,29 @@ fn pinned_commit_ids(view: &View) -> impl Iterator<Item = &CommitId> {
.flat_map(|target| target.added_ids())
}

#[derive(Error, Debug, PartialEq)]
#[derive(Error, Debug)]
pub enum GitExportError {
#[error("Git error: {0}")]
InternalGitError(#[from] git2::Error),
InternalGitError(#[source] Box<dyn std::error::Error + Send + Sync>),
#[error("The repo is not backed by a Git repo")]
UnexpectedBackend,
}

impl GitExportError {
fn from_git(source: impl Into<Box<dyn std::error::Error + Send + Sync>>) -> Self {
GitExportError::InternalGitError(source.into())
}
}

/// A ref we failed to export to Git, along with the reason it failed.
#[derive(Debug, PartialEq)]
#[derive(Debug)]
pub struct FailedRefExport {
pub name: RefName,
pub reason: FailedRefExportReason,
}

/// The reason we failed to export a ref to Git.
#[derive(Debug, PartialEq)]
#[derive(Debug)]
pub enum FailedRefExportReason {
/// The name is not allowed in Git.
InvalidGitName,
Expand All @@ -547,15 +559,15 @@ pub enum FailedRefExportReason {
/// We wanted to modify it, but Git had deleted it
ModifiedInJjDeletedInGit,
/// Failed to delete the ref from the Git repo
FailedToDelete(git2::Error),
FailedToDelete(Box<gix::reference::edit::Error>),
/// Failed to set the ref in the Git repo
FailedToSet(git2::Error),
FailedToSet(Box<gix::reference::edit::Error>),
}

#[derive(Debug)]
struct RefsToExport {
branches_to_update: BTreeMap<RefName, (Option<Oid>, Oid)>,
branches_to_delete: BTreeMap<RefName, Oid>,
branches_to_update: BTreeMap<RefName, (Option<gix::ObjectId>, gix::ObjectId)>,
branches_to_delete: BTreeMap<RefName, gix::ObjectId>,
failed_branches: HashMap<RefName, FailedRefExportReason>,
}

Expand All @@ -570,18 +582,16 @@ struct RefsToExport {
/// 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.
pub fn export_refs(
mut_repo: &mut MutableRepo,
git_repo: &git2::Repository,
) -> Result<Vec<FailedRefExport>, GitExportError> {
export_some_refs(mut_repo, git_repo, |_| true)
pub fn export_refs(mut_repo: &mut MutableRepo) -> Result<Vec<FailedRefExport>, GitExportError> {
export_some_refs(mut_repo, |_| true)
}

pub fn export_some_refs(
mut_repo: &mut MutableRepo,
git_repo: &git2::Repository,
git_ref_filter: impl Fn(&RefName) -> bool,
) -> Result<Vec<FailedRefExport>, GitExportError> {
let git_repo = get_git_repo(mut_repo.store()).ok_or(GitExportError::UnexpectedBackend)?;

let RefsToExport {
branches_to_update,
branches_to_delete,
Expand All @@ -594,18 +604,29 @@ pub fn export_some_refs(

// 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) = head_ref
.target()
.try_name()
.and_then(|name| str::from_utf8(name.as_bstr()).ok())
.and_then(parse_git_ref)
{
if let Some(parsed_ref) = parse_git_ref(head_git_ref) {
let old_target = head_ref.inner.target.clone();
if let Ok(current_git_commit_id) = head_ref.into_fully_peeled_id() {
let detach_head =
if let Some((_old_oid, new_oid)) = branches_to_update.get(&parsed_ref) {
*new_oid != current_git_commit.id()
*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())?;
git_repo
.reference(
"HEAD",
current_git_commit_id,
gix::refs::transaction::PreviousValue::MustExistAndMatch(old_target),
"export from jj",
)
.map_err(GitExportError::from_git)?;
}
}
}
Expand All @@ -615,7 +636,7 @@ pub fn export_some_refs(
failed_branches.insert(parsed_ref_name, FailedRefExportReason::InvalidGitName);
continue;
};
if let Err(reason) = delete_git_ref(git_repo, &git_ref_name, old_oid) {
if let Err(reason) = delete_git_ref(&git_repo, &git_ref_name, &old_oid) {
failed_branches.insert(parsed_ref_name, reason);
} else {
let new_target = RefTarget::absent();
Expand All @@ -627,7 +648,7 @@ pub fn export_some_refs(
failed_branches.insert(parsed_ref_name, FailedRefExportReason::InvalidGitName);
continue;
};
if let Err(reason) = update_git_ref(git_repo, &git_ref_name, old_oid, new_oid) {
if let Err(reason) = update_git_ref(&git_repo, &git_ref_name, old_oid, new_oid) {
failed_branches.insert(parsed_ref_name, reason);
} else {
let new_target = RefTarget::normal(CommitId::from_bytes(new_oid.as_bytes()));
Expand Down Expand Up @@ -738,7 +759,7 @@ fn diff_refs_to_export(
continue;
}
let old_oid = if let Some(id) = old_target.as_normal() {
Some(Oid::from_bytes(id.as_bytes()).unwrap())
Some(gix::ObjectId::from(id.as_bytes()))
} 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.
Expand All @@ -749,8 +770,8 @@ fn diff_refs_to_export(
None
};
if let Some(id) = new_target.as_normal() {
let new_oid = Oid::from_bytes(id.as_bytes());
branches_to_update.insert(ref_name, (old_oid, new_oid.unwrap()));
let new_oid = gix::ObjectId::from(id.as_bytes());
branches_to_update.insert(ref_name, (old_oid, new_oid));
} else if new_target.has_conflict() {
// Skip conflicts and leave the old value in git_refs
continue;
Expand All @@ -768,16 +789,16 @@ fn diff_refs_to_export(
}

fn delete_git_ref(
git_repo: &git2::Repository,
git_repo: &gix::Repository,
git_ref_name: &str,
old_oid: Oid,
old_oid: &gix::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) {
if let Ok(git_ref) = git_repo.find_reference(git_ref_name) {
if git_ref.inner.target.try_id() == Some(old_oid) {
// The branch has not been updated by git, so go ahead and delete it
git_repo_ref
git_ref
.delete()
.map_err(FailedRefExportReason::FailedToDelete)?;
.map_err(|err| FailedRefExportReason::FailedToDelete(err.into()))?;
} else {
// The branch was updated by git
return Err(FailedRefExportReason::DeletedInJjModifiedInGit);
Expand All @@ -789,37 +810,44 @@ fn delete_git_ref(
}

fn update_git_ref(
git_repo: &git2::Repository,
git_repo: &gix::Repository,
git_ref_name: &str,
old_oid: Option<Oid>,
new_oid: Oid,
old_oid: Option<gix::ObjectId>,
new_oid: gix::ObjectId,
) -> 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) {
if git_repo_ref.inner.target.try_id() != 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)?;
.reference(
git_ref_name,
new_oid,
gix::refs::transaction::PreviousValue::MustNotExist,
martinvonz marked this conversation as resolved.
Show resolved Hide resolved
"export from jj",
)
.map_err(|err| FailedRefExportReason::FailedToSet(err.into()))?;
}
}
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 branch was modified in jj. We can use gix API for updating under a lock.
if let Err(err) = git_repo.reference(
git_ref_name,
new_oid,
gix::refs::transaction::PreviousValue::MustExistAndMatch(old_oid.into()),
"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));
if git_repo_ref.inner.target.try_id() != Some(&new_oid) {
return Err(FailedRefExportReason::FailedToSet(err.into()));
}
} else {
// The reference was deleted in git and moved in jj
Expand Down
Loading