diff --git a/CHANGELOG.md b/CHANGELOG.md index bd640b6de3..880c776c25 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed bugs +* (#463) A bug in the export of branches to Git caused spurious conflicted + branches. This typically occurred when running in a working copy colocated + with Git (created by running `jj init --git-dir=.`). + * `jj edit root` now fails gracefully. * `jj git import` used to abandon a commit if Git branches and tags referring diff --git a/lib/src/git.rs b/lib/src/git.rs index 707128e131..b3c7eaaa06 100644 --- a/lib/src/git.rs +++ b/lib/src/git.rs @@ -179,10 +179,11 @@ pub enum GitExportError { /// Returns a stripped-down repo view of the state we just exported, to be used /// as `old_view` next time. fn export_changes( + mut_repo: &mut MutableRepo, old_view: &View, - new_view: &View, git_repo: &git2::Repository, ) -> Result { + let new_view = mut_repo.view(); let old_branches: HashSet<_> = old_view.branches().keys().cloned().collect(); let new_branches: HashSet<_> = new_view.branches().keys().cloned().collect(); let mut exported_view = old_view.store_view().clone(); @@ -239,11 +240,16 @@ fn export_changes( } for (git_ref_name, new_target) in refs_to_update { git_repo.reference(&git_ref_name, new_target, true, "export from jj")?; + mut_repo.set_git_ref( + git_ref_name, + RefTarget::Normal(CommitId::from_bytes(new_target.as_bytes())), + ); } for git_ref_name in refs_to_delete { if let Ok(mut git_ref) = git_repo.find_reference(&git_ref_name) { git_ref.delete()?; } + mut_repo.remove_git_ref(&git_ref_name); } Ok(exported_view) } @@ -268,7 +274,7 @@ pub fn export_refs( crate::op_store::View::default() }; let last_export_view = View::new(last_export_store_view); - let new_export_store_view = export_changes(&last_export_view, mut_repo.view(), git_repo)?; + let new_export_store_view = export_changes(mut_repo, &last_export_view, git_repo)?; if let Ok(mut last_export_file) = OpenOptions::new() .write(true) .create(true) diff --git a/lib/src/repo.rs b/lib/src/repo.rs index a995aa8dd7..817c3f8ea7 100644 --- a/lib/src/repo.rs +++ b/lib/src/repo.rs @@ -770,6 +770,10 @@ impl MutableRepo { self.view_mut().remove_tag(name); } + pub fn get_git_ref(&self, name: &str) -> Option { + self.view.borrow().get_git_ref(name) + } + pub fn set_git_ref(&mut self, name: String, target: RefTarget) { self.view_mut().set_git_ref(name, target); } diff --git a/lib/src/view.rs b/lib/src/view.rs index e9f2225956..7da6258717 100644 --- a/lib/src/view.rs +++ b/lib/src/view.rs @@ -232,6 +232,10 @@ impl View { self.data.tags.remove(name); } + pub fn get_git_ref(&self, name: &str) -> Option { + self.data.git_refs.get(name).cloned() + } + pub fn set_git_ref(&mut self, name: String, target: RefTarget) { self.data.git_refs.insert(name, target); } diff --git a/lib/tests/test_git.rs b/lib/tests/test_git.rs index 70d7f98f31..2e7fe641b1 100644 --- a/lib/tests/test_git.rs +++ b/lib/tests/test_git.rs @@ -417,6 +417,10 @@ fn test_export_refs_no_detach() { // Do an initial export to make sure `main` is considered assert_eq!(git::export_refs(mut_repo, &git_repo), Ok(())); + assert_eq!( + mut_repo.get_git_ref("refs/heads/main"), + Some(RefTarget::Normal(jj_id(&commit1))) + ); assert_eq!(git_repo.head().unwrap().name(), Some("refs/heads/main")); assert_eq!( git_repo.find_reference("refs/heads/main").unwrap().target(), @@ -441,6 +445,10 @@ fn test_export_refs_no_op() { // The export should be a no-op since nothing changed on the jj side since last // export assert_eq!(git::export_refs(mut_repo, &git_repo), Ok(())); + assert_eq!( + mut_repo.get_git_ref("refs/heads/main"), + Some(RefTarget::Normal(jj_id(&commit1))) + ); assert_eq!(git_repo.head().unwrap().name(), Some("refs/heads/main")); assert_eq!( git_repo.find_reference("refs/heads/main").unwrap().target(), @@ -477,6 +485,11 @@ fn test_export_refs_branch_changed() { ); mut_repo.remove_local_branch("delete-me"); assert_eq!(git::export_refs(mut_repo, &git_repo), Ok(())); + assert_eq!( + mut_repo.get_git_ref("refs/heads/main"), + Some(RefTarget::Normal(new_commit.id().clone())) + ); + assert_eq!(mut_repo.get_git_ref("refs/heads/delete-me"), None); assert_eq!( git_repo .find_reference("refs/heads/main") @@ -512,6 +525,10 @@ fn test_export_refs_current_branch_changed() { RefTarget::Normal(new_commit.id().clone()), ); assert_eq!(git::export_refs(mut_repo, &git_repo), Ok(())); + assert_eq!( + mut_repo.get_git_ref("refs/heads/main"), + Some(RefTarget::Normal(new_commit.id().clone())) + ); assert_eq!( git_repo .find_reference("refs/heads/main") @@ -543,6 +560,10 @@ fn test_export_refs_unborn_git_branch() { RefTarget::Normal(new_commit.id().clone()), ); assert_eq!(git::export_refs(mut_repo, &git_repo), Ok(())); + assert_eq!( + mut_repo.get_git_ref("refs/heads/main"), + Some(RefTarget::Normal(new_commit.id().clone())) + ); assert_eq!( git_repo .find_reference("refs/heads/main") @@ -579,12 +600,20 @@ fn test_export_import_sequence() { .reference("refs/heads/main", git_id(&commit_a), true, "test") .unwrap(); git::import_refs(mut_repo, &git_repo).unwrap(); + assert_eq!( + mut_repo.get_git_ref("refs/heads/main"), + Some(RefTarget::Normal(commit_a.id().clone())) + ); // Modify the branch in jj to point to B mut_repo.set_local_branch("main".to_string(), RefTarget::Normal(commit_b.id().clone())); // Export the branch to git assert_eq!(git::export_refs(mut_repo, &git_repo), Ok(())); + assert_eq!( + mut_repo.get_git_ref("refs/heads/main"), + Some(RefTarget::Normal(commit_b.id().clone())) + ); // Modify the branch in git to point to C git_repo @@ -593,13 +622,13 @@ fn test_export_import_sequence() { // Import from git git::import_refs(mut_repo, &git_repo).unwrap(); - // TODO: The branch should point to C, it shouldn't be a conflict + assert_eq!( + mut_repo.get_git_ref("refs/heads/main"), + Some(RefTarget::Normal(commit_c.id().clone())) + ); assert_eq!( mut_repo.view().get_local_branch("main"), - Some(RefTarget::Conflict { - removes: vec![commit_a.id().clone()], - adds: vec![commit_b.id().clone(), commit_c.id().clone()], - }) + Some(RefTarget::Normal(commit_c.id().clone())) ); } diff --git a/tests/test_git_colocated.rs b/tests/test_git_colocated.rs index bf04a94d45..e8dd45d5b7 100644 --- a/tests/test_git_colocated.rs +++ b/tests/test_git_colocated.rs @@ -168,10 +168,10 @@ fn test_git_colocated_branches() { "test", ) .unwrap(); - // TODO: Shouldn't be a conflict insta::assert_snapshot!(get_log_output(&test_env, &workspace_root), @r###" - @ 086821b6c35f5fdf07da884b859a14dcf85b5e36 master? - | o 6c0e140886d181602ae7a8e1ac41bc3094842370 master? + Working copy now at: eb08b363bb5e (no description set) + @ eb08b363bb5ef8ee549314260488980d7bbe8f63 + | o 6c0e140886d181602ae7a8e1ac41bc3094842370 master |/ o 230dd059e1b059aefc0da06a2e5a7dbf22362f22 o 0000000000000000000000000000000000000000