From: Elijah Newren <newren@xxxxxxxxx>
Similar to the previous commit, just as `git commit --amend` is a
foot-gun during conflict resolution, so is a partial commit (`git commit
<paths>`). Recording a conflict resolution is about capturing the state
of the entire tree on top of HEAD, not a subset of paths. For many years
we have rejected partial commits in the middle of
- a merge
- a cherry-pick
but, just like amending, this was never extended to the other operations
that can also leave conflicts to resolve:
- an `am` operation
- a revert
- a rebase that stopped for conflict resolution
Reuse sequencer_ongoing_operation(), introduced for the analogous
`--amend` check, to detect these and refuse the partial commit. A rebase
that stopped because a pick became empty is not conflict resolution and,
as an earlier patch established, is deliberately left permitted.
Signed-off-by: Elijah Newren <newren@xxxxxxxxx>
---
builtin/commit.c | 24 ++++++++++++++++++-----
sequencer.h | 5 +++--
t/t3404-rebase-interactive.sh | 34 +++++++++++++++++++++++++++++++++
t/t3507-cherry-pick-conflict.sh | 11 +++++++++++
t/t4151-am-abort.sh | 11 +++++++++++
5 files changed, 78 insertions(+), 7 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 83ea8619d6..e96c663bd5 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -515,11 +515,25 @@ static const char *prepare_index(const char **argv, const char *prefix,
*/
commit_style = COMMIT_PARTIAL;
- if (whence != FROM_COMMIT) {
- if (whence == FROM_MERGE)
- die(_("cannot do a partial commit during a merge."));
- else if (is_from_cherry_pick(whence))
- die(_("cannot do a partial commit during a cherry-pick."));
+ switch (sequencer_ongoing_operation(the_repository, whence)) {
+ case ONGOING_NONE:
+ break;
+ case ONGOING_MERGE:
+ die(_("cannot do a partial commit during a merge."));
+ case ONGOING_CHERRY_PICK:
+ die(_("cannot do a partial commit during a cherry-pick."));
+ case ONGOING_REBASE_EMPTY:
+ /*
+ * A pick that became empty is not a conflict, and creating
+ * a new commit (partial or not) poses no problem.
+ */
+ break;
+ case ONGOING_REVERT:
+ die(_("cannot do a partial commit during a revert."));
+ case ONGOING_AM:
+ die(_("cannot do a partial commit during an am session."));
+ case ONGOING_REBASE_CONFLICT:
+ die(_("cannot do a partial commit while resolving conflicts during a rebase."));
}
if (list_paths(&partial, !current_head ? NULL : "HEAD", &pathspec))
diff --git a/sequencer.h b/sequencer.h
index 3a4bd97db1..634d1ddcb3 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -279,8 +279,9 @@ int sequencer_determine_whence(struct repository *r, enum commit_whence *whence)
/*
* An in-progress operation that records its result (often a conflict
- * resolution) as a new commit on top of HEAD, during which amending
- * HEAD via "git commit --amend" is almost always a mistake.
+ * resolution) as a new commit on top of HEAD. Some ways of invoking
+ * "git commit" -- amending HEAD, or a partial commit -- are almost
+ * always a mistake during such an operation.
*/
enum ongoing_operation {
ONGOING_NONE = 0,
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 4e6c3e2f19..8c63682b7f 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1970,6 +1970,40 @@ test_expect_success 'commit --amend is refused at an apply-backend conflict stop
)
'
+test_expect_success 'partial commit is refused at a rebase conflict stop' '
+ test_when_finished "git rebase --abort" &&
+ git checkout --detach conflict-branch &&
+ (
+ set_fake_editor &&
+ FAKE_LINES="1 3" &&
+ export FAKE_LINES &&
+ test_must_fail git rebase -i A
+ ) &&
+ echo resolved >conflict &&
+ git add conflict &&
+ test_must_fail git commit conflict 2>err &&
+ test_grep "cannot do a partial commit while resolving conflicts during a rebase." err
+'
+
+test_expect_success 'partial commit is refused at an apply-backend conflict stop' '
+ test_when_finished "rm -rf apply-backend" &&
+ test_create_repo apply-backend &&
+ (
+ cd apply-backend &&
+ test_commit base file &&
+ git branch -M mainline &&
+ test_commit upstream file upstream &&
+ git checkout -b side mainline~1 &&
+ test_commit conflicting file side &&
+ test_commit unrelated other &&
+ test_must_fail git rebase --apply mainline &&
+ echo resolved >file &&
+ git add file &&
+ test_must_fail git commit file 2>err &&
+ test_grep "cannot do a partial commit while resolving conflicts during a rebase." err
+ )
+'
+
test_expect_success 'todo has correct onto hash' '
GIT_SEQUENCE_EDITOR=cat git rebase -i no-conflict-branch~4 no-conflict-branch >actual &&
onto=$(git rev-parse --short HEAD~4) &&
diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
index 42de398f76..c3d024c97f 100755
--- a/t/t3507-cherry-pick-conflict.sh
+++ b/t/t3507-cherry-pick-conflict.sh
@@ -375,6 +375,17 @@ test_expect_success 'commit --amend of revert fails' '
test_grep "in the middle of a revert -- cannot amend." err
'
+test_expect_success 'partial commit during a revert fails' '
+ pristine_detach initial &&
+
+ test_must_fail git revert picked &&
+ echo resolved >foo &&
+ git add foo &&
+ test_must_fail git commit foo 2>err &&
+
+ test_grep "cannot do a partial commit during a revert." err
+'
+
test_expect_success 'successful revert does not set REVERT_HEAD' '
pristine_detach base &&
git revert base &&
diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh
index 9313a074b2..c80269e015 100755
--- a/t/t4151-am-abort.sh
+++ b/t/t4151-am-abort.sh
@@ -74,6 +74,17 @@ test_expect_success 'commit --amend during a failed am fails' '
git am --abort
'
+test_expect_success 'partial commit during a failed am fails' '
+ git reset --hard initial &&
+ cp file-2-expect file-2 &&
+ test_must_fail git am 000[1245]-*.patch &&
+ echo resolved >file-1 &&
+ git add file-1 &&
+ test_must_fail git commit file-1 2>err &&
+ test_grep "cannot do a partial commit during an am session." err &&
+ git am --abort
+'
+
test_expect_success 'am -3 --skip removes otherfile-4' '
git reset --hard initial &&
test_must_fail git am -3 0003-*.patch &&
--
gitgitgadget