Re: [PATCH 1/3] rebase -r: do create merge commit after empty resolution

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Mar 28, 2025, 17:14 UTC
Message-ID: <CAPig+cS92W_gYuNsaTvQxiP3xBK7Wpg0__uVkgAU1x0OFJUZgQ@mail.gmail.com>
In-Reply-To: <6c8f77cb71c7e0c820704b1725331f4601d8876e.1743181401.git.gitgitgadget@gmail.com>

On Fri, Mar 28, 2025 at 1:03 PM Philippe Blain via GitGitGadget <gitgitgadget@gmail.com> wrote:

Show 38 quoted lines

> When a user runs 'git rebase --continue' to conclude a conflicted merge
> during a 'git rebase -r' invocation, we do not create a merge commit if
> the resolution was empty (i.e. if the index and HEAD are identical). We
> simply continue the rebase as if no 'merge' instruction had been given.
> This is confusing since all commits from the side branch are absent from
> the rebased history. What's more, if that 'merge' is the last
> instruction in the todo list, we fail to remove the merge state, such
> that running 'git status' shows we are still merging after the rebase
> has concluded.
> [...]
> Make sure to also remove MERGE_HEAD when a merge command fails to start.
> We already remove MERGE_MSG since e032abd5a0 (rebase: fix rewritten list
> for failed pick, 2023-09-06). Removing MERGE_HEAD ensures that in this
> situation, upon 'git rebase --continue' we still exit early in
> 'commit_staged_changes', without calling 'run_git_commit'. This is
> already covered by t5407.11, which fails without this change because we
> enter 'run_git_commit' and then fail to find 'rebase_path_message'.
>
> Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
> ---
> diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
> +test_expect_success '--continue creates merge commit after empty resolution' '
> +       git reset --hard main &&
> +       git checkout -b rebase_i_merge &&
> +       test_commit unrelated &&
> +       git checkout -b rebase_i_merge_side &&
> +       test_commit side2 main.txt &&
> +       git checkout rebase_i_merge &&
> +       test_commit side1 main.txt &&
> +       PICK=$(git rev-parse --short rebase_i_merge) &&
> +       test_must_fail git merge rebase_i_merge_side &&
> +       echo side1 >main.txt &&
> +       git add main.txt &&
> +       test_tick &&
> +       git commit --no-edit &&
> +       FAKE_LINES="1 2 3 5 6 7 8 9 10 11" &&
> +       export FAKE_LINES &&
> +       test_must_fail git rebase -ir main &&

I don't think you want to be setting FAKE_LINES like this since doing so will pollute the environment for all tests following this one. You can find existing precedent in this script which demonstrates the correct way to handle this case. Specifically, you'd want:

    test_must_fail env FAKE_LINES="1 2 3 5 6 7 8 9 10 11" \
        git rebase -ir main &&

Show 5 quoted lines

> +       echo side1 >main.txt &&
> +       git add main.txt &&
> +       git rebase --continue &&
> +       git log --merges >out &&
> +       test_grep "Merge branch '\''rebase_i_merge_side'\''" out

You could take advantage of the SQ variable defined by t/test-lib.sh to make this a bit easier to digest:

    test_grep "Merge branch ${SQ}rebase_i_merge_side${SQ}" out

Or, even simpler, you'll find that some test scripts just use regex wildcard "." to make the needle even more readable:

    test_grep "Merge branch .rebase_i_merge_side." out