Re: [PATCH 5/5] git-rebase.txt: add a note about 'ORIG_HEAD' being overwritten

From: Philippe Blain <levraiphilippeblain@gmail.com>
Date: Jan 9, 2023, 18:22 UTC
Message-ID: <85da5311-bef6-88d9-850d-888d874f6988@gmail.com>
In-Reply-To: <xmqq4jt122k2.fsf@gitster.g>

Hi Junio,

Le 2023-01-07 à 21:16, Junio C Hamano a écrit :

Show 16 quoted lines

> "Philippe Blain via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> From: Philippe Blain <levraiphilippeblain@gmail.com>
>>
>> 'ORIG_HEAD' is written at the start of the rebase, but is not guaranteed
>> to still point to the original branch tip at the end of the rebase.
>>
>> Indeed, using other commands that write 'ORIG_HEAD' during the rebase,
>> like splitting a commit using 'git reset HEAD^', will lead to 'ORIG_HEAD'
>> being overwritten.
> 
> Is that a news?  If a user does "reset", the user is asking that
> HEAD is changed and the old state kept in ORIG_HEAD at the same
> time, so while it is not wrong per-se to say that user can clobber
> ORIG_HEAD after rebase sets it first, it is pretty much expected,
> no?

I forgot to add a link to the mailing list thread and just added a Reported-by trailer here, but it was not expected by at least 2 users (including me) [1].

[1] https://lore.kernel.org/git/28ebf03b-e8bb-3769-556b-c9db17e43dbb@gmail.com/T/#m827179c5adcfb504d67f76d03c8e6942b55e5ed0

I think of 'git reset HEAD^' during a rebase, to split a existing commit, as rebase-related operation (it could almost be a different instruction in the TODO list), and so my expectation was that ORIG_HEAD at the end of the rebase would not be changed by what I did during the rebase.

But I can see how it would be confusing for people that would expect ORIG_HEAD to point to the pre-reset HEAD, and so I just thought I'd point that out in the doc to avoid potential future confusion.

Show 7 quoted lines

> 
> What would be unexpected is if "rebase" overwrote ORIG_HEAD after
> user did all these other things while it gave control back and then
> it took control back, and it would be worth documenting.
> 
> Having said that, I do not mind documenting this.  I am not sure "is
> not guaranteed" is a good way to phrase what happens, though.

I use that phrasing because the 'gitrevisions' entry on 'ORIG_HEAD' says:

  `ORIG_HEAD` is created by commands that move your `HEAD` in a drastic
  way ([...]),
  to record the position of the `HEAD` before their operation, so that
  you can easily change the tip of the branch back to the state before you ran
  them.

so it might be read to mean that 'ORIG_HEAD' always points to the pre-operation state. I'm open to other wording, though.

Show 18 quoted lines

> 
>> +[NOTE]
>> +`ORIG_HEAD` is not guaranteed to still point to the previous branch tip
>> +at the end of the rebase if other commands that write that pseudo-ref
>> +(e.g. `git reset`) are used during the rebase. The previous branch tip,
>> +however, is accessible using the reflog of the current branch
>> +(i.e. `@{1}`, see linkgit:gitrevisions[7]).
> 
>     `ORIG_HEAD` is set to point at the tip of the previous branch
>     when `rebase` begins, but the user can run commands (e.g. "git
>     reset") that overwrites `ORIG_HEAD` while `rebase` gives control
>     to the user (e.g. while asking to resolve conflict).
> 
> It is excellent to mention reflog, which is very much an upward
> compatible replacement of ORIG_HEAD.
> 
> Thanks.
>

Thanks, I'll consider that wording for v2.

Cheers,

Philippe.