Re: [PATCH] recursive submodules: detach HEAD from new state

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 22, 2017, 00:54 UTC
Message-ID: <xmqqr2sryvyi.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20171121223449.GI3429@aiede.mtv.corp.google.com>

Jonathan Nieder <jrnieder@gmail.com> writes:

Show 18 quoted lines

> The thread I am replying to appears to be where the patch comes from
> but I have some memories of more recent discussion that I'm not
> finding.
>
> More context:
> https://public-inbox.org/git/xmqqshd8i3ze.fsf@gitster.mtv.corp.google.com/
> says
>
>  * sb/submodule-recursive-checkout-detach-head (2017-07-28) 2 commits
>   - Documentation/checkout: clarify submodule HEADs to be detached
>   - recursive submodules: detach HEAD from new state
>
>   "git checkout --recursive" may overwrite and rewind the history of
>   the branch that happens to be checked out in submodule
>   repositories, which might not be desirable.  Detach the HEAD but
>   still allow the recursive checkout to succeed in such a case.
>
>   Expecting a reroll.

Sorry, I should have done my usual "cf. <message-id>" to help me recalling the discussion that lead to the marking there.

We kicked it back to 'pu' after the discussion that had <xmqq375ppqma.fsf@gitster.mtv.corp.google.com>, and the "send out a plan" sort-of happened with <20171109001007.11894-1-sbeller@google.com> which seemed to have converged to a conclusion in the subthread with <CAGZ79kZAvMKQUjbqWZkhy39sE5e9k1DmkiA42ywiw2NgY1+Xig@mail.gmail.com> where a preferred way would be to detach and opportunistically reattach to keep the illusion of submodule being on a branch as much as possible (correct me if I am misunderstanding the consensus).

I had a suspicion that such a random re-attachment may lead to an unpredictable behaviour from end-user's point of view that is confusing, but there wasn't a concrete patch to do so, so that was why I was waiting for a reroll so that people can take a look at it and see how well it fares.

The responses in this thread seems to indicate that now we are punting on that re-attachment plan and want to give this "always detach" to the end users, which I think is a lot more predictable thing to do. After such a recursive checkout from the top-level, any work done in the submodule from that state and is referenced from the top-level (recorded presumably with a recursive commit) is not referenced by anything other than the reflog of HEAD in the submodule repository, and I suspect many users are not used to and are comfortable with working on a detached HEAD for extended period of time, so this new behaviour might deserve a stronger warning to help them, but I am OK with the stance "We'll learn as we go---let's merge it as-is and see what happens".

Thanks for prodding. One less topic that is stale but has to be carried around in 'pu' is always a good thing ;-)