Skip to content

Commit

Permalink
git: add dummy conflict to index if necessary
Browse files Browse the repository at this point in the history
If the parent tree contains conflicts, we want the index to also contain
a conflict to ensure that the use can't accidentally commit conflict
markers using `git commit`. Since we can't represent conflicts with more
than 2 sides in the Git index, we need to add a dummy conflict in this
case. We use ".jj-do-not-resolve-this-conflict" as the dummy conflict to
indicate to the user that they should not attempt to resolve this
conflict using Git.
  • Loading branch information
scott2000 committed Jan 7, 2025
1 parent 42b390b commit e6a51d6
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 6 deletions.
19 changes: 17 additions & 2 deletions cli/tests/test_git_colocated.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1072,9 +1072,10 @@ fn test_git_colocated_update_index_3_sided_conflict() {
◆ 0000000000000000000000000000000000000000
"#);

// We can't add conflicts with more than 2 sides to the index, so they should
// show as unconflicted. The stat for base.txt should not change.
// We can't add conflicts with more than 2 sides to the index, so we add a dummy
// conflict instead. The stat for base.txt should not change.
insta::assert_snapshot!(get_index_state(&repo_path), @r#"
Ours Mode(FILE) eb8299123d2a ctime=0:0 mtime=0:0 size=0 .jj-do-not-resolve-this-conflict
Unconflicted Mode(FILE) df967b96a579 ctime=[nonzero] mtime=[nonzero] size=5 base.txt
Unconflicted Mode(FILE) dd8f930010b3 ctime=0:0 mtime=0:0 size=0 conflict.txt
Unconflicted Mode(FILE) dd8f930010b3 ctime=0:0 mtime=0:0 size=0 side-1.txt
Expand All @@ -1099,6 +1100,20 @@ fn test_git_colocated_update_index_3_sided_conflict() {

// Index should be the same after `jj new`.
insta::assert_snapshot!(get_index_state(&repo_path), @r#"
Ours Mode(FILE) eb8299123d2a ctime=0:0 mtime=0:0 size=0 .jj-do-not-resolve-this-conflict
Unconflicted Mode(FILE) df967b96a579 ctime=[nonzero] mtime=[nonzero] size=5 base.txt
Unconflicted Mode(FILE) dd8f930010b3 ctime=0:0 mtime=0:0 size=0 conflict.txt
Unconflicted Mode(FILE) dd8f930010b3 ctime=0:0 mtime=0:0 size=0 side-1.txt
Unconflicted Mode(FILE) 7b44e11df720 ctime=0:0 mtime=0:0 size=0 side-2.txt
Unconflicted Mode(FILE) 42f37a71bf20 ctime=0:0 mtime=0:0 size=0 side-3.txt
"#);

// If we add a file named ".jj-do-not-resolve-this-conflict", it should take
// precedence over the dummy conflict.
std::fs::write(repo_path.join(".jj-do-not-resolve-this-conflict"), "file\n").unwrap();
test_env.jj_cmd_ok(&repo_path, &["new"]);
insta::assert_snapshot!(get_index_state(&repo_path), @r#"
Unconflicted Mode(FILE) f73f3093ff86 ctime=0:0 mtime=0:0 size=0 .jj-do-not-resolve-this-conflict
Unconflicted Mode(FILE) df967b96a579 ctime=[nonzero] mtime=[nonzero] size=5 base.txt
Unconflicted Mode(FILE) dd8f930010b3 ctime=0:0 mtime=0:0 size=0 conflict.txt
Unconflicted Mode(FILE) dd8f930010b3 ctime=0:0 mtime=0:0 size=0 side-1.txt
Expand Down
41 changes: 37 additions & 4 deletions lib/src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ use crate::view::View;
pub const REMOTE_NAME_FOR_LOCAL_GIT_REPO: &str = "git";
/// Ref name used as a placeholder to unset HEAD without a commit.
const UNBORN_ROOT_REF_NAME: &str = "refs/jj/root";
/// Dummy file to be added to the index to indicate that the user is editing a
/// commit with a conflict that isn't represented in the Git index.
const INDEX_DUMMY_CONFLICT_FILE: &str = ".jj-do-not-resolve-this-conflict";

#[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Hash, Debug)]
pub enum RefName {
Expand Down Expand Up @@ -1030,8 +1033,8 @@ pub fn reset_head(mut_repo: &mut MutableRepo, wc_commit: &Commit) -> Result<(),
let parent_tree = wc_commit.parent_tree(mut_repo)?;

// Use the merged parent tree as the Git index, allowing `git diff` to show the
// same changes as `jj diff`. If the merged parent tree has 2-sided conflicts,
// then the Git index will also be conflicted.
// same changes as `jj diff`. If the merged parent tree has conflicts, then the
// Git index will also be conflicted.
let mut index = if let Some(tree) = parent_tree.as_merge().as_resolved() {
if tree.id() == mut_repo.store().empty_tree_id() {
// If the tree is empty, gix can fail to load the object (since Git doesn't
Expand Down Expand Up @@ -1124,6 +1127,8 @@ fn build_index_from_merged_tree(
);
};

let mut has_many_sided_conflict = false;

for (path, entry) in merged_tree.entries() {
let entry = entry?;
if let Some(resolved) = entry.as_resolved() {
Expand All @@ -1142,7 +1147,10 @@ fn build_index_from_merged_tree(
// first side as staged. This is preferable to adding the first 2 sides as a
// conflict, since some tools rely on being able to resolve conflicts using the
// index, which could lead to an incorrect conflict resolution if the index
// didn't contain all of the conflict sides.
// didn't contain all of the conflict sides. Instead, we add a dummy conflict of
// a file named ".jj-do-not-resolve-this-conflict" to prevent the user from
// accidentally committing the conflict markers.
has_many_sided_conflict = true;
push_index_entry(
&path,
conflict.first(),
Expand All @@ -1151,9 +1159,34 @@ fn build_index_from_merged_tree(
}
}

// Required after `dangerously_push_entry` for correctness
// Required after `dangerously_push_entry` for correctness. We use do a lookup
// in the index after this, so it must be sorted before we do the lookup.
index.sort_entries();

// If the conflict had an unrepresentable conflict and the dummy file path isn't
// already added in the index, add a dummy file as a conflict.
if has_many_sided_conflict
&& index
.entry_index_by_path(INDEX_DUMMY_CONFLICT_FILE.into())
.is_err()
{
let file_blob = git_repo
.write_blob(
b"The working copy commit contains conflicts which cannot be resolved using Git.\n",
)
.map_err(GitExportError::from_git)?;
index.dangerously_push_entry(
gix::index::entry::Stat::default(),
file_blob.detach(),
gix::index::entry::Flags::from_stage(gix::index::entry::Stage::Ours),
gix::index::entry::Mode::FILE,
INDEX_DUMMY_CONFLICT_FILE.into(),
);
// We need to sort again for correctness before writing the index file since we
// added a new entry.
index.sort_entries();
}

Ok(index)
}

Expand Down

0 comments on commit e6a51d6

Please sign in to comment.