Re: [PATCHv8 4/4] status: better advices when splitting a commit (during rebase -i)

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 8, 2012, 15:05 UTC
Message-ID: <7vzk8ddeua.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20120608112945.Horde.I4abIHwdC4BP0cYJzywVAeA@webmail.minatec.grenoble-inp.fr>

konglu@minatec.inpg.fr writes:

Show 12 quoted lines

>> Reading from git_path("HEAD") looked funny, as you may end up
>> reading the "ref: refs/heads/master".  Of course that would not
>> compare equal to what you would read from "rebase-merge/amend", and
>> that may be fine for the purpose of your test, but it still looks
>> somewhat funny.  As modern rebase is done on a detached HEAD,
>> perhaps it is a good idea to check if the HEAD is detached and
>> return false from this function if that is not the case.  I dunno.
>
> On second thoughts, I do not think that checking if HEAD is detached
> or not is needed, as the part of the code that includes reading can
> only be called during a rebase interactive, in which case the HEAD
> can only be detached.

"can only be detached" is making a huge assumption.

I'd rather see a code that verifies that the assumption still holds after changes are made to other parts of the system and gracefully degrade its behaviour when the assumption it makes no longer holds.

Besides, making sure that the HEAD is detached when you _think_ you are in the middle of a rebase is a necessary part of catching and reporting a potential mistake like this, no?

	$ git checkout -b throwaway
        $ git rebase --onto HEAD~5 HEAD~2
        ... conflicts ...
        $ git checkout throwaway