Re: [PATCH v2 0/6] Fix checkout-dash to work with rebase

From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Jun 18, 2013, 09:30 UTC
Message-ID: <CALkWK0nGeyna_vxnN0RMULRj3sEj3v=Q61cXrHDYDHYv-nZ4sg@mail.gmail.com>
In-Reply-To: <7vk3lsei4i.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:

Show 6 quoted lines

> I actually tried to reorder the patches and they seem to work OK as
> expected.  And I think it makes sense to order them the way I've
> been suggesting, so I'll tentatively queue the result of reordering
> on 'rr/rebase-checkout-reflog' and push it out as a part of 'pu'.
>
> Please check to see if I introduced a new bug while doing so.

Thanks for the reorder and commit message tweaks. I'm working on the series you put up on `pu` now.

Show 23 quoted lines

> For example, the one in "git reabse" does this:
>
>     GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $onto_name"
>     git checkout -q "$onto^0" || die "could not detach HEAD"
>     git update-ref ORIG_HEAD $orig_head
>     ...
>     run_specific_rebase
>
> But the specific rebase, e.g. git-rebase--interactive, does this:
>
>         case $head_name in
>         refs/*)
>                 message="$GIT_REFLOG_ACTION: $head_name onto $onto" &&
>                 git update-ref -m "$message" $head_name $newhead $orig_head &&
>                 git symbolic-ref \
>                   -m "$GIT_REFLOG_ACTION: returning to $head_name" \
>                   HEAD $head_name
>                 ;;
>         esac && {
>
> I think the message you added to "git reabse" is only meant for that
> specific "checkout $onto", but it is set globally.  Wouldn't it
> affect later use, which expected it to be "rebase" and nothing else?

Both rebase.sh and rebase--interactive.sh set a sane GIT_REFLOG_ACTION right on top (using set_reflog_action), so no worries. I'll just double-check to make sure that no bogus/ incorrect messages are printed.

> Perhaps something like this on top of the entire series may be
> sufficient (which will be queued as "SQUASH???" at the tip).

I think this takes the wrong approach to the problem. In my opinion, the correct approach is to actually overshadow die() with a function that clears GIT_REFLOG_ACTION before calling die().

>         git grep -C2 'git checkout' -- git-rebase\*.sh

Ugh.  I'll check all the codepaths thoroughly before submitting a re-roll.

Thanks.