Re: [PATCH v4 2/5] commit: allow a partial commit when a rebase pick becomes empty
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Sep 2, 2026, 15:39 UTC
- Message-ID
- <9950415d-ca3b-453b-9b0f-28c09a3d7f23@gmail.com>
- In-Reply-To
- <a0b9900437e7c2833960e5046b5acb6703f014b9.1788301481.git.gitgitgadget@gmail.com>
Hi Elijah
On 01/09/2026 23:24, Elijah Newren via GitGitGadget wrote:
Show 12 quoted lines
> From: Elijah Newren <newren@gmail.com> > > For years, we disallowed partial commits during merges or cherry-picks. > In commit 430b75f7209c (commit: give correct advice for empty commit > during a rebase, 2019-12-06) it was noted that the "cannot do a partial > commit during a cherry-pick" message was also printed when rebasing a > commit that became empty, and rather than drop the check in that case, > that commit opted to make the message print the actual operation that > was in progress. > > Since a commit that has become empty comes without conflicts, a new > partial commit poses no problems; remove the error in that case.
I'm not quite sure what I think about this. When we stop for a commit that becomes empty, we write CHERRY_PICK_HEAD and .git/MERGE_MSG so the user can preserve the commit by running "git commit --allow-empty". That makes me think we should complain about a partial commit. It also seems inconsistent with "git cherry-pick" where we still disallow a partial commit when we stop for a commit that becomes empty.
On the other hand, if the user has asked to edit the commit then allowing a partial commit would probably make sense as we know they wanted to modify it in some way. As I can't make up my mind I think its fair to say I don't have a strong opinion either way.
Thanks
Phillip
Show 43 quoted lines
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
> builtin/commit.c | 2 --
> t/t3404-rebase-interactive.sh | 5 ++---
> 2 files changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 17cc27e53e..01b79185e7 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -520,8 +520,6 @@ static const char *prepare_index(const char **argv, const char *prefix,
> 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."));
> - else if (is_from_rebase_now_empty(whence))
> - die(_("cannot do a partial commit during a rebase."));
> }
>
> if (list_paths(&partial, !current_head ? NULL : "HEAD", &pathspec))
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index ff11abb2f2..3588e16543 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -1858,7 +1858,7 @@ test_expect_success 'post-commit hook is called' '
> test_cmp expect actual
> '
>
> -test_expect_success 'correct error message for partial commit after empty pick' '
> +test_expect_success 'partial commit is allowed when a rebase pick becomes empty' '
> test_when_finished "git rebase --abort" &&
> (
> set_fake_editor &&
> @@ -1867,8 +1867,7 @@ test_expect_success 'correct error message for partial commit after empty pick'
> test_must_fail git rebase -i A D
> ) &&
> echo x >file1 &&
> - test_must_fail git commit file1 2>err &&
> - test_grep "cannot do a partial commit during a rebase." err
> + git commit file1
> '
>
> test_expect_success 'correct error message for commit --amend after empty pick' '