Re: [PATCH 5/6] status: do not depend on flaky reflog messages

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 14, 2013, 22:16 UTC
Message-ID: <7vfvwkmj7x.fsf@alter.siamese.dyndns.org>
In-Reply-To: <CALkWK0==Phbe-9QaOa3jkYMEvxb6F3kypRkk9RbzrLco-HkHKw@mail.gmail.com>

Ramkumar Ramachandra <artagnon@gmail.com> writes:

Show 10 quoted lines

> Junio C Hamano wrote:
>> If you first update "git checkout" so that it will pay attention to
>> a custom reflog-action exported by Porcelain scripts that may want
>> to internally use it to flip branches (and without a custom one, it
>> will still record "checkout: moving from A to B"), without exporting
>> custom reflog-action from "rebase" and other Porcelain scripts, that
>> would not affect any externally visible behaviour.
>
> rebase already sets a custom reflog-action; in fact,
> 'set_reflog_action rebase' is executed after it sources sh-setup.

Ah, OK, that is what I missed, so updating the "checkout" itself will affect the test Duy's commit updated.

But that does not change anything. The thing is, I fully agree with this:

> the "detached HEAD from" message no longer has anything to do with
> the rebase itself; it is completely _random_ and _incidental_.

because at the end of this series, we will not be recording the internal "checkout" as a checkout event in the reflog. What the top-line of "git status" says does not matter.

If I were doing this series, the first commit before doing anything else would be to strip that line from the expected output from "git status" for tests Duy's commit changed, with the justification perhaps like this:

    b397ea4863a1 (status: show more info than "currently not on any
    branch", 2013-03-13) wanted to make sure that after a checkout
    to detach HEAD, the user can see where the HEAD was originally
    detached from.

    The last test added by that commit to t7512 shows one example,
    immediately after HEAD is detached.  Add another test to show
    "detached HEAD from" form that should be shown when the user
    further resetted to another commit.

    The majority of tests in t7512 that commit updated, however, are
    looking at the status output while in "rebase", and the exact
    content of that first line relies on the fact that "git rebase"
    used "git checkout" to first detach HEAD before doing anything
    else, and allowed that to be recorded in the reflog just like a
    user initiated "checkout".  As far as the user is concerned,
    however, this operation is not a "checkout" that the user would
    want to count when later issuing "git checkout @{-1}", etc.

    We are going to fix "git rebase" not to let reflog record these
    internal events as "checkout"; these first lines will start
    saying different (and useless) things.  Improving them to give
    a message more appropriate during "rebase" (e.g. "# Rebasing
    branch X on top of Y", instead of "# Not currently on any
    branch" or "# HEAD detached from Z") is left to a separate
    future series, but until that happens, stop checking what
    appears on that first line.

and would keep the last one that shows "detached HEAD at". That is what that commit wantd to achieve. This first commit in the series would be a good place to add the missing one after that to show "detached HEAD from", doing "reset HEAD^". In other words, we can make this tirst step as a pure clean-up.

If stripping the first line to compare is too much, then I think it is an acceptable compromise to update the "# HEAD detached from $ONTO" Duy's patch changed from "# Not currently on ..." to whatever your updated "rebase" happens to produce, with the same (but weaker) justification as the last sentence in my "perhaps like this" above.