diff --git a/src/cmd/fold.rs b/src/cmd/fold.rs index 50f17ff2..25bcbc2b 100644 --- a/src/cmd/fold.rs +++ b/src/cmd/fold.rs @@ -11,6 +11,7 @@ use clap::{Arg, ArgGroup}; use crate::{ ext::{CommitExtended, RepositoryExtended}, patch::SingleRevisionSpec, + print_info_message, stack::{InitializationPolicy, Stack, StackAccess, StackStateAccess}, stupid::Stupid, }; @@ -124,7 +125,7 @@ fn run(matches: &clap::ArgMatches) -> Result<()> { let orig_head_tree_id = stack.get_branch_head().tree_id()?.detach(); let base_tree_id = base_commit.tree_id()?.detach(); stupid.read_tree_checkout(orig_head_tree_id, base_tree_id)?; - if let Err(e) = stupid.apply_to_worktree_and_index( + let applied_cleanly = match stupid.apply_to_worktree_and_index( diff.as_ref(), reject_flag, false, @@ -132,24 +133,42 @@ fn run(matches: &clap::ArgMatches) -> Result<()> { None, context_lines, ) { - stupid.read_tree_checkout_hard(orig_head_tree_id)?; - return Err(e); - } + Ok(None) => true, + Ok(Some(output)) => { + // If patch applied with conflicts print output of "git apply". + print_info_message(matches, &output); + false + } + Err(e) => { + // Reset the work tree on failure. + stupid.read_tree_checkout_hard(orig_head_tree_id)?; + return Err(e); + } + }; let applied_tree_id = stupid.write_tree()?; stupid.read_tree_checkout(applied_tree_id, orig_head_tree_id)?; - if stupid.merge_recursive(base_tree_id, orig_head_tree_id, applied_tree_id)? { - Ok(()) - } else { - Err(super::Error::CausedConflicts("merge conflicts".to_string()).into()) + if !stupid.merge_recursive(base_tree_id, orig_head_tree_id, applied_tree_id)? { + return Err(super::Error::CausedConflicts("merge conflicts".to_string()).into()); + } + if !applied_cleanly { + return Err(super::Error::CausedConflicts("patch conflicts".to_string()).into()); } + Ok(()) } else { - stupid.apply_to_worktree_and_index( + match stupid.apply_to_worktree_and_index( diff.as_ref(), reject_flag, false, strip_level, None, context_lines, - ) + ) { + Ok(None) => Ok(()), + Ok(Some(output)) => { + print_info_message(matches, &output); + Err(super::Error::CausedConflicts("patch conflicts".to_string()).into()) + } + Err(e) => Err(e), + } } } diff --git a/src/cmd/import.rs b/src/cmd/import.rs index 5440931b..d2a919e5 100644 --- a/src/cmd/import.rs +++ b/src/cmd/import.rs @@ -15,6 +15,7 @@ use crate::{ color::get_color_stdout, ext::{RepositoryExtended, TimeExtended}, patch::{patchedit, PatchName}, + print_info_message, stack::{InitializationPolicy, Stack, StackAccess, StackStateAccess}, stupid::Stupid, }; @@ -44,8 +45,10 @@ fn make() -> clap::Command { allows the patches source to be fetched from a url instead of from a \ local file.\n\ \n\ - If a patch does not apply cleanly, the failed diff is written to a \ - .stgit-failed.patch file and an empty patch is added to the stack.\n\ + If a patch does not apply cleanly import is aborted unless '--reject' \ + is specified, in which case it will apply to the work tree the parts \ + of the patch that are applicable, leave the rejected hunks in \ + corresponding *.rej files, and add an empty patch to the stack.\n\ \n\ The patch description must be separated from the diff with a \"---\" line.", ) @@ -603,15 +606,15 @@ fn create_patch<'repo>( } }; + let stupid = stack.repo.stupid(); let strip_level = strip_level.or_else(|| matches.get_one::("strip").copied()); let trimmed_diff = diff.trim_end_with(|c| c.is_ascii_whitespace()); - let tree_id = if trimmed_diff.is_empty() || trimmed_diff == b"---" { - stack.get_branch_head().tree_id()?.detach() + let applied_cleanly = if trimmed_diff.is_empty() || trimmed_diff == b"---" { + true } else { - let stupid = stack.repo.stupid(); - stupid.apply_to_worktree_and_index( + match stupid.apply_to_worktree_and_index( diff, matches.get_flag("reject"), matches.get_flag("3way"), @@ -620,11 +623,19 @@ fn create_patch<'repo>( .get_one::("directory") .map(|path_buf| path_buf.as_path()), matches.get_one::("context-lines").copied(), - )?; - - stupid.write_tree()? + ) { + Ok(None) => true, + Ok(Some(output)) => { + print_info_message(matches, &output); + false + } + Err(e) => return Err(e), + } }; + // If the patch was empty this will not create a new tree object. + let tree_id = stupid.write_tree()?; + let (new_patchname, commit_id) = match crate::patch::edit::EditBuilder::default() .original_patchname(Some(&patchname)) .override_tree_id(tree_id) @@ -647,7 +658,7 @@ fn create_patch<'repo>( ), }; - stack + let stack = stack .setup_transaction() .with_output_stream(get_color_stdout(matches)) .use_index_and_worktree(false) @@ -658,7 +669,13 @@ fn create_patch<'repo>( } trans.new_applied(&new_patchname, commit_id) }) - .execute(&format!("import: {new_patchname}")) + .execute(&format!("import: {new_patchname}"))?; + + if !applied_cleanly { + return Err(super::Error::CausedConflicts("patch conflicts".to_string()).into()); + } + + Ok(stack) } fn stripname(name: &str) -> &str { diff --git a/src/stupid/context.rs b/src/stupid/context.rs index 9b3ed385..f1b45181 100644 --- a/src/stupid/context.rs +++ b/src/stupid/context.rs @@ -121,6 +121,11 @@ impl<'repo, 'index> StupidContext<'repo, 'index> { Ok(()) } + /// Apply a patch (diff) to both the index and the working tree. + /// + /// Returns `None` if the patch applies cleanly or output of the `git apply` + /// command if the patch does not apply and caller indicated that rejects + /// are allowed. pub(crate) fn apply_to_worktree_and_index( &self, diff: &BStr, @@ -129,7 +134,7 @@ impl<'repo, 'index> StupidContext<'repo, 'index> { strip_level: Option, directory: Option<&Path>, context_lines: Option, - ) -> Result<()> { + ) -> Result> { let mut command = self.git_in_work_root()?; command.args(["apply", "--index"]); if reject { @@ -148,11 +153,17 @@ impl<'repo, 'index> StupidContext<'repo, 'index> { if let Some(context_lines) = context_lines { command.arg(format!("-C{context_lines}")); } - command - .stdout(Stdio::null()) - .in_and_out(diff)? - .require_success("apply --index")?; - Ok(()) + let apply_output = command.stdout(Stdio::null()).in_and_out(diff)?; + if apply_output.status.success() { + Ok(None) + } else { + let err = git_command_error("apply --index", &apply_output.stderr); + if reject && apply_output.status.code() == Some(1) { + Ok(Some(format!("{err:#}"))) + } else { + Err(err) + } + } } /// Apply diff between two trees to specified index. diff --git a/t/t1800-import.sh b/t/t1800-import.sh index bf323f79..31557954 100755 --- a/t/t1800-import.sh +++ b/t/t1800-import.sh @@ -366,6 +366,16 @@ test_expect_success 'Import series from stdin' ' stg delete --top ' +test_expect_success 'Import patch that does not apply cleanly with --reject' ' + conflict stg import --reject "$TEST_DIRECTORY"/t1800/diff-with-rejects 2>err && + git log -1 --pretty=format:%b >body && + test_when_finished "rm foo.txt.rej err body" && + test "$(echo $(stg top))" = "diff-with-rejects" && + test_line_count = 0 body && + stg reset --hard && + stg delete --top +' + test_expect_success STG_IMPORT_URL 'Attempt url' ' general_error stg import --url 2>err && grep -e "required arguments were not provided" err diff --git a/t/t1800/diff-with-rejects b/t/t1800/diff-with-rejects new file mode 100644 index 00000000..1fdace95 --- /dev/null +++ b/t/t1800/diff-with-rejects @@ -0,0 +1,29 @@ +test patch with rejects + +--- + + t/t1800/foo.txt | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +diff --git a/t/t1800/foo.txt b/t/t1800/foo.txt +index ad01662f..d609de80 100644 +--- a/foo.txt ++++ b/foo.txt +@@ -6,7 +6,7 @@ dobidim + dobidum + dobodam + dobodim +-dobodum ++dObodum + dibedam + dibedim + dibedum +@@ -18,7 +18,7 @@ dibodim + dibodum + dabedam + dabedim +-Dabedum ++dAbedum + dabidam + dabidim + dabidum diff --git a/t/t3600-fold.sh b/t/t3600-fold.sh index 392ca243..9c85599d 100755 --- a/t/t3600-fold.sh +++ b/t/t3600-fold.sh @@ -91,7 +91,7 @@ test_expect_success 'Attempt to fold conflicting patch with rejects' ' echo "hello" >foo.txt && echo "from p2" >>foo.txt && stg refresh && - command_error stg fold --reject fold1.diff 2>err && + conflict stg fold --reject fold1.diff 2>err && grep "patch failed" err && test -z "$(echo $(stg status --porcelain foo.txt))" && test -e foo.txt.rej &&