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

2013-04-16 Thread Ramkumar Ramachandra 
Matthieu Moy wrote:
 Because git rebase needs multiple runs in case of conflicts. You have
 to store the information somewhere in the filesystem, not in a variable.
 What you need to store is whether you need to unstash, and where you are
 in the process (in Junio's version, you may be doing the actual rebase,
 or fixing conflicts in index state application, or fixing conflicts in
 tree state application, or done). Storing what you have to do and where
 you are in the process really sounds like a job for the instruction
 sheet, no?

No.  Ultimately, the entry point of all these invocations is
git-rebase.sh.  The plan is to refactor calls from git-rebase.sh to
git-rebase--*.sh scripts so that those scripts return control to
git-rebase.sh, which will be the final exit point.  The logic is very
simple: On the very first invocation of rebase (ie. no existing rebase
in progress), stash.  If the return statement from the specific rebase
script is 1 (which means that there are conflicts to be resolved),
exit as usual.  If it is 0 (which means that the rebase completely
successfully), pop the stash before exiting as usual.

What's so complicated about that?  I'm against leaking the autostash
implementation detail into specific rebases, because I value a clean
and pleasant implementation over everything else.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-16 Thread Matthieu Moy 
Ramkumar Ramachandra artag...@gmail.com writes:

 If it is 0 (which means that the rebase completely successfully), pop
 the stash before exiting as usual.

And you're going to pop the stash even if no stash were triggered when
starting the rebase.

Really, think about it again: you're not going to guess whether you have
to unstash without storing this information somewhere. You may argue
about storing it outside the todolist, you can't unstash
unconditionally.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-16 Thread Ramkumar Ramachandra 
Matthieu Moy wrote:
 And you're going to pop the stash even if no stash were triggered when
 starting the rebase.

 Really, think about it again: you're not going to guess whether you have
 to unstash without storing this information somewhere. You may argue
 about storing it outside the todolist, you can't unstash
 unconditionally.

Yes, touching a simple autostash file at stash-time, and removing it
at pop-time will do.  I don't see why it should be part of a
(potentially user-editable) todo-list, which serves an entirely
different purpose.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-16 Thread Matthieu Moy 
Ramkumar Ramachandra artag...@gmail.com writes:

 Yes, touching a simple autostash file at stash-time, and removing it
 at pop-time will do.  I don't see why it should be part of a
 (potentially user-editable) todo-list, which serves an entirely
 different purpose.

You'll have to apply the index state and then the tree state. How do you
know whether the next call to git rebase should apply one or the other?

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-16 Thread Matthieu Moy 
Matthieu Moy matthieu@grenoble-inp.fr writes:

 Ramkumar Ramachandra artag...@gmail.com writes:

 Yes, touching a simple autostash file at stash-time, and removing it
 at pop-time will do.  I don't see why it should be part of a
 (potentially user-editable) todo-list, which serves an entirely
 different purpose.

 You'll have to apply the index state and then the tree state. How do you
 know whether the next call to git rebase should apply one or the other?

Plus: what happens if the user ran git stash during rebase? In Junio's
version, the right commits are applied. Running blindly stash pop may
pop the wrong stash.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-16 Thread Phil Hord 
On Tue, Apr 16, 2013 at 5:20 AM, Ramkumar Ramachandra
artag...@gmail.com wrote:
 Matthieu Moy wrote:
 No.  Ultimately, the entry point of all these invocations is
 git-rebase.sh.  The plan is to refactor calls from git-rebase.sh to
 git-rebase--*.sh scripts so that those scripts return control to
 git-rebase.sh, which will be the final exit point.  The logic is very
 simple: On the very first invocation of rebase (ie. no existing rebase
 in progress), stash.  If the return statement from the specific rebase
 script is 1 (which means that there are conflicts to be resolved),
 exit as usual.  If it is 0 (which means that the rebase completely
 successfully), pop the stash before exiting as usual.

 What's so complicated about that?  I'm against leaking the autostash
 implementation detail into specific rebases, because I value a clean
 and pleasant implementation over everything else.

It can be more complex than you realize.

   $ git pull --rebase --stash

It seems that there is already a .git/rebase-apply directory, and
I wonder if you are in the middle of another rebase.  If that is the
case, please try
git rebase (--continue | --abort | --skip)
If that is not the case, please
rm -fr .git/rebase-apply
and run me again.  I am stopping in case you still have something
valuable there.

If I follow the latter advice about 'rm -rf', who will remember that
'rebase' had something stashed, and what will they do with it when
they do?

What if it is weeks or months later?  I would be surprised to see
long-forgotten wip show up in my workspace all of a sudden.

Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-16 Thread Junio C Hamano 
Matthieu Moy matthieu@grenoble-inp.fr writes:

 Ramkumar Ramachandra artag...@gmail.com writes:

 If it is 0 (which means that the rebase completely successfully), pop
 the stash before exiting as usual.

 And you're going to pop the stash even if no stash were triggered when
 starting the rebase.

 Really, think about it again: you're not going to guess whether you have
 to unstash without storing this information somewhere. You may argue
 about storing it outside the todolist, you can't unstash
 unconditionally.

Yes, it can be part of todo, but it does not have to be.

Regardless of where the information is stored, implementing the last
step as stash pop will add a small inconsistency the end user
needs to be aware of.

Imagine git rebase stops, asks you to help with conflicts, and you
look at it, play with the working tree, and made a mess.  If this
was the last stash pop invocation, the way to go back and start
again is reset --hard  stash pop.  For all the other steps, that
is not how the user goes back to the originally conflicted state in
order to retry from scratch.

Makes me wonder if the world were a better place if the rebase-todo
list were implemented as a dedicated stash and rebase --continue
were a mere stash pop followed by a commit if pop goes smoothly.

I am not suggesting to actually do so, but it is an intriguing
thought from the perspective of end user experience, isn't it?

In any case, I am not saying that it is a _bad_ thing to implement
the last restore the WIP stage as stash pop. I am just pointing
out that the messaging needs to be done carefully to make sure the
user understands which one is which, as the way to deal with the
situation where it stops and asks the user for help would be
different from normal step in the rebase sequence that replays a
commit.


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-16 Thread Ramkumar Ramachandra 
Phil Hord wrote:
 If I follow the latter advice about 'rm -rf', who will remember that
 'rebase' had something stashed, and what will they do with it when
 they do?

 What if it is weeks or months later?  I would be surprised to see
 long-forgotten wip show up in my workspace all of a sudden.

Ultimately, I think an ideal implementation requires this entire
autostash implementation to reside completely within the $state_dir.
The issue of a long-forgotten WIP is then the same issue as a
long-forgotten rebase, and a rm -rf $state_dir will get rid of the WIP
as well.

The other reason is that it shouldn't interact with the rest of git.
This means no touching any refs or reflogs, and this can be
problematic if we decide to use the standard git stash.  I'm currently
working towards seeing if it's possible to get stash to create named
stashes that we can predictably retrieve later, to avoid rolling our
own homegrown stash.

Yes, the problem is much more complex than I initially thought.  It
was much simpler to implement it for git-pull.sh.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-15 Thread Matthieu Moy 
Ramkumar Ramachandra artag...@gmail.com writes:

 +stash_required () {
 + ! (git diff-files --quiet --ignore-submodules 
 + git diff-index --quiet --cached HEAD --ignore-submodules)
 +}

Isn't this too pessimistic? If the local changes do not overlap (in
terms of files) with the pulled changes, then autosquash is not needed.

There should be a test for the case of non-overlapping changes.

 +if test $autostash = true  stash_required
 +then
 + git stash || die $(gettext Cannot autostash) 
 + require_clean_work_tree pull Please commit or stash them. 
 + if eval $eval
 + then
 + git stash pop || die $(gettext Cannot pop stash)
 + else
 + exit_code=$?
 + echo Please run 'git stash pop' after commiting the conflict 
 resolution.
 + exit $exit_code
 + fi
 +else
 + eval exec $eval
 +fi

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.

Something should be done for git rebase and git pull --rebase too.
In this case, the UI can be much smoother, since the user would have to
run git rebase --continue anyway, so you can do the auto-stash-pop for
him by adding something like exec git stash pop at the end of the todo-list.

Ideally, git rebase --abort should auto-stash-pop too, although this
is much less trivial to implement.

Maybe a good first step is to implement --autosquash only for non-rebase
pull, and later to implement git rebase --autosquash and call it from
git pull.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-15 Thread Junio C Hamano 
Matthieu Moy matthieu@grenoble-inp.fr writes:

 Ramkumar Ramachandra artag...@gmail.com writes:

 +stash_required () {
 +! (git diff-files --quiet --ignore-submodules 
 +git diff-index --quiet --cached HEAD --ignore-submodules)
 +}

 Isn't this too pessimistic? If the local changes do not overlap (in
 terms of files) with the pulled changes, then autosquash is not needed.

Yes, that is why I said for pull-merge, --authsquash is neutral-to-better
and pull.autosquash is harmful.

But for pull-rebase folks, I can understand why this working tree
must be squeakily clean logic is appropriate, if we were to do
this. The root cause is because rebase insists to be run on such a
working tree.

And the worst part is that in order to check if local changes
overlap, you need to fetch first. But Ram's annoyance is about the
user being told the merge/rebase cannot proceed _after_ fetch is
done.

 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.

See my other message. I do not think autosquash would help merge
folks very much, and will actively hurt when it matters.

 Something should be done for git rebase and git pull --rebase too.

That I would agree. I am not sure autosquash is the best approach,
but we should be able to help them more.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-15 Thread Matthieu Moy 
Junio C Hamano gits...@pobox.com writes:

 Yes, that is why I said for pull-merge, --authsquash is neutral-to-better
 and pull.autosquash is harmful.

To speak from my experience: I find myself typing git stash  git pull
 git stash pop relatively often. Typical use-case: I start working on
something, a colleague works on the same thing and I need to see what he
did to continue. Probably not something so frequent for large projects,
but very frequent for small projects (e.g. writing a paper together with
a single .tex file and the deadline approaching). In this case, git
pull --rebase makes sense for advanced enough users, but newbies who
have been told rebase is too dangerous for you, don't use it, it would
be cool to have --autostash too.

I tend to agree that pull.autostash is harmful. At least in its current
form, it is really too easy to overlook the Please run 'git stash pop'
after commiting the conflict resolution. message:

do some important changes
$ git pull
fix conflicts
$ git status
tells me to commit
$ git commit
WTF, where are my important changes?!?

rebase wouldn't have this issue if stash pop is part of the sequence
to apply.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-15 Thread Ramkumar Ramachandra 
Junio C Hamano wrote:
 In the end, simple cases (the canonical example being Linus keeping
 a local change to Name = Unicycling Gorilla in his Makefile while
 running dozens of git pull every day) would not be helped with
 this feature very much because there is rarely overlap, while in a
 complex case the user really needs help to save away his WIP, the
 user is forced to resolve the conflict immediately upon the failed
 stash pop at the end of git pull.  If the conflict turns out to
 be something the user would not want to resolve at that moment, the
 user needs to know the way out, something like:

 git reset --hard;# go back to the result of pull
 git checkout -b wip ORIG_HEAD   ;# start from before the pull
 git stash pop   ;# recover his wip
 ... perhaps work a bit more ...
 git commit -a -m wip;# save it away for safety
 git checkout -  ;# result of pull

 so that he can come back tomorrow, check out wip branch, and
 decide what to do.

What does this have to do with pull at all?  git-pull.sh is a simple
shell script that runs fetch followed by a merge or rebase.  You're
handling a merge now, and there's proper support for it in the tool;
git merge --abort will work with one caveat:  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., to directly quote the manpage.  Really, it has
the same problem as rebase, if you want --abort to work.

Fixing git merge --abort and git rebase are topics for another day.
What I'm doing it making it easier to work with the merge/rebase
following the fetch by automatically stashing worktree + index
changes.

 The --autosquash option (or not adding either the configuration or
 the option) would encourage the user to think about the nature of
 WIP he has in the working tree before running git pull.

I'm lost.  What does git rebase --autosquash (or rebase.autosquash)
have anything to do with git pull --autostash?

 Is it a
 too complex beast that may interfere with what others are doing, or
 is it a trivial change that he can stash away and pop it back?  If
 the former, he will be far better off doing checkout -b, keep
 working until the local change gets into somewhat a better shape and
 commit, before pulling into the original branch.

A pull-merge person having complex worktree changes should not execute
git pull blindly in the first place.  That's what git fetch is for:
inspect the incoming changes, and decide how to integrate it into the
local branch.  git pull is a tool that serves an entirely different
purpose, in my opinion.

 But I suspect that pull.autostash
 configuration is not a good addition because it encourages a bad,
 pain-inducing workflow.  In simple cases it may not hurt, but when
 local changes are complex, it would actively hurt than not having it,
 and the configuration robs the incentive to choose.

But I'm a pull-rebase guy, and I always want pull.autostash.  Will you
deny me the configuration variable, simply because it is potentially
pain-inducing to pull-merge people who set it blindly and run git pull
blindly?

I'm not arguing linking pull.autostash to pull.rebase either: my
change is admittedly useful to *some* pull-merge people.  And I'm not
arguing for making it true by default.

 The equation is somewhat different for pull-rebase, as rebase
 insists you to start from a clean working tree, so download and
 then stop annoyance feels bigger.  I have a suspicion that
 loosening that may be a more productive fix to the real problem.

Rebase performs relatively simple operation on the worktree, and I
would like to keep it that way.  I have no desire to fix rebase.

 (require_clean_work_tree --quiet) || git stash

Good point.  Will fix in the next iteration.

 This cannot pop stash is really where the user gets in the deep
 yoghurt and needs help.

Yes, and your point being?  That life would've been easier if the user
had committed all those changes in the first place?  And that the
penalty for not having done that is now a git checkout wip ORIG_HEAD;
git stash pop; git checkout master; git rebase wip?

 Please resolve conflicts and commit, and then 'git stash pop'.

Cool, will fix.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-15 Thread Ramkumar Ramachandra 
Matthieu Moy wrote:
 Isn't this too pessimistic? If the local changes do not overlap (in
 terms of files) with the pulled changes, then autosquash is not needed.

 There should be a test for the case of non-overlapping changes.

In the pull-rebase case, no; it is not too pessimistic.

In the pull-merge case, maybe: if your worktree isn't clean, you lose
a lot of goodies like merge --abort anyway, and I hate that.
Secondly, it's impossible to determine whether the changes overlap or
not before actually running merge_trees().  If there were an easy way
to do it, I might have considered it.

Overall, I don't see how an extra stash/ stash pop where not
absolutely necessary hurts.

 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.

--autosquash reminds me of rebase.autosquash, and that is completely
unrelated to the issue at hand.  Did you mean git merge --squash (to
update the worktree/index but not create the actual commit?).  Sure,
it's probably useful to have a merge.squash configuration variable,
but I don't see what it has to do with the pull.autostash I'm
proposing.

 Something should be done for git rebase and git pull --rebase too.
 In this case, the UI can be much smoother, since the user would have to
 run git rebase --continue anyway, so you can do the auto-stash-pop for
 him by adding something like exec git stash pop at the end of the todo-list.

No.  I'm against executing a special codepath for a pull-rebase that
has no equivalent in the pull-merge world.  Or did you mean: have one
configuration variable to git merge --squash and do this for git
rebase, as if they're equivalent from the pull perspective?  No, they
aren't.

 Ideally, git rebase --abort should auto-stash-pop too, although this
 is much less trivial to implement.

As I already pointed out in my message to Junio, fixing rebase is
not the topic of discussion at all.

 Maybe a good first step is to implement --autosquash only for non-rebase
 pull, and later to implement git rebase --autosquash and call it from
 git pull.

Implement git rebase --autosquash?  If I just set rebase.autosquash
to true, the rebase will automatically autosquash whether called from
git pull or otherwise.  Sorry, I just don't understand what you're
saying.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-15 Thread Ramkumar Ramachandra 
Matthieu Moy wrote:
 I disagree. A configuration option is something you set once, and then
 forget about. A command, or a command-line option, is something you
 explicitely add when you need it.

You're making it out to be a much bigger difference than it actually
is.  Users will simply alias pull to 'pull --autostash' (a lot of them
already alias it to pull --ff-only, and I'm going to fix this soon).
The decision making process for creating a configuration variable
shouldn't be this is potentially dangerous, and therefore therefore
it shouldn't be a configuration variable, but rather this is a
rarely used option that users only need 50% of the time, and
therefore it shouldn't be a configuration variable.  In my case,
pull.autostash is my 90~95% usecase, and I'm not unique in this
aspect.  Therefore, it should be a configuration variable that can be
consciously turned off with a --no-autostash.

If your criticism were that git status doesn't show stash state, I
agree with you.  However, I don't agree with basing it on user
forgetfulness in having set pull.autostash a long time ago + lack of
observation skills to notice the message printed by git pull.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-15 Thread Ramkumar Ramachandra 
Ramkumar Ramachandra wrote:
 Okay, so you're proposing a uniform --autostash feature for both merge
 and rebase.  Sorry, I didn't get it last time due to the typos in your
 email; yeah, this is worth investigating.

So, I thought about this and have concluded that pull is the Right
place for autostash because of the following reasons:

0. autostash is purely a convenience function that's useful in simple
reduced cases like a dumb 'git pull'; when the script we're
implementing it for doesn't have a ton of command-line options.

1. git stash; git pull; git stash pop; is a very common idiom.  git
stash; git rebase master; git stash pop; is a less common idiom, but I
agree that it is one rebase case where autostash could be useful.
Having git rebase -i show exec git stash pop at the end of a
user-editable instruction sheet is less than ideal.  Having git rebase
--onto master HEAD~3 do an autostash _might_ be useful, but I'm not
sure about it.  git stash; git merge feature-branch; git stash pop; is
not a common idiom at all.

2. git-stash.sh is a helper script, in the same spirit as git-pull.sh
and git-rebase.sh.  It is natural and easy to implement autostash for
pull and rebase, but not for a C builtin like merge.  In fact, it's
impossible to implement it for merge unless we make git commit execute
git stash pop (yuck!).

If you want, you can implement a rebase.autostash, but that is not my
itch.  Considering the gymnastics the implementation would have to do,
I'd be against a merge.autostash.  So, again: what is your gripe
against my pull.autostash implementation, apart from the fact that git
status doesn't show stash information? (I _might_ decide to fix this)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-15 Thread Junio C Hamano 
Ramkumar Ramachandra artag...@gmail.com writes:

 Junio C Hamano wrote:
 In the end, simple cases (the canonical example being Linus keeping
 a local change to Name = Unicycling Gorilla in his Makefile while
 running dozens of git pull every day) would not be helped with
 this feature very much because there is rarely overlap, while in a
 complex case the user really needs help to save away his WIP, the
 user is forced to resolve the conflict immediately upon the failed
 stash pop at the end of git pull.  If the conflict turns out to
 be something the user would not want to resolve at that moment, the
 user needs to know the way out, something like:

 git reset --hard;# go back to the result of pull
 git checkout -b wip ORIG_HEAD   ;# start from before the pull
 git stash pop   ;# recover his wip
 ... perhaps work a bit more ...
 git commit -a -m wip;# save it away for safety
 git checkout -  ;# result of pull

 so that he can come back tomorrow, check out wip branch, and
 decide what to do.

 What does this have to do with pull at all?

Exactly.

By tempting the user to use autostash first, you are forcing the
user to say reset --hard (the first one), ORIG_HEAD (to start
from the pre-pull state), stash pop (to recover the autostashed
WIP), to a user who got conflict during stash pop after the pull
integrated the committed work with the remote side.

If the user did this instead:

... Let's save my large WIP away in a more permanent place first
git checkout -b wip
... perhaps work a bit more ...
git commit -a -m wip
git checkout -
... and then ...
git pull

the user wouldn't have had to do those extra steps.

Another thing to consider is that even with the current system, many
users do not have a clear idea on what the state of the working tree
is when a pull fails (there are largely two classes). Either the
merge failed without damaging the WIP before the pull at all, or
there wasn't any interaction between the WIP and the change pull
wanted to bring in and only the paths with conflicts need to be
resolved and added (i.e. commit -a is a no-no). This auto-pop
adds a third failure mode to pull. It is a non-issue for Git-heads
like us (we do read the messages from the programs), but not all
users are like us.

 The --autosquash option (or not adding either the configuration or
 the option) would encourage the user to think about the nature of
 WIP he has in the working tree before running git pull.

 I'm lost.  What does git rebase --autosquash (or rebase.autosquash)
 have anything to do with git pull --autostash?

Sorry, the typo meant --autostash.

 Is it a
 too complex beast that may interfere with what others are doing, or
 is it a trivial change that he can stash away and pop it back?  If
 the former, he will be far better off doing checkout -b, keep
 working until the local change gets into somewhat a better shape and
 commit, before pulling into the original branch.

 A pull-merge person having complex worktree changes should not execute
 git pull blindly in the first place.   That's what git fetch is for:

Huh?  You are not making sense.

I would understand it if it were That's what git commit is for,
though.

 But I'm a pull-rebase guy, and I always want pull.autostash.

That is why I said the equation is different for pull-rebase, where
rebase insists that you start from a squeaky clean working tree,
so you can check the condition early before git pull even starts.
While --autostash will not improve the life for pull-merge people
(and pull.autostash will actively hurt them), pull.autostash does
help pull-rebase people work around the limitation of rebase.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-15 Thread Junio C Hamano 
Matthieu Moy matthieu@grenoble-inp.fr writes:

 I think you're taking it the wrong way. You seem to insist that
 autostash should be a pull feature. I think it should be a feature of
 merge and rebase, and the convenience script git pull should expose
 them to the user.

 I see no reason to prevent users running fetch and then merge or rebase
 to use the autostash feature.

I agree that is a good way to look at the problem and would lead to
a much better design of the division of labor among these three
moving parts.

 It's not about fixing the existing rebase. It's about implementing
 autostash the right way.

 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.

Your rebase can do the autostash right way idea in the other
message is a good one, I think.  The rebase proper will sequencially
either apply patches (if the user is using the format-patch | am
variant) or cherry-pick commits (rebase -m).  Conceptually we can
view the WIP in the working tree as just another commit at the tip
of the original history to be rebased (modulo that it should not be
left as a commit in the resulting history, and we may need to
differentiate what was in the index and what was only in the working
tree).  Ignoring the conflicts at stash pop time issue for now, a
rough outline may look like:

git-rebase --autostash

git commit --allow-empty -m 'index part'
git commit --allow-empty -a -m 'working tree part'
git rebase --onto $UPSTREAM
git reset HEAD^
git reset --soft HEAD^

The first reset is to match the index to what was stashed with
the first commit ('index part') while keeping the working tree
changes the original WIP had ('working tree part'), and the last
reset --soft is to move the HEAD back to the tip of the originally
committed history.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-15 Thread Matthieu Moy 
Ramkumar Ramachandra artag...@gmail.com writes:

 Matthieu Moy wrote:
 AFAICT, git merge --abort is an alias for git reset --merge

 Yes, that is correct.

 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.

 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.

 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 .
 * branchbranch - 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.

 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/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-15 Thread Matthieu Moy 
Ramkumar Ramachandra artag...@gmail.com writes:

 Matthieu Moy wrote:
 I disagree. A configuration option is something you set once, and then
 forget about. A command, or a command-line option, is something you
 explicitely add when you need it.

 You're making it out to be a much bigger difference than it actually
 is.  Users will simply alias pull to 'pull --autostash' (a lot of them
 already alias it to pull --ff-only, and I'm going to fix this soon).

No, they don't.  Git forbids redefining commands with aliases. They may
have an alias like git pullauto or so, but not git pull.

 If your criticism were that git status doesn't show stash state, I
 agree with you.

There's not much we can do about it now, as Git cannot guess whether a
stash is to be re-applied later or just kept in case. My main use of
git stash is I want a reset --hard, but stash is safer, I wouldn't
want status to remind me when I have a stash because it is almost
always the case.

Showing the autostash status in git status would make sense OTOH,
but I agree that it's another topic.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-15 Thread Junio C Hamano 
Matthieu Moy matthieu@grenoble-inp.fr writes:

 Ramkumar Ramachandra artag...@gmail.com writes:

 Matthieu Moy wrote:
 AFAICT, git merge --abort is an alias for git reset --merge

 Yes, that is correct.

 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:

Heh, I still remember breaking git merge and got yelled at loudly.

cf. http://thread.gmane.org/gmane.comp.version-control.git/9073

 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.

 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.

Exactly.

 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 said.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-15 Thread Ramkumar Ramachandra 
Junio C Hamano wrote:
 By tempting the user to use autostash first, you are forcing the
 user to say reset --hard (the first one), ORIG_HEAD (to start
 from the pre-pull state), stash pop (to recover the autostashed
 WIP), to a user who got conflict during stash pop after the pull
 integrated the committed work with the remote side.

 If the user did this instead:

 ... Let's save my large WIP away in a more permanent place first
 git checkout -b wip
 ... perhaps work a bit more ...
 git commit -a -m wip
 git checkout -
 ... and then ...
 git pull

 the user wouldn't have had to do those extra steps.

Hm, yes.  For a pull-merge guy who opts for pull.autostash, the
penalty for forgetting to commit a big WIP in advance is too high.  It
probably makes sense to restrict the autostash feature to kick in only
in the rebase codepath: it might be a good idea to implement
rebase.autostash for reduced case of non-interactive rebases instead.
I'm only slightly iffy about --onto, but it's not a big concern.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-15 Thread Ramkumar Ramachandra 
Junio C Hamano wrote:
 git-rebase --autostash

 git commit --allow-empty -m 'index part'
 git commit --allow-empty -a -m 'working tree part'
 git rebase --onto $UPSTREAM
 git reset HEAD^
 git reset --soft HEAD^

Why are you rolling your own stash?  What do you have against git stash?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-15 Thread Ramkumar Ramachandra 
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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-15 Thread Ramkumar Ramachandra 
Matthieu Moy wrote:
 No, they don't.  Git forbids redefining commands with aliases. They may
 have an alias like git pullauto or so, but not git pull.

Ofcourse, but you get the point.  I use p for push, and pu for pull myself.

 There's not much we can do about it now, as Git cannot guess whether a
 stash is to be re-applied later or just kept in case. My main use of
 git stash is I want a reset --hard, but stash is safer, I wouldn't
 want status to remind me when I have a stash because it is almost
 always the case.

 Showing the autostash status in git status would make sense OTOH,
 but I agree that it's another topic.

If the HEAD of the stash contains a stash beginning with the message
pull.autostash: , show it in the status.  End of story.

Anyway, no point arguing about this since we've decided not to pursue
pull.autostash anyway.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-15 Thread John Keeping 
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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-15 Thread Ramkumar Ramachandra 
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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-15 Thread Junio C Hamano 
Ramkumar Ramachandra artag...@gmail.com writes:

 Junio C Hamano wrote:
 git-rebase --autostash

 git commit --allow-empty -m 'index part'
 git commit --allow-empty -a -m 'working tree part'
 git rebase --onto $UPSTREAM
 git reset HEAD^
 git reset --soft HEAD^

 Why are you rolling your own stash?  What do you have against git stash?

Nothing.  It just felt that it does not _have to be_ implemented
with stash, and explaining things in terms of commit and reset would
be simpler.

If rebase -m were to be taught to do this, the natural way to do
so is to 

  (1) Prepare the todo the usual way
  (2) Do those two commits for index and working tree
  (3) Append two insns (exec reset HEAD^ and exec reset --soft
  HEAD^) at the end of the rebase todo file.

Then the usual rebase --continue would just work.  Instead you
could use git stash at step #2 and add exec git stash pop at
step #3.

rebase--am could also be told to generate (on the preparation
side) and notice (on the application side) a pair of patch files at
the end that represent the index state and the working tree state
and apply them without making the WIP part into a commit.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-15 Thread Ramkumar Ramachandra 
Junio C Hamano wrote:
 If rebase -m were to be taught to do this, the natural way to do
 so is to

   (1) Prepare the todo the usual way
   (2) Do those two commits for index and working tree
   (3) Append two insns (exec reset HEAD^ and exec reset --soft
   HEAD^) at the end of the rebase todo file.

Er, no.  I don't want to touch the instruction sheet.  It becomes
especially problematic in -i, when the instruction sheet is
user-editable.

 rebase--am could also be told to generate (on the preparation
 side) and notice (on the application side) a pair of patch files at
 the end that represent the index state and the working tree state
 and apply them without making the WIP part into a commit.

Ugh, no.  I don't want to leak the implementation detail of autostash
into specific rebases.  Why can't I wrap the last statment in
git-rebase.sh in git stash/ git stash pop like I did with git-pull.sh?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-15 Thread John Keeping 
On Mon, Apr 15, 2013 at 11:38:20PM +0530, Ramkumar Ramachandra wrote:
 Junio C Hamano wrote:
  If rebase -m were to be taught to do this, the natural way to do
  so is to
 
(1) Prepare the todo the usual way
(2) Do those two commits for index and working tree
(3) Append two insns (exec reset HEAD^ and exec reset --soft
HEAD^) at the end of the rebase todo file.
 
 Er, no.  I don't want to touch the instruction sheet.  It becomes
 especially problematic in -i, when the instruction sheet is
 user-editable.
 
  rebase--am could also be told to generate (on the preparation
  side) and notice (on the application side) a pair of patch files at
  the end that represent the index state and the working tree state
  and apply them without making the WIP part into a commit.
 
 Ugh, no.  I don't want to leak the implementation detail of autostash
 into specific rebases.  Why can't I wrap the last statment in
 git-rebase.sh in git stash/ git stash pop like I did with git-pull.sh?

How does that work with the following:

- run_specific_rebase fails, so the user needs to fix it up and then run
  git rebase --continue.  We don't want to pop the stash in this case.

- the user runs git rebase --continue with staged changes, knowing the
  git-rebase will commit those.  We don't want to create a stash in this
  case since it will remove the changes the user wants to commit.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-15 Thread Matthieu Moy 
Ramkumar Ramachandra artag...@gmail.com writes:

 Junio C Hamano wrote:
 If rebase -m were to be taught to do this, the natural way to do
 so is to

   (1) Prepare the todo the usual way
   (2) Do those two commits for index and working tree
   (3) Append two insns (exec reset HEAD^ and exec reset --soft
   HEAD^) at the end of the rebase todo file.

 Er, no.  I don't want to touch the instruction sheet.  It becomes
 especially problematic in -i, when the instruction sheet is
 user-editable.

I do not find this problematic. It shows the user what's going on. It
may be a good idea to append the last instructions after launching the
editor if we want to partially hide it (but it's still going to be
visible with rebase --edit-todo)

 rebase--am could also be told to generate (on the preparation
 side) and notice (on the application side) a pair of patch files at
 the end that represent the index state and the working tree state
 and apply them without making the WIP part into a commit.

 Ugh, no.  I don't want to leak the implementation detail of autostash
 into specific rebases.  Why can't I wrap the last statment in
 git-rebase.sh in git stash/ git stash pop like I did with git-pull.sh?

Because git rebase needs multiple runs in case of conflicts. You have
to store the information somewhere in the filesystem, not in a variable.
What you need to store is whether you need to unstash, and where you are
in the process (in Junio's version, you may be doing the actual rebase,
or fixing conflicts in index state application, or fixing conflicts in
tree state application, or done). Storing what you have to do and where
you are in the process really sounds like a job for the instruction
sheet, no?

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-15 Thread Jonathan Nieder 
Matthieu Moy wrote:
 Junio C Hamano gits...@pobox.com writes:

 Yes, that is why I said for pull-merge, --authsquash is neutral-to-better
 and pull.autosquash is harmful.

 To speak from my experience: I find myself typing git stash  git pull
  git stash pop relatively often.

To that end, maybe git pull --stash would be an easier to type option
name than git pull --autostash.

That would also avoid the question of What is automatically going to
happen here?  Is it going to stash only sometimes?  Is it going to
automatically stash what I pull?.

I'm not a big stash user (except git stash --keep-index to prepare
to test changes in the index), so I can't say much more about the
details.

Thanks,
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-15 Thread Jonathan Nieder 
Ramkumar Ramachandra wrote:
 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.

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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-15 Thread Ramkumar Ramachandra 
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).
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-15 Thread Jonathan Nieder 
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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-14 Thread Junio C Hamano 
Ramkumar Ramachandra artag...@gmail.com writes:

 Currently, executing a 'git pull' on a dirty worktree gives the
 following annoying message:

s/annoying//;

In general, avoid forcing your value judgement to readers when you
do not have to.  Instead, you can just highlight the source of your
annoyance and let them naturally feel the annoyance themselves,
perhaps like this:

When the user runs git pull with local changes in the
working tree that interfere with the merge (or any change if
the user is using --rebase), the user is told that the merge
or rebase cannot be done _after_ the command spends time to
download necessary objects.

The user can then decide not to perform the pull and finish
what he was in the middle of doing (e.g. by git checkout -b
topic and then finishing the work), before coming back to
the branch and running git pull again, or git stash the
work in progress, run git pull and then git stash pop.

Introduce a new flag git pull --autostash to perform the
latter automatically. Also add pull.autostash configuration
to do so without giving the command line flag, whose effect
can be overriden by the --no-autostash option.

While I understand where this wish comes from, I am not sure if this
is generally a good idea, or if we are solving a wrong problem.

 Saved working directory and index state WIP on master: 8ea73ed baz
 HEAD is now at 8ea73ed baz

 ... # The merge/rebase process

I think there is one step missing after The merge/rebase process
above, which is

  ... # The 'stash pop' process

 Dropped refs/stash@{0} (83f47fbfb07a741817633f9191dc4a1530f79249)

If the pull-merge were something that would induce the annoyance
that triggered this topic, by definition, the local change overlaps
with the merge, and this internal stash pop will touch the paths
the merge touched and it is likely not result in Dropped but leave
further conflicts to be resolved.

In the end, simple cases (the canonical example being Linus keeping
a local change to Name = Unicycling Gorilla in his Makefile while
running dozens of git pull every day) would not be helped with
this feature very much because there is rarely overlap, while in a
complex case the user really needs help to save away his WIP, the
user is forced to resolve the conflict immediately upon the failed
stash pop at the end of git pull.  If the conflict turns out to
be something the user would not want to resolve at that moment, the
user needs to know the way out, something like:

git reset --hard;# go back to the result of pull
git checkout -b wip ORIG_HEAD   ;# start from before the pull
git stash pop   ;# recover his wip
... perhaps work a bit more ...
git commit -a -m wip;# save it away for safety
git checkout -  ;# result of pull

so that he can come back tomorrow, check out wip branch, and
decide what to do.


The --autosquash option (or not adding either the configuration or
the option) would encourage the user to think about the nature of
WIP he has in the working tree before running git pull.  Is it a
too complex beast that may interfere with what others are doing, or
is it a trivial change that he can stash away and pop it back?  If
the former, he will be far better off doing checkout -b, keep
working until the local change gets into somewhat a better shape and
commit, before pulling into the original branch.  If the latter,
he is better off doing either

 - git pull, after finding it conflicts, run git stash, git
   merge FETCH_HEAD and git stash pop; or

 - git pull --autostash.

And not having --autostash would mean he would do the former, and
will do stash only when it matters.  --autostash is a good to
neutral addition in that sense.  But I suspect that pull.autostash
configuration is not a good addition because it encourages a bad,
pain-inducing workflow.  In simple cases it may not hurt, but when
local changes are complex, it would actively hurt than not having it,
and the configuration robs the incentive to choose.

The equation is somewhat different for pull-rebase, as rebase
insists you to start from a clean working tree, so download and
then stop annoyance feels bigger.  I have a suspicion that
loosening that may be a more productive fix to the real problem.

 +stash_required () {
 + ! (git diff-files --quiet --ignore-submodules 
 + git diff-index --quiet --cached HEAD --ignore-submodules)
 +}

require_clean_work_tree refreshes the index before running
diff-files to make this check safe, but you do not do that here.  Do
you know the index has been refreshed when this is called?

Even though this is mere two-line logic, the duplication of the
logic look a bit wasteful.  Is it too hard to teach dry-run, quiet
mode to require_clean_work_tree helper?  Then the auto-stash
codepath can

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

2013-04-13 Thread Ramkumar Ramachandra 
Currently, executing a 'git pull' on a dirty worktree gives the
following annoying message:

# User doesn't notice dirty worktree
$ git pull

... # fetch operation

error: Your local changes to the following files would be overwritten by 
merge:
quux
Please, commit your changes or stash them before you can merge.
Aborting

At which point the user will stash her changes, re-execute git pull,
and pop the stash.  This process can easily be automated out for a
smooth:

$ git pull

... # fetch operation

Saved working directory and index state WIP on master: 8ea73ed baz
HEAD is now at 8ea73ed baz

... # The merge/rebase process

Dropped refs/stash@{0} (83f47fbfb07a741817633f9191dc4a1530f79249)

If there is a conflict during the merge/rebase process, the user has
to pop the stash by hand after committing the conflict resolution:

$ git pull

... # fetch operation

Saved working directory and index state WIP on master: 8ea73ed baz
HEAD is now at 8ea73ed baz

... # The merge/rebase process

Automatic merge failed; fix conflicts and then commit the result.
Please run 'git stash pop' after commiting the conflict resolution.

Introduce a new configuration variable pull.autostash that does
exactly this.

Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 Documentation/config.txt   |  5 
 Documentation/git-pull.txt |  7 ++
 git-pull.sh| 33 ++--
 t/t5521-pull-options.sh| 63 ++
 4 files changed, 106 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 3d750e0..6c5cd8e 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1804,6 +1804,11 @@ pull.rebase::
 it unless you understand the implications (see linkgit:git-rebase[1]
 for details).
 
+pull.autostash::
+   When true, automatically stash all changes before attempting
+   to merge/rebase, and pop the stash after a successful
+   merge/rebase.
+
 pull.octopus::
The default merge strategy to use when pulling multiple branches
at once.
diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index 24ab07a..1c5384b 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -94,6 +94,13 @@ must be given before the options meant for 'git fetch'.
has to be called afterwards to bring the work tree up to date with the
merge result.
 
+--[no-]autostash::
+   When turned on, automatically stash all changes before
+   attempting to merge/rebase, and pop the stash after a
+   successful merge/rebase.  Useful for people who want to pull
+   with a dirty worktree.  This option can also be set via the
+   `pull.autostash` configuration variable.
+
 Options related to merging
 ~~
 
diff --git a/git-pull.sh b/git-pull.sh
index 5fe69fa..4edc06a 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -48,6 +48,7 @@ if test -z $rebase
 then
rebase=$(git config --bool pull.rebase)
 fi
+autostash=$(git config --bool pull.autostash || echo false)
 dry_run=
 while :
 do
@@ -116,6 +117,12 @@ do
--no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase)
rebase=false
;;
+   --autostash)
+   autostash=true
+   ;;
+   --no-autostash)
+   autostash=false
+   ;;
--recurse-submodules)
recurse_submodules=--recurse-submodules
;;
@@ -202,7 +209,8 @@ test true = $rebase  {
then
die $(gettext updating an unborn branch with changes 
added to the index)
fi
-   else
+   elif test $autostash = false
+   then
require_clean_work_tree pull with rebase Please commit or 
stash them.
fi
oldremoteref= 
@@ -219,6 +227,12 @@ test true = $rebase  {
fi
done
 }
+
+stash_required () {
+   ! (git diff-files --quiet --ignore-submodules 
+   git diff-index --quiet --cached HEAD --ignore-submodules)
+}
+
 orig_head=$(git rev-parse -q --verify HEAD)
 git fetch $verbosity $progress $dry_run $recurse_submodules --update-head-ok 
$@ || exit 1
 test -z $dry_run || exit 0
@@ -294,4 +308,19 @@ true)
eval=$eval \\$merge_name\ HEAD $merge_head
;;
 esac
-eval exec $eval
+
+if test $autostash = true  stash_required
+then
+   git stash || die $(gettext Cannot autostash) 
+   require_clean_work_tree pull Please commit or stash them. 
+   if eval $eval
+   then
+   git stash pop || die $(gettext Cannot pop stash)
+   else
+   exit_code=$?
+   echo Please run 'git stash pop' after commiting the conflict 
resolution.
+   exit $exit_code
+   fi
+else
+   eval exec $eval
+fi
diff --git a/t/t5521-pull-options.sh