Skip to content

Commit

Permalink
fix martinvonz#864 TODO: Changelog, perhaps a test that shows the exa…
Browse files Browse the repository at this point in the history
…ct problem
  • Loading branch information
ilyagr committed May 11, 2023
1 parent f084d49 commit ac70c1d
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 14 deletions.
2 changes: 1 addition & 1 deletion docs/git-compatibility.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ commands and readonly Git commands. It's also useful when tools (e.g. build
tools) expect a Git repo to be present.

There are some bugs and surprising behavior related to `jj undo` in this mode,
such as #864 and #922.
such as #922.

## Creating a repo by cloning a Git repo

Expand Down
36 changes: 25 additions & 11 deletions lib/src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,13 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use std::collections::{BTreeMap, HashSet};
use std::collections::{BTreeMap, HashMap, HashSet};
use std::default::Default;
use std::path::PathBuf;

use git2::Oid;
use itertools::Itertools;
use maplit::hashset;
use thiserror::Error;

use crate::backend::{CommitId, ObjectId};
Expand Down Expand Up @@ -83,12 +84,15 @@ pub fn import_some_refs(
let store = mut_repo.store().clone();
let mut existing_git_refs = mut_repo.view().git_refs().clone();
let mut old_git_heads = vec![];
let mut new_git_heads = HashSet::new();
let mut new_git_heads = HashMap::new();
for (ref_name, old_target) in &existing_git_refs {
if git_ref_filter(ref_name) {
old_git_heads.extend(old_target.adds());
} else {
new_git_heads.extend(old_target.adds());
new_git_heads.insert(
ref_name.to_string(),
old_target.adds().iter().cloned().collect(),
);
}
}

Expand All @@ -103,7 +107,7 @@ pub fn import_some_refs(
// HEAD branch has been rewritten.
let head_commit_id = CommitId::from_bytes(head_git_commit.id().as_bytes());
let head_commit = store.get_commit(&head_commit_id).unwrap();
new_git_heads.insert(head_commit_id.clone());
new_git_heads.insert("HEAD".to_string(), hashset![head_commit_id.clone()]);
prevent_gc(git_repo, &head_commit_id);
mut_repo.add_head(&head_commit);
mut_repo.set_git_head(RefTarget::Normal(head_commit_id));
Expand Down Expand Up @@ -136,7 +140,7 @@ pub fn import_some_refs(
}
};
let id = CommitId::from_bytes(git_commit.id().as_bytes());
new_git_heads.insert(id.clone());
new_git_heads.insert(full_name.to_string(), hashset![id.clone()]);
if !git_ref_filter(&full_name) {
continue;
}
Expand Down Expand Up @@ -168,29 +172,39 @@ pub fn import_some_refs(
continue;
}
if let RefName::RemoteBranch { branch, remote: _ } = ref_name {
// TODO: Make `merge_single_ref` return a value
mut_repo.merge_single_ref(
&RefName::LocalBranch(branch),
&RefName::LocalBranch(branch.clone()),
old_git_target.as_ref(),
new_git_target.as_ref(),
);
if mut_repo.get_local_branch(&branch).is_none() {
// We are deleting this local branch to make it match the change to the
// remote-tracking branches. Note that this only happens if
// the branch was *not* changed locally, otherwise mut_repo.
// get_local_branch would return a conflict at this point.
new_git_heads.remove(&format!("refs/heads/{branch}"));
// TODO: Nice functions
}
}
}
}

// Find commits that are no longer referenced in the git repo and abandon them
// in jj as well. We must remove non-existing commits from new_git_heads, as
// they could have come from branches which were never fetched.
let new_git_heads = new_git_heads
.into_iter()
.filter(|id| mut_repo.index().has_id(id))
.collect_vec();
let mut new_git_heads_set = HashSet::new();
for set in new_git_heads.into_values() {
new_git_heads_set.extend(set.into_iter());
}
new_git_heads_set.retain(|id| mut_repo.index().has_id(id));
// 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
// putting them in the commit message).
let abandoned_commits = mut_repo
.index()
.walk_revs(&old_git_heads, &new_git_heads)
.walk_revs(&old_git_heads, &new_git_heads_set.into_iter().collect_vec())
.map(|entry| entry.commit_id())
.collect_vec();
let root_commit_id = mut_repo.store().root_commit_id().clone();
Expand Down
2 changes: 0 additions & 2 deletions tests/test_git_colocated.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,8 +263,6 @@ fn test_git_colocated_fetch_deleted_branch() {
// TODO: e1f4 should have been abandoned (#864)
insta::assert_snapshot!(get_log_output(&test_env, &clone_path), @r###"
@ 1fa8b2e27c8c3da9764bda953dd81a06fb292d1a
│ ◉ e1f4268fabd2c84e880c5eb5bd87e076180fc8e3
├─╯
◉ a86754f975f953fa25da4265764adc0c62e9ce6b A master HEAD@git
◉ 0000000000000000000000000000000000000000
"###);
Expand Down

0 comments on commit ac70c1d

Please sign in to comment.