Hi Elijah
On 29/02/2020 15:51, Elijah Newren wrote:
Hi Phillip and Blaise,
On Sat, Feb 29, 2020 at 6:30 AM Phillip Wood <phillip.wood123@xxxxxxxxx> wrote:
Hi Blaise
On 28/02/2020 17:36, Blaise Garant wrote:
Hello,
I don't know if this is a bug but it was unexpected for us. I
accidentally added untracked files through a `git add .` while doing
an interactive rebase and aborting the rebase deleted those files. Is
this to be expected?
I agree that this is surprising and undesirable but it's not unexpected
given the way --abort is implemented. 'rebase --abort' calls 'reset
--hard <branch we're rebasing>' so it will discard all the uncommitted
changes in the worktree and reset the worktree and index to the branch tip.
And it's worth noting that if they had done something similar outside
of rebase/merge/cherry-pick/etc.:
git add $UNTRACKED
git reset --hard
then the $UNTRACKED file would be deleted, so this isn't new or
unusual but matches git behavior elsewhere.
It's not new but it can be confusing. rm warns if you remove a newly
added file but reset --hard will happily blow it away
$ touch untracked
$ git add untracked
$ git rm untracked
error: the following file has changes staged in the index:
untracked
(use --cached to keep the file, or -f to force removal)
$ git reset --hard
HEAD is now at <oid> <subject>
The tricky thing with your situation is that the files are tracked at
the point we call 'reset --hard' as they've been added to the index so
git feels free to discard them. Perhaps rather than calling 'reset
--hard' it would be better to use a custom callback with unpack_trees()
that errors out if there are any paths in the index that are not in
HEAD, the commit we just picked or the branch tip we're resetting to. If
Should such a special callback also be used for reset --hard?
>
Also, this special callback, as stated here, wouldn't work: paths can
exist in a merge that didn't exist in any of the three commits being
threeway merged. All of the following situations are cases where that
can happen (and there may be more that I'm just not thinking of off
the top of my head):
1) merge in a not-fully clean state. rebase may disallow this, at
least right now, but merge traditionally hasn't. I'm not sure
cherry-pick --no-commit disallows this.
I'm pretty sure cherry-pick --no-commit will operate on a dirty index,
I'd forgotten about that
2) directory/file or submodule/file or submodule/subdirectory or
regular-file/symlink conflicts. the merge machinery should be free to
rename paths to something that didn't exist on either side so that the
paths from both side can coexist.
3) directory renames. If one side renamed z/->y/, and the other side
added a new file z/new, the merge should be allowed to move that file
to y/new (depending on the setting of merge.directoryRenames...).
Note that y/new didn't exist on either side of history nor in the
merge base.
Thanks for pointing that out. I'd assumed we'd only check index entries
in stage 0 so if the user aborts while these entries are unmerged then
it wouldn't be a problem but if they've partially resolved the merge
then --abort could fail.
we do that we should consider using the same thing for
'cherry-pick/merge/reset --abort' as well. --autostash potentially
complicates things as the file might be in the stash but not in the
other commits but lets not worry about that at the moment.
reset --abort? Not sure what you're referring to here.
I meant revert
I'm not sure what a good way forward is, blindly wiping out newly added
files is not great from the users point of view but avoiding false
positives is tricky
Best Wishes
Phillip
