Ramkumar Ramachandra <artagnon@xxxxxxxxx> 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 simply be:
(require_clean_work_tree --quiet) || git stash
no?
> +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")"
This "cannot pop stash" is really where the user gets in the deep
yoghurt and needs help.
> + else
> + exit_code=$?
> + echo "Please run 'git stash pop' after commiting the conflict resolution."
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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html