From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 25 Nov 2025 20:06:44 GMT
Subject: Re: [PATCH 1/1] replay: add --revert option to reverse commit changes
Message-ID: <xmqqfra1ri5n.fsf@gitster.g>
In-Reply-To: <xmqqjyzdrjf1.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

> By the way, I probably would not be queuing this version today, as
> this has obvious conflict with a large code movement made by
> Patrick's "history" series, which itself is expecting a reroll.
>
> Perhaps collect review comments on this iteration a bit more and
> wait for that other topic to be rerolled, and if it turns out to be
> solid enough, base a v2 of this patch on top of it?

While I cannot test it with other topics, I had a chance to run
tests after applying the patch directly on top of 'master':

    $ make CC=clang SANITIZE=address,leak test
    ...
    Test Summary Report
    -------------------
    t3650-replay-basics.sh                           (Wstat: 256 (exited 1) Tests: 31 Failed: 5)
      Failed tests:  23-25, 27, 31
      Non-zero exit status: 1

The first failure was this one

    expecting success of 3650.23 'using replay with --revert to revert a commit': 
            # Revert commits D and E from topic2
            git replay --revert --onto topic1 topic1..topic2 >result &&

            test_line_count = 1 result &&
            NEW_TOPIC2=$(cut -f 3 -d " " result) &&

            # Verify the result updates the topic2 branch
            printf "update refs/heads/topic2 " >expect &&
            printf "%s " $NEW_TOPIC2 >>expect &&
            git rev-parse topic2 >>expect &&

            test_cmp expect result &&

            # Verify the commit messages contain "Revert"
            # topic1..topic2 contains D and E, so we get 2 reverts on top of topic1 (which has F, C, B, A)
            git log --format=%s $NEW_TOPIC2 >actual &&
            test_line_count = 6 actual &&
            head -n 1 actual >first-line &&
            test_grep "^Revert" first-line

    test_line_count: line count for result != 1

The "result" file has 0 bytes (hence 0 lines).

Actually, address or leak sanitizing build is not needed to
reproduce this problem, it seems.

    $ make CC=clang test

Was sufficient to see the same first failure.