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

From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Jun 14, 2013, 06:27 UTC
Message-ID: <CALkWK0=NAiGDVWbwHXMmEffPF9wKXd23BdwOntfdvNCpVe8fiA@mail.gmail.com>
In-Reply-To: <7vd2rpu3kf.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:

Show 6 quoted lines

>> What is wrong with git describe?  Is this cheaper, or am I missing something?
>
> I think what you are missing is that the "detached from" is not
> about your current HEAD after you flipped it around with many resets
> and commits.  It is about what tag or what specific commit you
> detached your HEAD at originally.

No, it is about what tag of specific commit you detached your HEAD from, *without using checkout*. If you used checkout, you'd get the "detached at" message, and I haven't changed that.

> "You can ask the same to describe" is wrong and is not a valid
> argument.  Once you replace the HEAD^ with a distant commit
> (e.g. HEAD~400) in the third step in the example, "describe" will
> not talk about v1.8.2 at all.

You're missing the point: how do I, as the end-user, detach my HEAD *without using checkout*? The hypothetical example you have given is:

  $ git checkout HEAD^
  $ git update-ref HEAD $(git rev-parse HEAD~400)

Which end-user executes that?

> Your argument can be that it is not a useful piece of information,
> and as you can probably guess from my "I wouldn't be surprised"
> above, I am not sure how useful it would be and in what situation
> [*1*].

Precisely. It is a poorly thought-out feature that locks things up too tightly, and makes life hell for contributors. It must therefore be removed.

> But the original commit thought it was necessary and that was done
> for a reason; we need to be careful here.  With a good justification
> why it is not necessary (or misleading to the user), I do not think
> we cannot change it.

We cannot reverse-engineer intents. All we can do is look at the evidence in front of us. Read the commit message, and look at the newly added test. There is absolutely no indication about why this "detached from" is useful, and where.

Show 14 quoted lines

>         $ git rebase master
>         ... replays some but stops
>         $ git status
>
> currently uses that "HEAD detached from" codepath, but I think that
> is a mistake.  We could not just tell the HEAD is detached, but the
> reason _why_ the HEAD is detached (i.e. we are in the middle of a
> rebase).  The prompt script can do it, "status" should be able to do
> the same, and do a lot more sensible thing than unconditionally
> showing that "HEAD detached from" and then say "You are currently
> rebasing" on a separate line.  Most likely we do not want to even
> say "Not currently on any branch" but just say "You are currently
> rebasing branch X on top of Y" (and perhaps "N commits remaining to
> be replayed").

That information is available in .git/{rebase-apply,rebase-merge}, and your suggestion pertains to improving show_rebase_in_progress(). The first line is about the state of HEAD, and is completely orthogonal to the issue at hand.

  artagnon|rebase-rev-failure|REBASE-i 2/3:~/src/git$ git status
  # HEAD detached from a7e9fd4
  # You are currently editing a commit while rebasing branch
'rebase-rev-failure' on '9926f66'.
  #
  nothing to commit, working directory clean

That first line about "HEAD detached from ..." is completely useless. And yes, my prompt is more useful. No prizes for guessing how often I use the long form of git status.

Show 5 quoted lines

> *1* One thing I could think of is to start sightseeing or (more
>     realistically) manually bisecting at a given release point,
>     reset the detached HEAD around several times, and then want to
>     be reminded where the session started from.  I do not think it
>     is particularly a very good example, though.

The example you have given now is:

  $ git checkout @^
  # or whatever bisect command to detach HEAD
  $ git reset @~3
  ...
  $ git reset @^
  ...
  $ git reset @~5
  ....
  # when was HEAD originally detached?

Yes, it is a contrived example where this feature arguably has some utility. Is it worth putting the information in the status for such an esoteric example? If one really wants this information, they can open up the reflog and grep for "checkout: ".