Philippe Blain wrote:
> 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.
> > + 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.
> > 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.
> > 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`.
> 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
