Re: Regression in 'git pull --rebase --autostash' since v2.32.0
- From
Felipe Contreras <felipe.contreras@gmail.com>
- Date
- Jul 18, 2021, 03:05 UTC
- Message-ID
- <60f39a72c9c8e_2fb208b3@natae.notmuch>
- In-Reply-To
- <1329d238-e38a-7c8b-d468-500a0ae38bbb@gmail.com>
Philippe Blain wrote:
Show 65 quoted lines
> Le 2021-07-17 à 15:34, Felipe Contreras a écrit :
> > Hello,
> >
> > Philippe Blain wrote:
> >> Your recent clean-up of 'git pull --autostash' seems to unfortunately have made things
> >> worse if the pull brings new files that conflict with existing untracked files,
> >> which makes the pull abort,
> >> and there are tracked files with modifications (so --autostash comes into play).
> >>
> >> Before your change, 'git pull --no-rebase --autostash' did *not* apply the autostash
> >> after the pull failed, thus loosing modifications to tracked files (and it did not save the
> >> stash entry !). 'git pull --rebase --autostash' correctly applied the autostash, but ended with
> >> a strange "error: could not detach HEAD".
> >>
> >> After your change, both 'git pull --no-rebase --autostash' and 'git pull --rebase --autostash'
> >> have the same buggy behaviour: they do not apply the autostash and do not save it in the stash list.
> >>
> >> I had already documented the old behaviour at [1]. Here, I copy my reproducer script
> >> (save it as "script"):
> >
> > I cannot reproduce this. In my case the reproducer script never puts
> > anything in the stash list.
>
> I tried again with GIT_CONFIG_GLOBAL=/dev/null on my end (and manually
> setting GIT_AUTHOR_{NAME,EMAIL}) to be sure that my config was not playing
> a role, and I still reproduce. Copying the output from my 1st email
> (for --rebase, but it's the same thing for --no-rebase):
>
>
> > ** ./script --rebase --autostash **
> >
> > ~~~
> > + git status
> > On branch master
> > Your branch is up to date with 'origin/master'.
> >
> > Changes not staged for commit:
> > (use "git add <file>..." to update what will be committed)
> > (use "git restore <file>..." to discard changes in working directory)
> > modified: file
> >
> > Untracked files:
> > (use "git add <file>..." to include in what will be committed)
> > other
> >
> > no changes added to commit (use "git add" and/or "git commit -a")
> > + git pull --rebase --autostash
> > remote: Enumerating objects: 4, done.
> > remote: Counting objects: 100% (4/4), done.
> > remote: Compressing objects: 100% (2/2), done.
> > remote: Total 3 (delta 0), reused 0 (delta 0), pack-reused 0
> > Unpacking objects: 100% (3/3), 283 bytes | 94.00 KiB/s, done.
> > From /Users/Philippe/Code/GIT-devel/BUGS/ggg-759-pull-autotash-untracked/test
> > 4ebab2f..fc7a169 master -> origin/master
> > Updating 4ebab2f..fc7a169
> > Created autostash: cfd51b5
> > error: The following untracked working tree files would be overwritten by merge:
> > other
> > Please move or remove them before you merge.
> > Aborting
>
> Here is the bug: we should see "Applied autostash" after "Aborting". The 'git status'
> below is to verify that the autostash was indeed not applied, and the
> 'git stash list' was to check if the autostash was at least available
> in the stash list (it's not).Ahh, I thought you expected `git stash list` to show something. If this is the bug then yeah, I can reproduce.
Show 51 quoted lines
> > + git status > > On branch master > > Your branch is behind 'origin/master' by 1 commit, and can be fast-forwarded. > > (use "git pull" to update your local branch) > > > > Untracked files: > > (use "git add <file>..." to include in what will be committed) > > other > > > > nothing added to commit but untracked files present (use "git add" to track) > > + git stash list > > # empty! > > ~~~ > > Back to your reply: > > > > > Moreover, this is not an issue of `git pull`, but `git merge`. > > > > I can reproduce the problem that the modifications are lost like this: > > > > git init test > > ( > > cd test > > date >> file > > git add file > > git commit -m 'add file' > > date >> other > > git add other > > git commit -m 'add other' > > git checkout -b topic @~ > > date >> other > > date >> file > > git status > > git "$@" master > > git status > > git stash list > > ) > > > > Running this with 'rebase --autostash' fails and nothing is put in the > > stash list, but the modifications to 'file' remain. I think this is the > > correct behavior. > > Yes. We see the following output: > > Created autostash: 69775ee > Applied autostash. > Successfully rebased and updated refs/heads/topic. > > The code correctly applied the autostash after the rebase, so it's > normal it's not kept in the stash list. I agree it's the correct behaviour.
Good.
Show 5 quoted lines
> > But with 'merge --autostash' the modifications to 'file' are lost. That > > is a bug, and it's already present in v2.32. > > That's also true. From what I understand it dates back to the introduction > of 'git merge --autostash'.
Yes, that's probably true.
Show 7 quoted lines
> > Do you agree? > > I agree that the bug with 'merge --autostash' was already present in v2.32.0. > But since your 340062243a (pull: cleanup autostash check, 2021-06-17), > 'git pull --rebase --autostash' calls 'git merge --autostash' in the fast-forward > case, and so we hit the bug, which was not the case before since the fast-forward > merge shortcut was skipped if '--autostash' was given for 'git pull --rebase'.
Yes we hit the bug with `git pull`, but the bug was there already when people did `git merge`.
Show 5 quoted lines
> The fix I suggested in [1] seems to fix both cases, but as I wrote there > it still leaves two code paths that might trigger the bug. I'm not familiar > at all with the code for these two code paths, so I'm not ready to send > a formal patch; I thought I'd send the diff anyway if you or Denton (or anyone > else) wants to start from there.
Unfortunately I'm not familiar with the `git merge` code either, I've only been modifying the `git pull` code, so I would also be starting from scratch if I tried to fix that myself.
But I think that's where it should be fixed.
Cheers.
-- Felipe Contreras