mirrors/jj
1
0
Fork 0
mirror of https://github.com/jj-vcs/jj.git synced 2025-12-23 06:01:01 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

cli: resolve: leave executable bit unchanged when using external tool

Browse source 
`jj resolve` will currently try to resolve the executable bit and will
clear it if it can't be resolved. I don't think the executable bit
should be affected by the external merge tool. We could preset the
flags on the files we pass to the merge tool and then expect the merge
tool to update the flags, but we don't do that. I'm not sure that's a
good idea (few merge tools probably expect that), but since we don't
do it, I think it's better to leave the executable bits
unchanged. This patch makes it so by calling
`Merge::with_new_file_ids()` on the original conflict whether or not
the content-level conflict was resolved.

I noticed this was while working on the copy-tracking proposal in PR
#4988, which involves adding copy information in the `TreeValue`. If
we do implement that, then we should preserve copy information here
too (in addition to the executable flag). That will automatically work
after this patch.
This commit is contained in:
Martin von Zweigbergk 2025-01-08 14:11:05 -08:00
parent 9fe2075650
commit a01d0bf773
1 changed files with 6 additions and 13 deletions
Download patch file Download diff file Expand all files Collapse all files

19
cli/src/merge_tools/external.rs
View file

@ -9,7 +9,6 @@ use std::sync::Arc;
use bstr::BString;
use itertools::Itertools;
use jj_lib::backend::MergedTreeId;
use jj_lib::backend::TreeValue;
use jj_lib::conflicts;
use jj_lib::conflicts::choose_materialized_conflict_marker_len;
use jj_lib::conflicts::materialize_merge_result_to_bytes_with_marker_len;
@ -303,19 +302,13 @@ fn run_mergetool_external_single_file(
ExternalToolError::InvalidConflictMarkers { exit_status },
));
}
let new_tree_value = match new_file_ids.into_resolved() {
Ok(new_file_id) => Merge::normal(TreeValue::File {
id: new_file_id.unwrap(),
executable: conflict
.to_executable_merge()
.as_ref()
.and_then(Merge::resolve_trivial)
.copied()
.unwrap_or_default(),
}),
Err(new_file_ids) => conflict.with_new_file_ids(&new_file_ids),
// Update the file ids only, leaving the executable flags unchanged
let new_file_ids = if let Some(resolved) = new_file_ids.as_resolved() {
Merge::from_vec(vec![resolved.clone(); conflict.as_slice().len()])
} else {
new_file_ids
};
let new_tree_value = conflict.with_new_file_ids(&new_file_ids);
tree_builder.set_or_remove(repo_path.to_owned(), new_tree_value);
Ok(())
}