bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Eli Zaretskii <eliz@gnu.org>]

> Cc: monnier@iro.umontreal.ca, esr@snark.thyrsus.com, 20292@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Thu, 14 May 2015 04:24:12 +0300
> 
>     Report a bug in Git, I think.
> 
> I believe it's your turn to report a Git bug now. ;)

So we do turns on this?

> Still, even if it's fixed, we'll have a lot of users, for years to come, that use Git without that fix. Note how you and I are using quite different versions, and the latest release is 2.4.1.

I only have an old version because there's no newer one for Windows,
and I can't be bothered enough to build my own.

Anyway, the fact that it takes a long time for a fix to percolate
shouldn't preclude us from reporting it.

> > It doesn't make sense to have the
> > outcome of "stash pop" wrt the index/staging depend on whether there
> > were conflicts or not, which is what happening here: if I "stash pop"
> > after pulling when none of my local stashed changes are in conflict
> > with the pulled/merged content, I get back modified and unstaged
> > files.  Why would the existence of conflicts during "stash pop"
> > produce any different effect for files _without_ conflicts, except by
> > some obscure bug?
> 
> As a wild guess, maybe the files that get staged automatically are the result of automatic conflict resolution (there was some divergence, but it was resolved automatically; maybe the "no divergence" case is also treated like this, for simplicity). IOW, the "worse is better" kind of reasons.

No, I reproduced this when some of the stashed files were not changed
at all upstream, i.e. there shouldn't have been a need for any
conflict resolution, automatic or otherwise, in those files.

_My_ wild guess is that Git simply invokes the same code as is used in
a "normal" merge, and that one stages files that are without conflicts.

>         Unstage the automatically-staged files?
> 
>     If we can do that, yes.  But how do we know which files to unstage?
> 
> That the the difficulty: right after applying the stash we could know (all of them!), but Emacs can't know whether the user staged anything else between then and now (when all conflicts have been resolved). IOW, the user is better positioned to call 'git reset'.

The user is always better positioned, but we'd like VC to DTRT in the
more popular situations where the user could be saved from the
nuisance of figuring things out and typing shell commands.  That's the
main goal of VC, isn't it?

If we know all the stashed files, how about invoking "git reset" for
all of them?  It cannot hurt, can it?

>     No!  That'd be a step back.  The current treatment of stashed changes
>     is better than it was before the change.  Also note that conflicts
>     like this are quite rare, so the way vc-git worked previously was
>     wrong in 99% of cases, why the one we have now is wrong in perhaps
>     0.1%.
> 
> I don't know where you got the percentages. My stashes routinely touch multiple files, and it's easy to imagine how not all of them could have conflicts after applying.

I'm talking about conflicts, not about the number of files.  How many
times did you have conflicts in "stash pop"?  I almost never have
them.

> The odds are hard to calculate, but the probability really must be in tens of percents, not below one.

Now I wonder where did _you_ get your percentages.

> The current behavior is bad because it looks random.

I agree it's bad, but only if (a) there are multiple changed files,
and (b) some, but not all, of them have conflicts.  Otherwise, the
behavior is correct.  By contrast, the previous behavior was always
wrong.

>     It seems to me we've uncovered a bug in Git (gasp!).  Git has no
>     reasons to want the changes staged, certainly not depending on whether
>     there were conflicts.
> 
> Staging changes is the Git way to mark conflict as resolved.

Not for uncommitted changes that were stashed, it ain't.  For "normal"
merge conflicts, yes, because a conflict-free merge would have
committed the changes, so staging is a step in the right direction.
But for conflicts in stashed uncommitted changes, it's a step in the
wrong direction, especially in files that didn't have conflicts at
all.

> Ergo, it expects the conflicting files to be staged after the user resolves the conflicts. Then it won't make a lot of sense to leave the rest of the files unstaged, would it? Maybe that's the reasoning.

It's a flawed reasoning, IMO.  I stashed the changes because they are
not yet ready to be committed, and I wanted them out of my way for a
while.  When I pop the stash, I want them uncommitted as they were
before.  Having them staged means that I might inadvertently commit
them with my next "git commit", something that wasn't supposed to
happen before the stash.

> It's hard for me to tell without knowing exactly why Git conflates conflict resolution and staging.

It's a bug.