Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

7 messages, 4 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

From: Matthieu Moy <hidden>
Date: 2016-06-15 22:56:50

Ramkumar Ramachandra [off-list ref] writes:
Matthieu Moy wrote:
quoted
AFAICT, "git merge --abort" is an alias for "git reset --merge"
Yes, that is correct.
quoted
which
was precisely designed to reset only modifications comming from a merge,
and not the local changes that were present before the merge was
started. The man pages are relatively obscure on the subject, but I'd
call that a documentation bug.
I see.  Either way, we need a clean worktree for it to work, no?
No, you don't. Just try if you're not convinced:

$ git checkout -b branch
Switched to a new branch 'branch'
$ date > test.txt && git commit -m 'on branch' test.txt
[branch 2482623] on branch
 1 file changed, 1 insertion(+), 1 deletion(-)
$ git checkout -
Switched to branch 'master'
$ date > test.txt && git commit -m 'on master' test.txt
[master c322d35] on master
 1 file changed, 1 insertion(+), 1 deletion(-)
$ date > other.txt 
$ git status
# On branch master
# Changes not staged for commit:
#
#       modified:   other.txt
#
no changes added to commit (use "git add" and/or "git commit -a")
$ git merge branch
Auto-merging test.txt
CONFLICT (content): Merge conflict in test.txt
Automatic merge failed; fix conflicts and then commit the result.
$ git status
# On branch master
# You have unmerged paths.
#
# Unmerged paths:
#
#       both modified:      test.txt
#
# Changes not staged for commit:
#
#       modified:   other.txt
#
no changes added to commit (use "git add" and/or "git commit -a")
$ git merge --abort
$ git status
# On branch master
# Changes not staged for commit:
#
#       modified:   other.txt
#
no changes added to commit (use "git add" and/or "git commit -a")
$ 

There may be corner-cases where it doesn't work, but I never encountered
such case.
quoted
It does. stashing means the user will have to "stash pop" later. One
extra step, one extra opportunity to forget something important.
That's only if there are conflicts.  If there are conflicts, you'll
have to stash anyway if:
- You're doing a pull-merge and want merge --abort to work.
Again, no.
quoted
A minor annoyance is that it will touch files that have no reason to be
touched, hence may trigger extra rebuilds with "make", disturbing text
editors that have the file open, etc.
Okay, I need to ask you something at this point: do you ever run merge
on a dirty worktree unless you're absolutely sure that your local
changes won't conflict with the changes introduced by the merge? 
Most of the time, I just run "git pull" or "git merge". I know it's
conservative enough, to it will stop if there's anything dangerous.
That's only a pull-merge.  Unfortunately, making git-pull.sh uniform
means that we have to fall back to the least-common-denominator of
functionality (which is currently pull-rebase).
You may want to, but you don't have to. pull-merge and pull-rebase
already have different behavior in case of non-overlapping changes:

$ git pull --rebase . branch
Cannot pull with rebase: You have unstaged changes.
Please commit or stash them.
$ git pull --no-rebase . branch
From .
 * branch            branch     -> FETCH_HEAD
[...]

I don't see any reason to restrict to the common denominator in the same
situation for another feature.

I can accept the "it's too hard to implement" argument, but not "it
doesn't bring anything".
quoted
As a user, when I run "git rebase --continue" and it tells me it's done,
I expect the work to actually be done. This is the case today. This
won't be the case after autostash is introduced if the user has to
remember to run "stash pop" afterwards.
And how will you implement that for merge, since there is no merge
--continue to execute stash pop from?  Do you propose to make commit
do the stash pop'ing?
No, I'm not proposing to do anything for merge. There's no reason to try
being uniform in conflict resolution for pull-merge and pull-rebase as
it is already different now. We already have "git rebase --continue", we
don't have "git merge --continue". So what? The fact that merge doesn't
have the equivalent doesn't mean we should not do something for "rebase
--continue".

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

From: Ramkumar Ramachandra <hidden>
Date: 2016-06-15 22:56:50

Matthieu Moy wrote:
No, you don't. Just try if you're not convinced:
Oh, I trusted the documentation on this one and never tried with a
dirty worktree myself.  Please fix the documentation, if you know how
exactly to correct it.
No, I'm not proposing to do anything for merge. There's no reason to try
being uniform in conflict resolution for pull-merge and pull-rebase as
it is already different now. We already have "git rebase --continue", we
don't have "git merge --continue". So what? The fact that merge doesn't
have the equivalent doesn't mean we should not do something for "rebase
--continue".
Well, you can't blame me for the misunderstanding then.

In a previous email, you wrote:
Shouldn't this belong to "git merge" instead (i.e. implement "git merge
--autosquash" and call it from "git pull")? Users doing "git fetch &&
git merge" by hand should be able to use --autosquash, I think.
Junio's criticism of pull.autostash hurting pull-merge people is
cogent; my current plan is to ditch pull.autostash altogether, and
implement rebase.autostash for the reduced case of a non-interactive
rebase.

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

From: John Keeping <hidden>
Date: 2016-06-15 22:56:50

On Mon, Apr 15, 2013 at 10:15:54PM +0530, Ramkumar Ramachandra wrote:
Junio's criticism of pull.autostash hurting pull-merge people is
cogent; my current plan is to ditch pull.autostash altogether, and
implement rebase.autostash for the reduced case of a non-interactive
rebase.
Why restrict it to non-interactive?  I'd find it useful when doing
interactive rebases as well - consider the case when you simply want to
re-order some commits.

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

From: Ramkumar Ramachandra <hidden>
Date: 2016-06-15 22:56:50

John Keeping wrote:
Why restrict it to non-interactive?  I'd find it useful when doing
interactive rebases as well - consider the case when you simply want to
re-order some commits.
Actually, I made a mistake: it should be doable for any specific
rebase (includes rebase--interactive, rebase--am, and rebase--merge)
just as easily, without leaking the autostash detail into them.  The
last statement in git-rebase.sh is run_specific_rebase, which just
needs to be wrapped in a git stash/ git stash pop.

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:56:51

Ramkumar Ramachandra wrote:
Matthieu Moy wrote:
quoted
No, you don't. Just try if you're not convinced:
Oh, I trusted the documentation on this one and never tried with a
dirty worktree myself.  Please fix the documentation, if you know how
exactly to correct it.
The manual says:

	"git pull" and "git merge" will stop without doing anything when
	local uncommitted changes overlap with files that git pull/git
	merge may need to update.

That accurately describes the behavior.

I wouldn't be surprised if there's still a documentation bug, though:
a lack of clarity, a missing hint somewhere else, something else
misleading.  That seems especially likely when you say "I trusted the
documentation on this one".  Care to point to the appropriate section,
so it can be fixed?

Thanks,
Jonathan

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

From: Ramkumar Ramachandra <hidden>
Date: 2016-06-15 22:56:51

Jonathan Nieder wrote:
I wouldn't be surprised if there's still a documentation bug, though:
a lack of clarity, a missing hint somewhere else, something else
misleading.  That seems especially likely when you say "I trusted the
documentation on this one".  Care to point to the appropriate section,
so it can be fixed?
As I pointed out in a previous email, I'm referring to the --abort
section of git-merge(1):

"If there were uncommitted worktree changes present when the merge
started, git merge --abort will in some cases be unable to reconstruct
these changes. It is therefore recommended to always commit or stash
your changes before running git merge."

Matthieu (and probably others) run git merge with a dirty worktree
most of the time, while I never do (because I read this section).

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:56:51

Ramkumar Ramachandra wrote:
"If there were uncommitted worktree changes present when the merge
started, git merge --abort will in some cases be unable to reconstruct
these changes. It is therefore recommended to always commit or stash
your changes before running git merge."

Matthieu (and probably others) run git merge with a dirty worktree
most of the time, while I never do (because I read this section).
The above says "in some cases".  It's presumably referring to the
case "One exception is when the changed index entries are in the state
that would result from the merge already" described in the
"PRE-MERGE CHECKS" section.

Improved wording welcome.

Ciao,
Jonathan
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help