git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* git rebase/git rebase --abort cause inconsistent state
@ 2020-11-06 18:32 Eugen Konkov
  2020-11-06 18:34 ` Eugen Konkov
  2020-11-06 20:27 ` Elijah Newren
  0 siblings, 2 replies; 9+ messages in thread
From: Eugen Konkov @ 2020-11-06 18:32 UTC (permalink / raw)
  To: Git Mailing List

Hi

I try to rebase, get conflicts. So I decide to --abort

After --abort I expect state before rebasing, but I get conflicts.

I  supposet  this  is  because `git rebase` switches to not branch and
--abort can not return to branch I was on before rebasing

Is this a bug?




kes@work ~/t/lib/MaitreD $ git rebase dev local/dev
Created autostash: 566876c8
warning: Cannot merge binary files: share/ChangeAgreement.docx (HEAD vs. f2442d9a... Update Docs.pm)
Auto-merging share/ChangeAgreement.docx
CONFLICT (content): Merge conflict in share/ChangeAgreement.docx
error: could not apply f2442d9a... Update Docs.pm
Resolve all conflicts manually, mark them as resolved with
"git add/rm <conflicted_files>", then run "git rebase --continue".
You can instead skip this commit: run "git rebase --skip".
To abort and get back to the state before "git rebase", run "git rebase --abort".
Could not apply f2442d9a... Update Docs.pm
kes@work ~/t/lib/MaitreD $ git rebase --abort 
Applying autostash resulted in conflicts.
Your changes are safe in the stash.
You can run "git stash pop" or "git stash drop" at any time.

Here is a tree before rebasing:
> a9597aaa (HEAD -> dev) Use DateTime with correct timezone
> 822ff801 Add link to Podio into mail
> 65575afe Update Docs.pm
| < e0003861 (local/dev) Update podio.t - test person contacts
| < 28ab8630 Create docdate if agreement is new and update test for that
| < 208ead68 Specified checking of person
| < f2442d9a Update Docs.pm
|/  
o 6d9c2159 (xtucha/test, xtucha/dev) Leave only one example in month

Here is conflicts:
HEAD detached from 142c1b15
Changes to be committed:
  (use "git restore --staged <file>..." to unstage)
1       modified:   ../../Makefile
2       modified:   ../../etc/maitre_d.development.conf
3       modified:   Command/bank_statement.pm
4       modified:   Command/invoicing.pm
5       modified:   Command/reminding.pm
6       modified:   Controller/Cart.pm
7       modified:   Controller/Saldo.pm

Unmerged paths:
  (use "git restore --staged <file>..." to unstage)
  (use "git add <file>..." to mark resolution)
8       both modified:   Controller/Podio.pm

$ git --version
git version 2.28.0


-- 
Best regards,
Eugen Konkov


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: git rebase/git rebase --abort cause inconsistent state
  2020-11-06 18:32 git rebase/git rebase --abort cause inconsistent state Eugen Konkov
@ 2020-11-06 18:34 ` Eugen Konkov
  2020-11-06 20:27 ` Elijah Newren
  1 sibling, 0 replies; 9+ messages in thread
From: Eugen Konkov @ 2020-11-06 18:34 UTC (permalink / raw)
  To: Git Mailing List

Hello Eugen,

Friday, November 6, 2020, 8:32:13 PM, you wrote:

> Hi

> I try to rebase, get conflicts. So I decide to --abort

> After --abort I expect state before rebasing, but I get conflicts.

> I  supposet  this  is  because `git rebase` switches to not branch and
> --abort can not return to branch I was on before rebasing

> Is this a bug?




> kes@work ~/t/lib/MaitreD $ git rebase dev local/dev
> Created autostash: 566876c8
> warning: Cannot merge binary files: share/ChangeAgreement.docx
> (HEAD vs. f2442d9a... Update Docs.pm)
> Auto-merging share/ChangeAgreement.docx
> CONFLICT (content): Merge conflict in share/ChangeAgreement.docx
> error: could not apply f2442d9a... Update Docs.pm
> Resolve all conflicts manually, mark them as resolved with
> "git add/rm <conflicted_files>", then run "git rebase --continue".
> You can instead skip this commit: run "git rebase --skip".
> To abort and get back to the state before "git rebase", run "git rebase --abort".
> Could not apply f2442d9a... Update Docs.pm
> kes@work ~/t/lib/MaitreD $ git rebase --abort 
> Applying autostash resulted in conflicts.
> Your changes are safe in the stash.
> You can run "git stash pop" or "git stash drop" at any time.

> Here is a tree before rebasing:
>> a9597aaa (HEAD -> dev) Use DateTime with correct timezone
>> 822ff801 Add link to Podio into mail
>> 65575afe Update Docs.pm
> | < e0003861 (local/dev) Update podio.t - test person contacts
> | < 28ab8630 Create docdate if agreement is new and update test for that
> | < 208ead68 Specified checking of person
> | < f2442d9a Update Docs.pm
> |/  
> o 6d9c2159 (xtucha/test, xtucha/dev) Leave only one example in month

> Here is conflicts:
> HEAD detached from 142c1b15
> Changes to be committed:
>   (use "git restore --staged <file>..." to unstage)
> 1       modified:   ../../Makefile
> 2       modified:   ../../etc/maitre_d.development.conf
> 3       modified:   Command/bank_statement.pm
> 4       modified:   Command/invoicing.pm
> 5       modified:   Command/reminding.pm
> 6       modified:   Controller/Cart.pm
> 7       modified:   Controller/Saldo.pm

> Unmerged paths:
>   (use "git restore --staged <file>..." to unstage)
>   (use "git add <file>..." to mark resolution)
> 8       both modified:   Controller/Podio.pm

> $ git --version
> git version 2.28.0


history after --abort:
* e0003861 (HEAD, local/dev) Update podio.t - test person contacts
* 28ab8630 Create docdate if agreement is new and update test for that
* 208ead68 Specified checking of person
* f2442d9a Update Docs.pm
* 6d9c2159 (xtucha/test, xtucha/dev) Leave only one example in month


history before rebase:
a9597aaa (HEAD -> dev) Use DateTime with correct timezone



-- 
Best regards,
Eugen Konkov


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: git rebase/git rebase --abort cause inconsistent state
  2020-11-06 18:32 git rebase/git rebase --abort cause inconsistent state Eugen Konkov
  2020-11-06 18:34 ` Eugen Konkov
@ 2020-11-06 20:27 ` Elijah Newren
  2020-11-06 23:13   ` Johannes Sixt
  1 sibling, 1 reply; 9+ messages in thread
From: Elijah Newren @ 2020-11-06 20:27 UTC (permalink / raw)
  To: Eugen Konkov; +Cc: Git Mailing List, Thomas Gummerer, Johannes Schindelin

On Fri, Nov 6, 2020 at 10:41 AM Eugen Konkov <kes-kes@yandex.ru> wrote:
>
> Hi
>
> I try to rebase, get conflicts. So I decide to --abort
>
> After --abort I expect state before rebasing, but I get conflicts.
>
> I  suppose this  is  because `git rebase` switches to not branch and
> --abort can not return to branch I was on before rebasing
>
> Is this a bug?
>
>
>
>
> kes@work ~/t/lib/MaitreD $ git rebase dev local/dev
> Created autostash: 566876c8
> warning: Cannot merge binary files: share/ChangeAgreement.docx (HEAD vs. f2442d9a... Update Docs.pm)
> Auto-merging share/ChangeAgreement.docx
> CONFLICT (content): Merge conflict in share/ChangeAgreement.docx
> error: could not apply f2442d9a... Update Docs.pm
> Resolve all conflicts manually, mark them as resolved with
> "git add/rm <conflicted_files>", then run "git rebase --continue".
> You can instead skip this commit: run "git rebase --skip".
> To abort and get back to the state before "git rebase", run "git rebase --abort".
> Could not apply f2442d9a... Update Docs.pm
> kes@work ~/t/lib/MaitreD $ git rebase --abort
> Applying autostash resulted in conflicts.
^^^^^^

Looks like you have rebase.autostash set to true and have some
uncommitted changes before your rebase started; it looks like it was
the reapplying of that stash at the time you abort is the thing that
failed.

According to the rebase docs for the --abort flag:
"If <branch> was provided when the rebase operation was started, then
HEAD will be reset to <branch>"
which suggests that the abort should switch you back to the original
branch, where the application of your local changes should be safe.
I'll cc the two most prolific committers to builtin/stash.c to get
their comments.

Some questions they may be interested in, though:  Is this bug
repeatable?  Can you find steps to reproduce and/or share your
repository?  Can you verify that you don't get this bug when
rebase.autostash is off?  What do your local changes before the rebase
look like and what are the nature of the conflicts afterwards (how
does a "git diff" before the rebase compare to a "git diff" after)?


> Your changes are safe in the stash.
> You can run "git stash pop" or "git stash drop" at any time.
>
> Here is a tree before rebasing:
> > a9597aaa (HEAD -> dev) Use DateTime with correct timezone
> > 822ff801 Add link to Podio into mail
> > 65575afe Update Docs.pm
> | < e0003861 (local/dev) Update podio.t - test person contacts
> | < 28ab8630 Create docdate if agreement is new and update test for that
> | < 208ead68 Specified checking of person
> | < f2442d9a Update Docs.pm
> |/
> o 6d9c2159 (xtucha/test, xtucha/dev) Leave only one example in month
>
> Here is conflicts:
> HEAD detached from 142c1b15
> Changes to be committed:
>   (use "git restore --staged <file>..." to unstage)
> 1       modified:   ../../Makefile
> 2       modified:   ../../etc/maitre_d.development.conf
> 3       modified:   Command/bank_statement.pm
> 4       modified:   Command/invoicing.pm
> 5       modified:   Command/reminding.pm
> 6       modified:   Controller/Cart.pm
> 7       modified:   Controller/Saldo.pm
>
> Unmerged paths:
>   (use "git restore --staged <file>..." to unstage)
>   (use "git add <file>..." to mark resolution)
> 8       both modified:   Controller/Podio.pm
>
> $ git --version
> git version 2.28.0
>
>
> --
> Best regards,
> Eugen Konkov
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: git rebase/git rebase --abort cause inconsistent state
  2020-11-06 20:27 ` Elijah Newren
@ 2020-11-06 23:13   ` Johannes Sixt
  2020-11-09 11:46     ` Eugen Konkov
  0 siblings, 1 reply; 9+ messages in thread
From: Johannes Sixt @ 2020-11-06 23:13 UTC (permalink / raw)
  To: Elijah Newren, Eugen Konkov
  Cc: Git Mailing List, Thomas Gummerer, Johannes Schindelin

Am 06.11.20 um 21:27 schrieb Elijah Newren:
> On Fri, Nov 6, 2020 at 10:41 AM Eugen Konkov <kes-kes@yandex.ru> wrote:
>> I try to rebase, get conflicts. So I decide to --abort
>>
>> After --abort I expect state before rebasing, but I get conflicts.
>>
>> I  suppose this  is  because `git rebase` switches to not branch and
>> --abort can not return to branch I was on before rebasing
>>
>> Is this a bug?
>>
>>
>>
>>
>> kes@work ~/t/lib/MaitreD $ git rebase dev local/dev
>> Created autostash: 566876c8
>> warning: Cannot merge binary files: share/ChangeAgreement.docx (HEAD vs. f2442d9a... Update Docs.pm)
>> Auto-merging share/ChangeAgreement.docx
>> CONFLICT (content): Merge conflict in share/ChangeAgreement.docx
>> error: could not apply f2442d9a... Update Docs.pm
>> Resolve all conflicts manually, mark them as resolved with
>> "git add/rm <conflicted_files>", then run "git rebase --continue".
>> You can instead skip this commit: run "git rebase --skip".
>> To abort and get back to the state before "git rebase", run "git rebase --abort".
>> Could not apply f2442d9a... Update Docs.pm
>> kes@work ~/t/lib/MaitreD $ git rebase --abort
>> Applying autostash resulted in conflicts.
> ^^^^^^
> 
> Looks like you have rebase.autostash set to true and have some
> uncommitted changes before your rebase started; it looks like it was
> the reapplying of that stash at the time you abort is the thing that
> failed.
> 
> According to the rebase docs for the --abort flag:
> "If <branch> was provided when the rebase operation was started, then
> HEAD will be reset to <branch>"
> which suggests that the abort should switch you back to the original
> branch, where the application of your local changes should be safe.

Unfortunately, that is not always the case, for example, in this one.

>> Your changes are safe in the stash.
>> You can run "git stash pop" or "git stash drop" at any time.
>>
>> Here is a tree before rebasing:
>>> a9597aaa (HEAD -> dev) Use DateTime with correct timezone >>> 822ff801 Add link to Podio into mail
>>> 65575afe Update Docs.pm
>> | < e0003861 (local/dev) Update podio.t - test person contacts
>> | < 28ab8630 Create docdate if agreement is new and update test for that
>> | < 208ead68 Specified checking of person
>> | < f2442d9a Update Docs.pm
>> |/
>> o 6d9c2159 (xtucha/test, xtucha/dev) Leave only one example in month

You start at branch dev. Then you use the two argument form

     git rebase dev local/dev

and when you later

     git rebase --abort

then you are not warped back to dev, but to local/dev:

> history after --abort:
> * e0003861 (HEAD, local/dev) Update podio.t - test person contacts
> * 28ab8630 Create docdate if agreement is new and update test for that
> * 208ead68 Specified checking of person
> * f2442d9a Update Docs.pm
> * 6d9c2159 (xtucha/test, xtucha/dev) Leave only one example in month

and at this point, your stashed changes, which were snapshot when you 
were on branch dev, are obvously in conflict with branch local/dev.

I'm not saying that that the behavior should be like this, I'm just 
explaining what was going on. I hate this behavior, BTW.

-- Hannes

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: git rebase/git rebase --abort cause inconsistent state
  2020-11-06 23:13   ` Johannes Sixt
@ 2020-11-09 11:46     ` Eugen Konkov
  2020-11-09 18:11       ` Junio C Hamano
  0 siblings, 1 reply; 9+ messages in thread
From: Eugen Konkov @ 2020-11-09 11:46 UTC (permalink / raw)
  To: Johannes Sixt, Elijah Newren
  Cc: Git Mailing List, Thomas Gummerer, Johannes Schindelin

Hello Johannes,

Saturday, November 7, 2020, 1:13:04 AM, you wrote:

> Am 06.11.20 um 21:27 schrieb Elijah Newren:
>> On Fri, Nov 6, 2020 at 10:41 AM Eugen Konkov <kes-kes@yandex.ru> wrote:
>>> I try to rebase, get conflicts. So I decide to --abort
>>>
>>> After --abort I expect state before rebasing, but I get conflicts.
>>>
>>> I  suppose this  is  because `git rebase` switches to not branch and
>>> --abort can not return to branch I was on before rebasing
>>>
>>> Is this a bug?
>>>
>>>
>>>
>>>
>>> kes@work ~/t/lib/MaitreD $ git rebase dev local/dev
>>> Created autostash: 566876c8
>>> warning: Cannot merge binary files: share/ChangeAgreement.docx (HEAD vs. f2442d9a... Update Docs.pm)
>>> Auto-merging share/ChangeAgreement.docx
>>> CONFLICT (content): Merge conflict in share/ChangeAgreement.docx
>>> error: could not apply f2442d9a... Update Docs.pm
>>> Resolve all conflicts manually, mark them as resolved with
>>> "git add/rm <conflicted_files>", then run "git rebase --continue".
>>> You can instead skip this commit: run "git rebase --skip".
>>> To abort and get back to the state before "git rebase", run "git rebase --abort".
>>> Could not apply f2442d9a... Update Docs.pm
>>> kes@work ~/t/lib/MaitreD $ git rebase --abort
>>> Applying autostash resulted in conflicts.
>> ^^^^^^
>> 
>> Looks like you have rebase.autostash set to true and have some
>> uncommitted changes before your rebase started; it looks like it was
>> the reapplying of that stash at the time you abort is the thing that
>> failed.
>> 
>> According to the rebase docs for the --abort flag:
>> "If <branch> was provided when the rebase operation was started, then
>> HEAD will be reset to <branch>"
>> which suggests that the abort should switch you back to the original
>> branch, where the application of your local changes should be safe.

> Unfortunately, that is not always the case, for example, in this one.

>>> Your changes are safe in the stash.
>>> You can run "git stash pop" or "git stash drop" at any time.
>>>
>>> Here is a tree before rebasing:
>>>> a9597aaa (HEAD -> dev) Use DateTime with correct timezone >>> 822ff801 Add link to Podio into mail
>>>> 65575afe Update Docs.pm
>>> | < e0003861 (local/dev) Update podio.t - test person contacts
>>> | < 28ab8630 Create docdate if agreement is new and update test for that
>>> | < 208ead68 Specified checking of person
>>> | < f2442d9a Update Docs.pm
>>> |/
>>> o 6d9c2159 (xtucha/test, xtucha/dev) Leave only one example in month

> You start at branch dev. Then you use the two argument form

>      git rebase dev local/dev

> and when you later

>      git rebase --abort

> then you are not warped back to dev, but to local/dev:

I suppose `git rebase --abort` should return me back to `dev`, because
this is the state I was before the command. hmm... suppose it will not
return to original branch when [branch] parameter is specified for git
rebase


>> history after --abort:
>> * e0003861 (HEAD, local/dev) Update podio.t - test person contacts
>> * 28ab8630 Create docdate if agreement is new and update test for that
>> * 208ead68 Specified checking of person
>> * f2442d9a Update Docs.pm
>> * 6d9c2159 (xtucha/test, xtucha/dev) Leave only one example in month

> and at this point, your stashed changes, which were snapshot when you 
> were on branch dev, are obvously in conflict with branch local/dev.

> I'm not saying that that the behavior should be like this, I'm just 
> explaining what was going on. I hate this behavior, BTW.
I also get inconsisten results https://stackoverflow.com/q/64592489/4632019
This depends on the remote history:
1)  when  there is changes to branch on remote server (sorry, it is named local
on pictures) and local changes to this branch
2)  when there is changes only to branch on remote server and no local
changes, so fast forward is possible


> Is this bug repeatable?
Yes

>Can you find steps to reproduce and/or share your repository?
do for commits.
push them to remote server
on  second  machine  fetch  this  branch  and  change the history. For
example the first made commit
push force back to server
fetch changes history from remote server on first machine
Then try to rebase remote history locally: git rebase dev local/dev
***local is name for local server, but this is remote history

This occur:
> and at this point, your stashed changes, which were snapshot when you
> were on branch dev, are obvously in conflict with branch local/dev.

Actually  here  I  this  of  `git  rebase  dev  local/dev`  as synonym
(probably incorrect) for `git pull --rebase`
Probably  here I am requred an option to drop those local commits that
were pushed to remote and which was changed on remote,
like this is done when `git pull --rebase`

I  prefer do `git fetch/git rebase` manually to keep thins in control.
`git pull` to my mind makes too many magic =(


>Can you verify that you don't get this bug when rebase.autostash is off?
This has no matter


>What do your local changes before the rebase
>look like and what are the nature of the conflicts afterwards (how
>does a "git diff" before the rebase compare to a "git diff" after)?
changes  was  at  binary  .docx  files.  Hope  I repeat upper those steps to
reproduce problem.


> -- Hannes



-- 
Best regards,
Eugen Konkov


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: git rebase/git rebase --abort cause inconsistent state
  2020-11-09 11:46     ` Eugen Konkov
@ 2020-11-09 18:11       ` Junio C Hamano
  2020-11-10 17:59         ` Eugen Konkov
  2020-11-10 22:28         ` Johannes Schindelin
  0 siblings, 2 replies; 9+ messages in thread
From: Junio C Hamano @ 2020-11-09 18:11 UTC (permalink / raw)
  To: Eugen Konkov
  Cc: Johannes Sixt, Elijah Newren, Git Mailing List, Thomas Gummerer,
	Johannes Schindelin

Eugen Konkov <kes-kes@yandex.ru> writes:

>> You start at branch dev. Then you use the two argument form
>
>>      git rebase dev local/dev
>
>> and when you later
>
>>      git rebase --abort
>
>> then you are not warped back to dev, but to local/dev:
>
> I suppose `git rebase --abort` should return me back to `dev`, because
> this is the state I was before the command. hmm... suppose it will not
> return to original branch when [branch] parameter is specified for git
> rebase

Yes, "git rebase [--onto C] A B" has always been a short-hand for

	git checkout B
	git rebase [--onto C] A

which means that if the second rebase step aborts, rebase wants to
go back to the state before the rebase started, i.e. immediately
after "checkout B" was done.

I think the root cause of the problem is that addition of the
"--autostash" feature (which came much later than the two-arg form)
was designed poorly.  If it wanted to keep the "two-arg form is a
mere short-hand for checkout followed by rebase" semantics to avoid
confusing existing users (which is probably a good thing and that
seems to be what the code does), then the auto-stash should have
been added _after_ we switch to the branch we rebase, i.e. B.  That
way, the stash would be applicable if the rebase gets aborted and
goes back to the original B, where the stash was taken from.

Of course, that would also mean that the original modification in
the working tree and the index may not allow you to move to branch B
(i.e. starting from your original branch O, and editing files in the
working tree, "git checkout B" may notice that you edited files that
are different between O and B and refuse to check out branch B to
prevent you from losing your local modifications), but that probably
is a good thing, if "two-arg form is a mere short-hand" paradigm is
to be kept.  So, "use autostash and you can always rebase in a clean
state" would no longer hold.

Another thing we could have done when adding "--autostash", was to
redefine the meaning of the two-arg form.  Then it starts to make
sense to take a stash _before_ switching to the branch to be rebased
(i.e.  B), to go back to the original branch before switching to B,
and then to unstash on the working tree of the original branch that
is checked out after aborting.

Note that such an alternative design would have had its own issues.
With such a different semantics of two-arg form, if a rebase cleanly
finishes, instead of staying on the rebased branch B, we MUST go
back to the original branch to unstash what was autostashed.
Usually people expect after a rebase to play with the rebased state
(e.g. test build), so staying on branch B that was just rebased
would be far more usable than going back to unrelated original
branch (and possibly unstashing).

In any case, the ship has long sailed, so ...

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: git rebase/git rebase --abort cause inconsistent state
  2020-11-09 18:11       ` Junio C Hamano
@ 2020-11-10 17:59         ` Eugen Konkov
  2020-11-10 22:28         ` Johannes Schindelin
  1 sibling, 0 replies; 9+ messages in thread
From: Eugen Konkov @ 2020-11-10 17:59 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Johannes Sixt, Elijah Newren, Git Mailing List, Thomas Gummerer,
	Johannes Schindelin

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

Hello Junio,

Monday, November 9, 2020, 8:11:46 PM, you wrote:

> Eugen Konkov <kes-kes@yandex.ru> writes:

>>> You start at branch dev. Then you use the two argument form
>>
>>>      git rebase dev local/dev
>>
>>> and when you later
>>
>>>      git rebase --abort
>>
>>> then you are not warped back to dev, but to local/dev:
>>
>> I suppose `git rebase --abort` should return me back to `dev`, because
>> this is the state I was before the command. hmm... suppose it will not
>> return to original branch when [branch] parameter is specified for git
>> rebase

> Yes, "git rebase [--onto C] A B" has always been a short-hand for

>         git checkout B
>         git rebase [--onto C] A

> which means that if the second rebase step aborts, rebase wants to
> go back to the state before the rebase started, i.e. immediately
> after "checkout B" was done.

> I think the root cause of the problem is that addition of the
> "--autostash" feature (which came much later than the two-arg form)
> was designed poorly.  If it wanted to keep the "two-arg form is a
> mere short-hand for checkout followed by rebase" semantics to avoid
> confusing existing users (which is probably a good thing and that
> seems to be what the code does), then the auto-stash should have
> been added _after_ we switch to the branch we rebase, i.e. B.  That
> way, the stash would be applicable if the rebase gets aborted and
> goes back to the original B, where the stash was taken from.

> Of course, that would also mean that the original modification in
> the working tree and the index may not allow you to move to branch B
> (i.e. starting from your original branch O, and editing files in the
> working tree, "git checkout B" may notice that you edited files that
> are different between O and B and refuse to check out branch B to
> prevent you from losing your local modifications), but that probably
> is a good thing, if "two-arg form is a mere short-hand" paradigm is
> to be kept.  So, "use autostash and you can always rebase in a clean
> state" would no longer hold.

> Another thing we could have done when adding "--autostash", was to
> redefine the meaning of the two-arg form.  Then it starts to make
> sense to take a stash _before_ switching to the branch to be rebased
> (i.e.  B), to go back to the original branch before switching to B,
> and then to unstash on the working tree of the original branch that
> is checked out after aborting.

> Note that such an alternative design would have had its own issues.
> With such a different semantics of two-arg form, if a rebase cleanly
> finishes, instead of staying on the rebased branch B, we MUST go
> back to the original branch to unstash what was autostashed.
> Usually people expect after a rebase to play with the rebased state
> (e.g. test build), so staying on branch B that was just rebased
> would be far more usable than going back to unrelated original
> branch (and possibly unstashing).

> In any case, the ship has long sailed, so ...

I should try that usecases to have an opinion on that.

Currently I just add picture when `dev` is moved while rebasing.
This does not occur when `local/dev` does not point to `dev`
(when `local/dev/` and `dev` point different commits)

Also I will try --onto and how it suits to my work flow.





-- 
Best regards,
Eugen Konkov

[-- Attachment #2: _3.jpg --]
[-- Type: image/jpeg, Size: 94656 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: git rebase/git rebase --abort cause inconsistent state
  2020-11-09 18:11       ` Junio C Hamano
  2020-11-10 17:59         ` Eugen Konkov
@ 2020-11-10 22:28         ` Johannes Schindelin
  2020-11-11  7:10           ` Johannes Sixt
  1 sibling, 1 reply; 9+ messages in thread
From: Johannes Schindelin @ 2020-11-10 22:28 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Eugen Konkov, Johannes Sixt, Elijah Newren, Git Mailing List,
	Thomas Gummerer

Hi Junio,

On Mon, 9 Nov 2020, Junio C Hamano wrote:

> Eugen Konkov <kes-kes@yandex.ru> writes:
>
> >> You start at branch dev. Then you use the two argument form
> >
> >>      git rebase dev local/dev
> >
> >> and when you later
> >
> >>      git rebase --abort
> >
> >> then you are not warped back to dev, but to local/dev:
> >
> > I suppose `git rebase --abort` should return me back to `dev`, because
> > this is the state I was before the command. hmm... suppose it will not
> > return to original branch when [branch] parameter is specified for git
> > rebase
>
> Yes, "git rebase [--onto C] A B" has always been a short-hand for
>
> 	git checkout B
> 	git rebase [--onto C] A
>
> which means that if the second rebase step aborts, rebase wants to
> go back to the state before the rebase started, i.e. immediately
> after "checkout B" was done.
>
> I think the root cause of the problem is that addition of the
> "--autostash" feature (which came much later than the two-arg form)
> was designed poorly.  If it wanted to keep the "two-arg form is a
> mere short-hand for checkout followed by rebase" semantics to avoid
> confusing existing users (which is probably a good thing and that
> seems to be what the code does), then the auto-stash should have
> been added _after_ we switch to the branch we rebase, i.e. B.  That
> way, the stash would be applicable if the rebase gets aborted and
> goes back to the original B, where the stash was taken from.

That makes a ton of sense to me.

> Of course, that would also mean that the original modification in
> the working tree and the index may not allow you to move to branch B
> (i.e. starting from your original branch O, and editing files in the
> working tree, "git checkout B" may notice that you edited files that
> are different between O and B and refuse to check out branch B to
> prevent you from losing your local modifications), but that probably
> is a good thing, if "two-arg form is a mere short-hand" paradigm is
> to be kept.  So, "use autostash and you can always rebase in a clean
> state" would no longer hold.

I agree with that, too.

> Another thing we could have done when adding "--autostash", was to
> redefine the meaning of the two-arg form.  Then it starts to make
> sense to take a stash _before_ switching to the branch to be rebased
> (i.e.  B), to go back to the original branch before switching to B,
> and then to unstash on the working tree of the original branch that
> is checked out after aborting.
>
> Note that such an alternative design would have had its own issues.
> With such a different semantics of two-arg form, if a rebase cleanly
> finishes, instead of staying on the rebased branch B, we MUST go
> back to the original branch to unstash what was autostashed.
> Usually people expect after a rebase to play with the rebased state
> (e.g. test build), so staying on branch B that was just rebased
> would be far more usable than going back to unrelated original
> branch (and possibly unstashing).
>
> In any case, the ship has long sailed, so ...

Right. I think by now, the sanest way out of this fix is to do as you say,
stash only _after_ switching to the branch (if that was asked for).

Unfortunately, it is not that trivial to change `git rebase` to autostash
_after_ switching branches: we actually do skip the actual _checkout_
part, for performance reasons, as of 767a9c417eb (rebase -i: stop checking
out the tip of the branch to rebase, 2020-01-24).

My wishful thinking part wants Elijah's merge-ort work to be complete
already so that we can implement a purely in-memory, throw-away
cherry-pick of the autostashed changes on top of the branch to switch to,
and add that as a mandatory check _right after_ autostashing in `git
rebase`. I guess at some stage that will happen.

In the meantime, we might need to disallow `--autostash` with implicit
branch switching.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: git rebase/git rebase --abort cause inconsistent state
  2020-11-10 22:28         ` Johannes Schindelin
@ 2020-11-11  7:10           ` Johannes Sixt
  0 siblings, 0 replies; 9+ messages in thread
From: Johannes Sixt @ 2020-11-11  7:10 UTC (permalink / raw)
  To: Johannes Schindelin, Junio C Hamano
  Cc: Eugen Konkov, Elijah Newren, Git Mailing List, Thomas Gummerer

Am 10.11.20 um 23:28 schrieb Johannes Schindelin:
> On Mon, 9 Nov 2020, Junio C Hamano wrote:
>> Eugen Konkov <kes-kes@yandex.ru> writes:
>>
>>>> You start at branch dev. Then you use the two argument form
>>>
>>>>       git rebase dev local/dev
>>>
>>>> and when you later
>>>
>>>>       git rebase --abort
>>>
>>>> then you are not warped back to dev, but to local/dev:
>>>
>>> I suppose `git rebase --abort` should return me back to `dev`, because
>>> this is the state I was before the command. hmm... suppose it will not
>>> return to original branch when [branch] parameter is specified for git
>>> rebase
>>
>> Yes, "git rebase [--onto C] A B" has always been a short-hand for
>>
>> 	git checkout B
>> 	git rebase [--onto C] A
>>
>> which means that if the second rebase step aborts, rebase wants to
>> go back to the state before the rebase started, i.e. immediately
>> after "checkout B" was done.
>>
>> I think the root cause of the problem is that addition of the
>> "--autostash" feature (which came much later than the two-arg form)
>> was designed poorly.  If it wanted to keep the "two-arg form is a
>> mere short-hand for checkout followed by rebase" semantics to avoid
>> confusing existing users (which is probably a good thing and that
>> seems to be what the code does), then the auto-stash should have
>> been added _after_ we switch to the branch we rebase, i.e. B.  That
>> way, the stash would be applicable if the rebase gets aborted and
>> goes back to the original B, where the stash was taken from.
> 
> That makes a ton of sense to me.

Not to me. In particular, I would prefer to move away from the mental 
model "two-arg form is shorthand for checkout followed by rebase".

First of all, it does not match the mental model of inexperienced users. 
You have to have been deep in Git operations long enough to know that 
the two-arg form is implemented by an initial checkout so that the 
rebase can proceed as if it were the usual one-arg form.

Second, this initial checkout in two-arg form is not necessary at all to 
begin the rebase. As a first step, the commits to be rebased must be 
determined. For this, the traditional way is to ask for the range 
BASE..HEAD (and in order not to change this query for two-arg form, the 
checkout was added). But the commits can be determined with 
BASE..${second_arg:-HEAD} without requiring a checkout. Then the first 
unavoidable checkout is the one that goes to ONTO (with some further 
shortcuts in an interactive rebase).

I really don't give a dime for the initial checkout. After a botched 
two-arg rebase, I usually prefer that --abort brings me back to the 
branch were I was when I started, and not to the branch that was the 
second arg of the rebase.

>> In any case, the ship has long sailed, so ...

Well, then order it back. rebase is porcelain, not plumbing.

-- Hannes

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-11-11  7:11 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-06 18:32 git rebase/git rebase --abort cause inconsistent state Eugen Konkov
2020-11-06 18:34 ` Eugen Konkov
2020-11-06 20:27 ` Elijah Newren
2020-11-06 23:13   ` Johannes Sixt
2020-11-09 11:46     ` Eugen Konkov
2020-11-09 18:11       ` Junio C Hamano
2020-11-10 17:59         ` Eugen Konkov
2020-11-10 22:28         ` Johannes Schindelin
2020-11-11  7:10           ` Johannes Sixt

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).