unexpected file deletion after using git rebase --abort

unexpected file deletion after using git rebase --abort

[Paul A. Kennedy]

Hello!

I lost a previously untracked file that I added to the index in the
middle of a git rebase --interactive session after a git rebase --abort.
This was unexpected.

    $ ls forgotten_file
    forgotten_file
    $ git rebase --interactive HEAD~3 
      [change first rebase command from pick to  edit]
    $ git add forgotten_file
    $ git rebase --abort
    $ ls forgotten_file
    ls: cannot access forgotten_file: No such file or directory
    $

I was (of course) able to find the SHA-1 of the dangling blob using 'git
fsck', and then retrieve the file using 'git cat-file -p SHA1'

Should this behaviour be considered a bug?  That is, should the contents
of the working directory (including untracked files) before the git
rebase invocation be returned (as if preserved by a git stash
--include-untracked)?  

If we don't expect this, should we update the documentation for the
--abort heading in the git rebase man page to indicate that newly
staged content will be lost after a git rebase --abort?

This is for git version 1.8.3

Paul

--
Paul A. Kennedy
pakenned@pobox.com

Re: unexpected file deletion after using git rebase --abort

[Jonathan Nieder]

Paul A. Kennedy wrote:

> If we don't expect this, should we update the documentation for the
> --abort heading in the git rebase man page to indicate that newly
> staged content will be lost after a git rebase --abort?

How about something along these lines?

diff --git i/Documentation/git-rebase.txt w/Documentation/git-rebase.txt
index 6b2e1c8..dcae40d 100644
--- i/Documentation/git-rebase.txt
+++ w/Documentation/git-rebase.txt
@@ -240,6 +240,9 @@ leave out at most one of A and B, in which case it defaults to HEAD.
 	started, then HEAD will be reset to <branch>. Otherwise HEAD
 	will be reset to where it was when the rebase operation was
 	started.
++
+This discards any changes to files tracked in the working tree or <branch>.
+You may want to stash your changes first (see linkgit:git-stash[1]).
 
 --keep-empty::
 	Keep the commits that do not change anything from its

Re: unexpected file deletion after using git rebase --abort

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> Paul A. Kennedy wrote:
>
>> If we don't expect this, should we update the documentation for the
>> --abort heading in the git rebase man page to indicate that newly
>> staged content will be lost after a git rebase --abort?
>
> How about something along these lines?
>
> diff --git i/Documentation/git-rebase.txt w/Documentation/git-rebase.txt
> index 6b2e1c8..dcae40d 100644
> --- i/Documentation/git-rebase.txt
> +++ w/Documentation/git-rebase.txt
> @@ -240,6 +240,9 @@ leave out at most one of A and B, in which case it defaults to HEAD.
>  	started, then HEAD will be reset to <branch>. Otherwise HEAD
>  	will be reset to where it was when the rebase operation was
>  	started.
> ++
> +This discards any changes to files tracked in the working tree or <branch>.
> +You may want to stash your changes first (see linkgit:git-stash[1]).
>  

"rebase --abort" is typically used to get rid of conflicted mess the
user does not want to resolve right now, and "stash" would not be a
sensible thing to use in such a situation, I think.  Doesn't it even
refuse to work if there is a conflicted entry in the index?

Re: unexpected file deletion after using git rebase --abort

[Paul A. Kennedy]

On Wed, Jul 03, 2013 at 04:04:23PM -0700, Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
> > Paul A. Kennedy wrote:
> > > If we don't expect this, should we update the documentation for the
> > > --abort heading in the git rebase man page to indicate that newly
> > > staged content will be lost after a git rebase --abort?
> >
> > How about something along these lines?
> >
> > diff --git i/Documentation/git-rebase.txt w/Documentation/git-rebase.txt
> > index 6b2e1c8..dcae40d 100644
> > --- i/Documentation/git-rebase.txt
> > +++ w/Documentation/git-rebase.txt
> > @@ -240,6 +240,9 @@ leave out at most one of A and B, in which case it defaults to HEAD.
> >  	started, then HEAD will be reset to <branch>. Otherwise HEAD
> >  	will be reset to where it was when the rebase operation was
> >  	started.
> > ++
> > +This discards any changes to files tracked in the working tree or <branch>.
> > +You may want to stash your changes first (see linkgit:git-stash[1]).
> >  
> 
> "rebase --abort" is typically used to get rid of conflicted mess the
> user does not want to resolve right now, and "stash" would not be a
> sensible thing to use in such a situation, I think.  Doesn't it even
> refuse to work if there is a conflicted entry in the index?

Thanks for thinking about this with me.  

After contemplating your messages, I think that it's unreasonable to
expect git rebase --abort to be able to properly handle content from
completely outside the repo and only placed in the index.

I think that Jonathan's suggestion makes too mild a point (and I
think Junio's objection may be a consequence of this).  I've added a
little paragraph to the documentation that covers two cases: what you
should do before you started (i.e. git-stash if messing about with
adding content); and how to recover in case you managed to "lose"
content in this way (hence the links to git-fsck and git-cat-file).

This is the diff (against v.1.8.3.2 in the git tree):

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index aca8405..ffaef29 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -238,6 +238,13 @@ leave out at most one of A and B, in which case it defaults to HEAD.
 	will be reset to where it was when the rebase operation was
 	started.
 
+	Untracked files added to the index will not be unstaged, and
+	therefore, not present in the working directory upon abort.
+	Unstage files before the abort, or stash untracked content before
+	starting the rebase (see linkgit:git-stash[1]).  Dangling blobs
+	may be found and recovered using fsck and cat-file (see
+	linkgit:git-fsck[1], linkgit:git-cat-file[1]).
+
 --keep-empty::
 	Keep the commits that do not change anything from its
 	parents in the result.

--
Paul A. Kennedy
pakenned@pobox.com

Re: unexpected file deletion after using git rebase --abort

[Eric Sunshine]

On Thu, Jul 4, 2013 at 3:35 PM, Paul A. Kennedy <pakenned@pobox.com> wrote:
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index aca8405..ffaef29 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -238,6 +238,13 @@ leave out at most one of A and B, in which case it defaults to HEAD.
>         will be reset to where it was when the rebase operation was
>         started.
>
> +       Untracked files added to the index will not be unstaged, and
> +       therefore, not present in the working directory upon abort.
> +       Unstage files before the abort, or stash untracked content before
> +       starting the rebase (see linkgit:git-stash[1]).  Dangling blobs
> +       may be found and recovered using fsck and cat-file (see
> +       linkgit:git-fsck[1], linkgit:git-cat-file[1]).
> +

Not commenting about the change in general, just one issue: The
transition to "dangling blobs" seems abrupt and may convey little
meaning to non-expert users. Perhaps lead in to that sentence with
something along the lines of:

  "If you neglect to unstage untracked files before abort, they become
dangling blogs, which may be found ..."

Also, a bit earlier, perhaps: s/Unstage files/Manually unstage files/

Re: unexpected file deletion after using git rebase --abort

[Junio C Hamano]

"Paul A. Kennedy" <pakenned@pobox.com> writes:

>> "rebase --abort" is typically used to get rid of conflicted mess the
>> user does not want to resolve right now, and "stash" would not be a
>> sensible thing to use in such a situation, I think.  Doesn't it even
>> refuse to work if there is a conflicted entry in the index?
>
> Thanks for thinking about this with me.  
>
> After contemplating your messages, I think that it's unreasonable to
> expect git rebase --abort to be able to properly handle content from
> completely outside the repo and only placed in the index.

The essense of "--abort" is to bring your index and the working tree
state back to the state you had before you got into a conflicted
mess.

For paths that still have higher/conflicted stages in the index, it
is easy to decide what should happen.  Because we do not even allow
rebase to start with an index with conflicted paths, they must have
come from the rebase operation itself, so matching them to what is
in the HEAD (and removing such conflicted paths that are not in the
HEAD) is always the right thing.

But for paths in the index that are already resolved at stage #0,
it is not simple.  They can come from

 (1) a clean automatic resolution (including the case where the
     rebase did not even touch the path).

 (2) a conflict that was resolved manually and then "git add"ed.

 (3) manual "git add" of a path that a mechanical part of the rebase
     operation did not touch at all.

For (1) and (2), it is the right thing to match with HEAD and
to remove the path if it is not in HEAD (e.g. the path may have
appeared by attempting to replay a change to add it).

For (3), a change may update existing files in HEAD or it may add a
new path not in HEAD.  Ideally, the former should be reverted to
HEAD, and the latter should be only reverted in the index but leave
the file in the working tree untracked.

But there is no good way to tell these three classes apart,
unfortunately.  With recent Git, I think you can tell paths in
category (2) by noticing that they have "REUC" entries for them in
the index extension section.

The only case we may want to behave differently is "git add" that
adds a path that does not exist (at any stage) to the index, so
theoretically, we could teach the end-user facing "git add" to leave
a new extension entry to the index, teach "git reset --no-so-hard"
to notice the index extension to leave the path in the working tree
when reverting to a HEAD that does not have such a path and use it
in "rebase --abort" codepath.  Which would also allow us to be less
aggressive in a case like this:

	... have many untracked and unignored paths
        $ git add .
        ... oops, I did not mean it
        $ git reset --hard
	... double-oops, these untracked paths are all gone

But that is "theoretical" primarily because it is unclear what the
exhaustive set of situations where we must clear such a "this is
newly added" mark to avoid false positives.  After "git commit" that
records such a path in a commit is obviously one of them, but that
is not the only one, and every place we forget to clear the mark
will be a cause of confusion.

Alternatively, perhaps we could create the "REUC" extension for
category (1) paths as well.  Then "git reset --not-so-hard $commit"
could be changed to behave the same way as it does today, except
that it would not remove paths without "REUC" extension entry from
the working tree.

> +	Untracked files added to the index will not be unstaged, and
> +	therefore, not present in the working directory upon abort.
> +	Unstage files before the abort, or stash untracked content before
> +	starting the rebase (see linkgit:git-stash[1]).

"Unstage files" is a less than ideal suggestion.

	$ git rebase
        ... oops conflicted
        ... ouch, this is too much for me to resolve
        $ git reset
        $ git rebase --abort

would leave a path that was introduced by the change being replayed
in the working tree, which may later interfere when you try to
checkout a branch that does have that path.  At least it has to be
"Unstage such files that were untracked and added by you" to be
helpful, I think.