Re: Submodule regression in 2.14?

From: Stefan Beller <sbeller@google.com>
Date: Aug 18, 2017, 04:02 UTC
Message-ID: <CAGZ79kZdnJ+bATTxKBhsJnKaJWGqcBu3MOQ9eK7m4j3dJPNbTw@mail.gmail.com>
In-Reply-To: <xmqqd17tei7m.fsf@gitster.mtv.corp.google.com>

On Thu, Aug 17, 2017 at 7:13 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 11 quoted lines

> Stefan Beller <sbeller@google.com> writes:
>
>> Are you saying this might be a design mistake and
>> the .update ought to be respected by all the other
>> commands? For example
>>     git reset --recurse-submodules
>> should ignore the .update= none?
>
> I have been under the impression that that has been the traditional
> desire of what .update ought to mean.  I personally do not have a
> strong opinion---at least not yet.

In this context note v2.14.0-rc1-34-g7463e2ec3 (bw/submodule-config-cleanup~7, "unpack-trees: don't respect submodule.update") that is going opposite of your impression.

Show 8 quoted lines

>> When designing these new recursive submodule functionality
>> outside the "submodule" command, I'd want submodules
>> to behave as much as possible like trees.
>
> I think that is sensible as long as the user does not explicitly say
> "this and that submodule behave differently" by giving configuration
> variables.  Perhaps .update is one of those that should countermand
> the default behaviour of "--recurse-submodules"?

Maybe, I'll think about it. However there is no such equivalent for trees (and AFAICT never came up) to treat a specific directory other than the rest in worktree operations.

The problem with the issue in question is however: git-pull is a combination of two other high level commands (fetch/merge), the fetch component already had a recursive behavior, and that commit in question added a bit for the merge component, so the UX is hard to get right for both of them:

    git pull --recurse=fetch-only
    git pull --recurse=merge-respects-update-strategy

is what I'd want to avoid.

So maybe we can just respect the update strategy before starting the local part.

Stefan