merge: materialize conflicts in executable files like regular files

AFAICT, all callers of `Merge::to_file_merge()` are already well
prepared for working with executable files. It's called from these
places:

* `local_working_copy.rs`: Materialized conflicts are correctly
  updated using `Merge::with_new_file_ids()`.

* `merge_tools/`: Same as above.

* `cmd_cat()`: We already ignore the executable bit when we print
  non-conflicted files, so it makes sense to also ignore it for
  conflicted files.

* `git_diff_part()`: We print all conflicts with mode "100644" (the
  mode for regular files). Maybe it's best to use "100755" for
  conflicts that are unambiguously executable, or maybe it's better to
  use a fake mode like "000000" for all conflicts. Either way, the
  current behavior seems fine.

* `diff_content()`: We use the diff content in various diff
  formats. We could add more detail about the executable bits in some
  of them, but I think the current output is fine. For example,
  instead of our current "Created conflict in my-file", we could say
  "Created conflict in executable file my-file" or "Created conflict
  in ambiguously executable file my-file". That's getting verbose,
  though.

So, I think all we need to do is to make `Merge::to_file_merge()` not
require its inputs to be non-executable.

Closes #1279.
This commit is contained in:
Martin von Zweigbergk 2023-10-23 20:01:00 -07:00 committed by Martin von Zweigbergk
parent daa23173c5
commit 8157d4ff63
3 changed files with 28 additions and 17 deletions

View file

@ -66,6 +66,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
in the working copy no longer leads to a crash
([#976](https://github.com/martinvonz/jj/issues/976)).
* Conflicts in executable files can now be resolved just like conflicts in
non-executable files ([#1279](https://github.com/martinvonz/jj/issues/1279)).
## [0.10.0] - 2023-10-04
### Breaking changes

View file

@ -73,10 +73,13 @@ fn test_chmod_regular_conflict() {
let stdout = test_env.jj_cmd_success(&repo_path, &["cat", "file"]);
insta::assert_snapshot!(stdout,
@r###"
Conflict:
Removing file with id df967b96a579e45a18b8251732d16804b2e56a55
Adding executable file with id 587be6b4c3f93f93c489c0111bba5596147a26cb
Adding file with id 8ba3a16384aacc37d01564b28401755ce8053f51
<<<<<<<
%%%%%%%
-base
+x
+++++++
n
>>>>>>>
"###);
// Test chmodding a conflict
@ -89,10 +92,13 @@ fn test_chmod_regular_conflict() {
let stdout = test_env.jj_cmd_success(&repo_path, &["cat", "file"]);
insta::assert_snapshot!(stdout,
@r###"
Conflict:
Removing executable file with id df967b96a579e45a18b8251732d16804b2e56a55
Adding executable file with id 587be6b4c3f93f93c489c0111bba5596147a26cb
Adding executable file with id 8ba3a16384aacc37d01564b28401755ce8053f51
<<<<<<<
%%%%%%%
-base
+x
+++++++
n
>>>>>>>
"###);
test_env.jj_cmd_ok(&repo_path, &["chmod", "n", "file"]);
let stdout = test_env.jj_cmd_success(&repo_path, &["debug", "tree"]);
@ -232,8 +238,11 @@ fn test_chmod_file_dir_deletion_conflicts() {
let stdout = test_env.jj_cmd_success(&repo_path, &["cat", "-r=file_deletion", "file"]);
insta::assert_snapshot!(stdout,
@r###"
Conflict:
Removing executable file with id df967b96a579e45a18b8251732d16804b2e56a55
Adding executable file with id 78981922613b2afb6025042ff6bd878ac1994e85
<<<<<<<
+++++++
a
%%%%%%%
-base
>>>>>>>
"###);
}

View file

@ -447,15 +447,14 @@ impl Merge<Option<TreeValue>> {
.all(|value| matches!(value, Some(TreeValue::Tree(_)) | None))
}
/// If this merge contains only non-executable files or absent entries,
/// returns a merge of the `FileId`s`.
/// If this merge contains only files or absent entries, returns a merge of
/// the `FileId`s`. The executable bits will be ignored. Use
/// `Merge::with_new_file_ids()` to produce a new merge with the original
/// executable bits preserved.
pub fn to_file_merge(&self) -> Option<Merge<Option<FileId>>> {
self.maybe_map(|term| match term {
None => Some(None),
Some(TreeValue::File {
id,
executable: false,
}) => Some(Some(id.clone())),
Some(TreeValue::File { id, executable: _ }) => Some(Some(id.clone())),
_ => None,
})
}