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

From: Stefan Beller <sbeller@google.com>
Date: Jul 24, 2017, 19:07 UTC
Message-ID: <CAGZ79kaYzHzSdDZuMpM6UFXLon0SME-nC=9_09PLhLEADfuscw@mail.gmail.com>
In-Reply-To: <20170724180312.GE13924@aiede.mtv.corp.google.com>

On Mon, Jul 24, 2017 at 11:03 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:

Show 16 quoted lines

> Hi,
>
> Stefan Beller wrote:
>
>> When a submodule is on a branch and in its superproject you run a
>> recursive checkout, the branch of the submodule is updated to what the
>> superproject checks out. This is very unexpected in the current model of
>> Git as e.g. 'submodule update' always detaches the submodule HEAD.
>>
>> Despite having plans to have submodule HEADS not detached in the future,
>> the current behavior is really bad as it doesn't match user expectations
>> and it is not checking for loss of commits (only to be recovered via the
>> reflog).
>
> I think the corrected behavior doesn't match user expectations,
> either.

Well, what is the user expectation?

>
> Could this patch include some documentation to help users know what to
> expect?

Sure, once we figured out what is reasonable.

Show 14 quoted lines

>
>> Detach the HEAD unconditionally in the submodule when updating it.
>>
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> ---
>> This is a resend of [1], which did not receive any attention.
>
> Yikes.  Yes, this bug looks problematic.  Thanks for working on it.
>
>> I improved the commit message laying out the current state of affairs,
>> arguing that any future plan should not weigh in as much as the current
>> possible data loss.
>
> Can you elaborate on what you mean about data loss?

Assume we have a submodule 'sub' inside the superproject 'super', then

    git -C super/sub checkout <my-unrelated-branch>
    git -C super checkout <some-tree-ish>

modifies my-unrelated-branch in the submodule, which is not related to the superproject in any way.

This patch would detach from that branch and have the HEAD contain the desired sha1. To think that further we'd still have potential data loss:

    git -C super/sub checkout <my-unrelated-branch>
    git -C super checkout <some-tree-ish>
    # fine so far as sub is in detached HEAD, but:
     ... hack hack hack ... in 'sub'
    git -C super/sub commit -m "work"
    git -C super checkout <other-tree-ish>
    # subs work is only to be recovered via reflog!

However this matches the current behavior of "submodule update" which also tips, that are not reachable from any ref.

> At first glance
> it would seem to me that detaching HEAD could lead to data loss since
> there isn't a branch to keep track of the user's work.

yes, but that is the same with "submodule update", which is what people may have in mind?

>  Are you saying
> the current behavior of updating whatever branch HEAD is on (which,
> don't get me wrong, is a wrong behavior that needs fixing) bypassed
> the reflog?

No, I am not saying that. I am saying that updating an unrelated branch (which is dragged into the affair just because HEAD points at it) is very subtle thing, as any commits on that branch can be considered safe (it is on a branch, right?) but the detached HEAD is the default unsafe mode we currently have.

Thanks, Stefan