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

Ramkumar Ramachandra <artagnon@gmail.com> writes:

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.

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

      ... # The 'stash pop' process

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.

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 simply be:

	(require_clean_work_tree --quiet) || git stash

no?

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

Given that many people do not read messages carefully, it would be a better idea to list them in the order the user carries them out, perhaps

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

or something.