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

git: when exporting, skip conflicted branches

Browse source
This commit is contained in:
Martin von Zweigbergk 2022-10-31 09:10:22 -07:00 committed by Martin von Zweigbergk
parent 759ddd1e60
commit 5eba305844
4 changed files with 46 additions and 23 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

27
lib/src/git.rs
View file

@ -13,6 +13,7 @@
// limitations under the License.
use std::collections::{BTreeMap, BTreeSet, HashSet};
use std::default::Default;
use std::fs::OpenOptions;
use std::io::Read;
use std::path::PathBuf;
@ -24,7 +25,7 @@ use thiserror::Error;
use crate::backend::CommitId;
use crate::commit::Commit;
use crate::op_store::{OperationId, RefTarget};
use crate::op_store::{BranchTarget, OperationId, RefTarget};
use crate::repo::{MutableRepo, ReadonlyRepo};
use crate::view::{RefName, View};
use crate::{op_store, simple_op_store, simple_op_store_model};
@ -175,13 +176,16 @@ pub enum GitExportError {
}
/// Reflect changes between two Jujutsu repo views in the underlying Git repo.
pub fn export_changes(
/// Returns a stripped-down repo view of the state we just exported, to be used
/// as `old_view` next time.
fn export_changes(
old_view: &View,
new_view: &View,
git_repo: &git2::Repository,
) -> Result<(), GitExportError> {
) -> Result<crate::op_store::View, GitExportError> {
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();
// First find the changes we want need to make and then make them all at once to
// reduce the risk of making some changes before we fail.
let mut refs_to_update = BTreeMap::new();
@ -196,16 +200,25 @@ pub fn export_changes(
if let Some(new_branch) = new_branch {
match new_branch {
RefTarget::Normal(id) => {
exported_view.branches.insert(
branch_name.clone(),
BranchTarget {
local_target: Some(RefTarget::Normal(id.clone())),
remote_targets: Default::default(),
},
);
refs_to_update.insert(
git_ref_name.clone(),
Oid::from_bytes(id.as_bytes()).unwrap(),
);
}
RefTarget::Conflict { .. } => {
return Err(GitExportError::ConflictedBranch(branch_name.to_string()));
// Skip conflicts and leave the old value in `exported_view`
continue;
}
}
} else {
exported_view.branches.remove(branch_name);
refs_to_delete.insert(git_ref_name.clone());
}
}
@ -232,7 +245,7 @@ pub fn export_changes(
git_ref.delete()?;
}
}
Ok(())
Ok(exported_view)
}
/// Reflect changes made in the Jujutsu repo since last export in the underlying
@ -254,13 +267,13 @@ pub fn export_refs(
op_store::View::default()
};
let last_export_view = View::new(last_export_store_view);
export_changes(&last_export_view, repo.view(), git_repo)?;
let new_export_store_view = export_changes(&last_export_view, repo.view(), git_repo)?;
if let Ok(mut last_export_file) = OpenOptions::new()
.write(true)
.create(true)
.open(&last_export_path)
{
let thrift_view = simple_op_store_model::View::from(repo.operation().view().store_view());
let thrift_view = simple_op_store_model::View::from(&new_export_store_view);
simple_op_store::write_thrift(&thrift_view, &mut last_export_file)
.map_err(|err| GitExportError::WriteStateError(err.to_string()))?;
}

20
lib/tests/test_git.rs
View file

@ -19,7 +19,7 @@ use git2::Oid;
use jujutsu_lib::backend::CommitId;
use jujutsu_lib::commit::Commit;
use jujutsu_lib::git;
use jujutsu_lib::git::{GitExportError, GitFetchError, GitPushError, GitRefUpdate};
use jujutsu_lib::git::{GitFetchError, GitPushError, GitRefUpdate};
use jujutsu_lib::git_backend::GitBackend;
use jujutsu_lib::op_store::{BranchTarget, RefTarget};
use jujutsu_lib::repo::ReadonlyRepo;
@ -659,10 +659,22 @@ fn test_export_conflicts() {
},
);
test_data.repo = tx.commit();
// TODO: Make it succeed instead, just skipping the conflicted branch
assert_eq!(git::export_refs(&test_data.repo, &git_repo), Ok(()));
assert_eq!(
git::export_refs(&test_data.repo, &git_repo),
Err(GitExportError::ConflictedBranch("feature".to_string()))
git_repo
.find_reference("refs/heads/feature")
.unwrap()
.target()
.unwrap(),
git_id(&commit_a)
);
assert_eq!(
git_repo
.find_reference("refs/heads/main")
.unwrap()
.target()
.unwrap(),
git_id(&commit_b)
);
}

11
src/cli_util.rs
View file

@ -165,14 +165,9 @@ impl From<GitImportError> for CommandError {
impl From<GitExportError> for CommandError {
fn from(err: GitExportError) -> Self {
match err {
GitExportError::ConflictedBranch(branch_name) => {
user_error(format!("Cannot export conflicted branch '{branch_name}'"))
}
err => CommandError::InternalError(format!(
"Failed to export refs to underlying Git repo: {err}"
)),
}
CommandError::InternalError(format!(
"Failed to export refs to underlying Git repo: {err}"
))
}
}

11
tests/test_git_colocated.rs
View file

@ -168,10 +168,13 @@ fn test_git_colocated_branches() {
"test",
)
.unwrap();
// TODO: Shouldn't be a conflict (https://github.com/martinvonz/jj/issues/463)
let stderr = test_env.jj_cmd_failure(&workspace_root, &["st"]);
insta::assert_snapshot!(stderr, @r###"
Error: Cannot export conflicted branch 'master'
// TODO: Shouldn't be a conflict
insta::assert_snapshot!(get_log_output(&test_env, &workspace_root), @r###"
@ 086821b6c35f5fdf07da884b859a14dcf85b5e36 master?
| o 6c0e140886d181602ae7a8e1ac41bc3094842370 master?
|/
o 230dd059e1b059aefc0da06a2e5a7dbf22362f22
o 0000000000000000000000000000000000000000
"###);
}