Re: [PATCH] Proof-of-concept patch to remember what the detached HEAD was

["Björn Steinbrink" <B.Steinbrink@gmx.de>]

On 2009.10.18 15:47:13 -0700, Junio C Hamano wrote:
> Björn Steinbrink <B.Steinbrink@gmx.de> writes:
> > On 2009.10.17 02:04:02 -0700, Junio C Hamano wrote:
> >  1) No uncommitted changes => No problem
> >  2) Uncommitted changes merge cleanly => No problem
> >  3) Uncommitted changes causes conflicts =>
> >    - User can resolve
> >    - User can start over (git update --retry)
> >    - User can give up (git update --abort)
> 
> By "--abort", if you meant to discard the local change, that is only
> suitable for people who would say "what I was doing was minor anyway, and
> I'll redo them based on the updated upstream", and may not be so useful, I
> think.  The user may want to pretend that he did not even start "update"
> (i.e. not pulled while he was still working on something) at this point,
> and if you meant by "give up" (aka --abort) to "reset --hard @{1} &&
> unstash", I think it makes quite a lot of sense.  Then the user has an
> option to fork a topic at that point:
> 
>     git update --abort
>     git checkout -b topic
>     work on more with committing
>     git checkout master
>     git update

Yes, I meant the latter. The former would just be "git reset --hard" and
is pointless to be rewrapped in "git update --abort". Maybe "abort" is
the wrong word there? I'm not a native speaker and basically took that
from "git rebase", which returns you to your (unchanged) branch head,
i.e. the state you were in before you started the rebase.

> But then this starts to look more like an enhanced failure recovery mode
> for "git pull" command.

Yeah, I notice that, also while working on my "proof-of-concept"
implementation. Currently, I simply suggest "git pull" to the user when
the update is not a fast-forward one.

I'm probably influenced by a latent hatred for "git pull"... Explaining
that it is fetch+merge/rebase and uses defaults from the config which
are automagically setup is probably something my fingers can do on
their own by now. Confusion about "git pull" probably beats
misunderstandings about HEAD.

If it wasn't so inconvenient for people that actually commit, I'd even
dare to suggest:

git update
 - FF update only, using branch.<name>.{remote,merge}
 - suggests "git update --merge" or "git update --rebase" if non-ff

git update --merge
 - Does a merge

git update --rebase
 - Does a rebase

And "git pull" would stop using branch.<name>.{remote,merge} and require
command line arguments.

That would at least raise awareness that "git update --merge" is doing a
merge, unlike "git pull", which many new users simply treat as magic,
not realizing what actually happens (and thus they create butt-ugly
criss-cross merge histories). Of course, I can't tell whether being
aware that a merge happens actually makes the user realize that they
shouldn't update/pull/merge 50 times a day.

But passing --merge all the time seems just too inconvenient. And having
a config option to make --merge or --rebase the default would probably
end up with --merge as the "default default", ultimately turning "git
update" into "git pull".

Anyway, I'm probably getting quite far off the track here.

> In addition, I think that you would internally implement the "save" step
> with "stash" (which would be a sane thing to do), but then you would need
> to worry about the case where the user was in the middle of a merge (or
> "revert", "cherry-pick", "checkout -m") that did not complete.  "git pull"
> fails upfront, says why and tells users what to do.  "git update" should
> do the same.

Yup, got the same check in my PoC

> > I do see problems with a "stash around merge" thing ("stash" around
> > rebase seems easier, as that could just create a commit and reset later,
> > but I'm not exactly sure that such smartness is a good idea). As soon as
> > the merge has conflicts, you need to know that you have to unstash after
> > committing the merge, but what I have in mind is fast-forward only (or
> > possibly reset, when upstream was rewritten).  Primarily for users that
> > don't commit at all, but just look at things [*1*].
> 
> Ok.  If you have a clean way to guarantee that "update" users won't
> commit, I think the above would sort of make sense and my earlier worries
> about (1) a user who wish he did not fetch and (2) a user who was doing
> something more complex and had conflicts already would go away.

Currently it's just:
test -z "$(git rev-list -1 $upstream..)"

As a "is a fast-forward" check, suggesting the user to use "git pull"
instead of "git update", if it is not. Maybe I should use "git
merge-base" there instead? Is that better? Not sure whether history
simplication might break the rev-list based test...

> If the sole target audience is "minor changes only, never committing"
> people, then I would even rescind my earlier suggestion on --abort; it
> should mean "remove the minor changes and get pristine copy of the
> upstream---the user will redo the minor changes to adjust to the updated
> upstream from scratch", to keep the end user experience simpler and
> clearer.

Hmhm... If the user can't resolve the conflict, but still keep those
changes, he might want to ask someone else for help. And then he might
want to present his changes to that other person, so I think allowing
the user to go back to the old commit with his changes on top is better.
Maybe "git update --drop" could do the "drop the user's changes", if the
user wants to do so. No support for going back is what's bad about "svn
update" (and, I guess, "git checkout --merge").

> I am undecided if it is a good thing to divide the userbase into two
> classes, "update" people and "work-commit-fetch-merge-resolve" people.

I expect that there are already two userbases. Developers in one
userbase and users that just want the latest code in the other. And
these users might have some uncommitted stuff (either self-made or
possibly found somewhere, or maybe a bit of both).

I just wonder what's easier/better: to cater to each one or trying to
unify them by trying to educate the one that actually doesn't care.

Björn