Bug in rebase --autosquash

Bug in rebase --autosquash

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1687 bytes --]

Hello,

after:

	uwe@taurus:~/tmp/git$ git version
	git version 2.37.2
	uwe@taurus:~/tmp/git$ git init
	Initialized empty Git repository in /home/uwe/tmp/git/.git/
	uwe@taurus:~/tmp/git$ echo contnt > file
	uwe@taurus:~/tmp/git$ git add file
	uwe@taurus:~/tmp/git$ git commit -m 'file with content'
	...
	uwe@taurus:~/tmp/git$ echo 'content' > file
	uwe@taurus:~/tmp/git$ git commit --fixup=@ file
	[main ef8f0bd27a56] fixup! file with content
	uwe@taurus:~/tmp/git$ echo 'more content' >> file
	uwe@taurus:~/tmp/git$ git commit --fixup=@ file
	[main b40a214bf5fb] fixup! fixup! file with content
	 1 file changed, 1 insertion(+)

running

	git rebase -i --autosquash @~2

my editor presents me with:

	pick ef8f0bd27a56 fixup! file with content
	pick b40a214bf5fb fixup! fixup! file with content

However I would have expected

	pick ef8f0bd27a56 fixup! file with content
	fixup b40a214bf5fb fixup! fixup! file with content

instead.

Is this a feature I don't understand?

When the original commit is in the range (i.e. with

	git rebase -i --autosquash --root

) it does the right thing, i.e. my editor then has:

	pick 0c1d21698c38 file with content
	fixup ef8f0bd27a56 fixup! file with content
	fixup b40a214bf5fb fixup! fixup! file with content

. Unrelated to that, but annoying is, that

	git rebase --autosquash --root

doesn't do the right thing. It doesn't change the history at all, while
I expected it to only contain a single commit then.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: Bug in rebase --autosquash

[Felipe Contreras]

On Wed, Aug 17, 2022 at 4:54 AM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> after:

...

> running
>
>         git rebase -i --autosquash @~2
>
> my editor presents me with:
>
>         pick ef8f0bd27a56 fixup! file with content
>         pick b40a214bf5fb fixup! fixup! file with content
>
> However I would have expected
>
>         pick ef8f0bd27a56 fixup! file with content
>         fixup b40a214bf5fb fixup! fixup! file with content
>
> instead.
>
> Is this a feature I don't understand?

The problem is the code removes all "fixup! " prefixes and then tries
to find a commit with that title, which isn't in the list. If you
specify the commit hash, then it finds it and it works as you
intended.

I don't see why the code would do that though, since you are clearly
intending to fix "fixup! file with content", not "file with content".

This patch below (apologies for gmail auto wrapping in advance) should
make it work as you intend.

--- a/sequencer.c
+++ b/sequencer.c
@@ -6213,12 +6213,8 @@ int todo_list_rearrange_squash(struct todo_list
*todo_list)
                if (skip_fixupish(subject, &p)) {
                        struct commit *commit2;

-                       for (;;) {
-                               while (isspace(*p))
-                                       p++;
-                               if (!skip_fixupish(p, &p))
-                                       break;
-                       }
+                       while (isspace(*p))
+                               p++;

                        entry = hashmap_get_entry_from_hash(&subject2item,
                                                strhash(p), p,

-- 
Felipe Contreras

Re: Bug in rebase --autosquash

[Johannes Schindelin]

[-- Attachment #1: Type: text/plain, Size: 1433 bytes --]

Hi Uwe,

On Wed, 17 Aug 2022, Uwe Kleine-König wrote:

> after:
>
> 	uwe@taurus:~/tmp/git$ git version
> 	git version 2.37.2
> 	uwe@taurus:~/tmp/git$ git init
> 	Initialized empty Git repository in /home/uwe/tmp/git/.git/
> 	uwe@taurus:~/tmp/git$ echo contnt > file
> 	uwe@taurus:~/tmp/git$ git add file
> 	uwe@taurus:~/tmp/git$ git commit -m 'file with content'
> 	...
> 	uwe@taurus:~/tmp/git$ echo 'content' > file
> 	uwe@taurus:~/tmp/git$ git commit --fixup=@ file
> 	[main ef8f0bd27a56] fixup! file with content
> 	uwe@taurus:~/tmp/git$ echo 'more content' >> file
> 	uwe@taurus:~/tmp/git$ git commit --fixup=@ file
> 	[main b40a214bf5fb] fixup! fixup! file with content
> 	 1 file changed, 1 insertion(+)
>
> running
>
> 	git rebase -i --autosquash @~2
>
> my editor presents me with:
>
> 	pick ef8f0bd27a56 fixup! file with content
> 	pick b40a214bf5fb fixup! fixup! file with content
>
> However I would have expected
>
> 	pick ef8f0bd27a56 fixup! file with content
> 	fixup b40a214bf5fb fixup! fixup! file with content
>
> instead.
>
> Is this a feature I don't understand?

This demonstrates a somewhat obscure feature where the `fixup!` of a
`fixup!` magically targets the original commit.

I understand that it is somewhat unintuitive that `fixup! fixup! abc` does
not target `fixup! abc` but instead `abc`, but that's how the tool has
been behaving forever.

Ciao,
Dscho

Re: Bug in rebase --autosquash

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> This demonstrates a somewhat obscure feature where the `fixup!` of a
> `fixup!` magically targets the original commit.
>
> I understand that it is somewhat unintuitive that `fixup! fixup! abc` does
> not target `fixup! abc` but instead `abc`, but that's how the tool has
> been behaving forever.

It certainly is handy when the original commit A has a fixup commit
B, which gets later fixed up by another commit C, resulting in a
sequence like this:

    $ git rebase -i A~1 C

    pick A original commit
    fixup B fixup! original commit
    fixup C fixup! fixup! original commit

But if the user for some reason finds it is not quite ready to touch
the original commit by squashing in the fixes, it may be reasonable
to want to squash the two fixes together, so that it can later be
applied to the original commit.  And it would be one natural way to
request that with

    $ git rebase -i A C

that is, leave the history up to A intact, but everything above
tweaked.  Without --autosquash, you would get

    pick B fixup! original commit
    pick C fixup! fixup! original commit

and you would manually do

    pick B fixup! original commit
    fixup C fixup! fixup! original commit

to squash B and C together into a single fix-up to be applied
later.  It does not look all too unreasonable to expect the
"--autosquash" feature to do that automatically for you.

Thanks.