[BUG] "git checkout BRANCH -- FILE" deletes staged commits

[BUG] "git checkout BRANCH -- FILE" deletes staged commits

[Tasnad Kernetzky]

[-- Attachment #1.1: Type: text/plain, Size: 1142 bytes --]

Hi,

I suppose I found a bug in git version 2.24.0. Please consider the small
shell script to reproduce.

Best,

Tasnad


#!/bin/bash
# init repo, create master branch
git init
echo 'master' > tst
git add tst
git ci -m 'master'

# create branch "B" where tst contains the line from master plus a line "B"
git checkout -b B
echo 'B' >> tst
git commit -am 'B'

# back in master: stage a change
git checkout master
echo 'master_dirty' >> tst
git add tst
echo '*** tst should contain [master, master_dirty]: ***'
cat tst
echo '-------------'

# checkout copies over contents from B -> I guess the bug is here,
changes are not detected
git checkout B -- tst
git status
echo '-> this seems to be wrong, change is not detected'

# if we now go to B and back to master, we loose staged changes
echo '*** tst should contain [master, B]: ***'
cat tst
echo '-------------'
git checkout B
echo '*** tst should contain [master, B]: ***'
cat tst
echo '-------------'
git checkout master
echo '*** tst should contain [master, B]: ***'
cat tst
echo '-------------'
echo '-> wrong / staged changes lost!?'



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [BUG] "git checkout BRANCH -- FILE" deletes staged commits

[Brandon McCaig]

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

On Tue, Nov 19, 2019 at 06:10:01PM +0100, Tasnad Kernetzky wrote:
> Hi,

Hello,

> I suppose I found a bug in git version 2.24.0. Please consider the small
> shell script to reproduce.
*snip*
> # checkout copies over contents from B -> I guess the bug is here,
> changes are not detected
> git checkout B -- tst
> git status
> echo '-> this seems to be wrong, change is not detected'
> 
> # if we now go to B and back to master, we loose staged changes
> echo '*** tst should contain [master, B]: ***'
> cat tst
> echo '-------------'
> git checkout B
> echo '*** tst should contain [master, B]: ***'
> cat tst
> echo '-------------'
> git checkout master
> echo '*** tst should contain [master, B]: ***'
> cat tst
> echo '-------------'
> echo '-> wrong / staged changes lost!?'

I don't think this is a bug at all.

This form of checkout is basically a way to "revert" or "restore"
a file to match whichever tree-ish you select:

    git checkout ... -- file...

It is not a merge. It is overwriting whatever version of the file
you have, and staging those changes. There's no point warning you
that your file is different because you're giving git strict
instructions to overwrite the file. You shouldn't be doing that
unless you already know what the file's state is and you know you
don't want it. If in doubt stash, commit, or copy the file
first.

When you do `git checkout B -- tst` you are basically saying to
checkout the file tst as it exists at the tip of branch B and
stage it for commit.

When you switch back to branch B the state of the tst file is the
same as it exists in the branch B. There is no conflict here so
it succeeds, and once it does you no longer have any changes made
to tst because the version in your index and working tree matches
the version in the HEAD commit.

git status at this point would report nothing (assuming no other
files are modified).

When you switch back to master it happily overwrites tst to match
the version committed to the head of master because the version
you had on B is already committed (and is easily recovered from B
if desired). There's no conflict there and no reason to fail or
refuse.

This is normal behavior. Check the documentation and if you think
it's unclear you can make suggestions to improve it or better yet
submit a patch that does just that for review.

I haven't really been paying much attention to the development of
git, but I recall efforts were being made to split the various
behaviors of git-checkout into separate commands to help improve
the interface because some people struggle with it. Sounds like
these take the form of git-switch to change branches and
git-restore to restore files to some other version. It sounds
like git-restore will still work the same way in this regard
though so I don't think that changes anything.

Regards,



-- 
Brandon McCaig <bamccaig@gmail.com> <bambams@castopulence.org>
Castopulence Software <https://www.castopulence.org/>
Blog <http://www.bambams.ca/>
perl -E '$_=q{V zrna gur orfg jvgu jung V fnl. }.
q{Vg qbrfa'\''g nyjnlf fbhaq gung jnl.};
tr/A-Ma-mN-Zn-z/N-Zn-zA-Ma-m/;say'


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Fwd: [BUG] "git checkout BRANCH -- FILE" deletes staged commits

[Tasnad Kernetzky]

Hi Brandon,

Thanks for your deep inspection!

I know that the checkout command I was using was supposed to overwrite
the file, but I didn't know that it automatically stages changes. So I
agree, that step makes sense and I should have rtfm.

My only suggestion for the docs would be to add "staging" or so to the
sentence "When a |<tree-ish>| is given, the paths that match the
|<pathspec>| are updated both in the index and in the working tree.".
However, I think it's also clear enough as-is, so I don't see much room
for improvement there.

Please find my last comment in-line.


Best,

Tasnad


On 22.11.19 04:14, Brandon McCaig wrote:
[...]
> When you switch back to branch B the state of the tst file is the
> same as it exists in the branch B. There is no conflict here so
> it succeeds, and once it does you no longer have any changes made
> to tst because the version in your index and working tree matches
> the version in the HEAD commit.
>
> git status at this point would report nothing (assuming no other
> files are modified).

This is the point I actually considered as a bug. There are staged
changes and usually git doesn't let me switch away from a branch in such
cases.

Although I don't lose data, I do lose the newest state of the master
branch, if I'm not cautious and remember that I copied over changed from
"B".

I would prefere at least a warning :) 

Re: [BUG] "git checkout BRANCH -- FILE" deletes staged commits

[Brandon McCaig]

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

Tasnad:

On Mon, Nov 25, 2019 at 09:15:18PM +0100, Tasnad Kernetzky wrote:
> Hi Brandon,

Hello,

> On 22.11.19 04:14, Brandon McCaig wrote: [...]
> > When you switch back to branch B the state of the tst file is
> > the same as it exists in the branch B. There is no conflict
> > here so it succeeds, and once it does you no longer have any
> > changes made to tst because the version in your index and
> > working tree matches the version in the HEAD commit.
> >
> > git status at this point would report nothing (assuming no
> > other files are modified).
> 
> This is the point I actually considered as a bug. There are
> staged changes and usually git doesn't let me switch away from
> a branch in such cases.

I think that Git normally will let you change branches even if
you have changes in your index or working tree as long as the
version of the file in your destination branch matches the HEAD
version. In other words no merge is necessary and no changes can
be lost. I regularly do this when I start making changes on
develop or master, and then decide to branch after the fact. It's
a bit less typing than stashing in between. :)

For example:

	git init checkout
	cd checkout
	echo master >log
	git add log
	git commit -m master
	git checkout -b B
	echo B >>log
	git add log
	git checkout master
	git status

Regards,


-- 
Brandon McCaig <bamccaig@gmail.com> <bambams@castopulence.org>
Castopulence Software <https://www.castopulence.org/>
Blog <http://www.bambams.ca/>
perl -E '$_=q{V zrna gur orfg jvgu jung V fnl. }.
q{Vg qbrfa'\''g nyjnlf fbhaq gung jnl.};
tr/A-Ma-mN-Zn-z/N-Zn-zA-Ma-m/;say'


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]