On Thu, Aug 3, 2017 at 1:37 PM, Junio C Hamano <[email protected]> wrote:
> Brandon Williams <[email protected]> writes:
>
>> The 'submodule.update' config was historically used and respected by the
>> 'submodule update' command because update handled a variety of different
>> ways it updated a submodule. As we begin teaching other commands about
>> submodules it makes more sense for the different settings of
>> 'submodule.update' to be handled by the individual commands themselves
>> (checkout, rebase, merge, etc) so it shouldn't be respected by the
>> native checkout command.
>
> Soooo... what's the externally observable effect of this change? Is
> it something that can be illustrated in a set of new tests?
The illustration can be as follows
git config submodule.NAME.update none
git checkout -f --recurse-submodules HEAD
git status
# observe dirty submodule, which is
# not what checkout -f promises
> IOW does this commit by itself want to change the behaviour of
> "submodule update" and existing (indirect) users of unpack-trees?
> Or does it want to keep the documented behaviour of "submodule
> update" while correcting unintended triggering in other (indirect)
> users of unpack-trees of the same machinery that is being removed in
> this patch?
"submodule update" is unaffected, only the recently introduced submodule
awareness of checkout/reset/read-tree are changed.
This option is documented as
submodule.<name>.update
The default update procedure for a submodule. This variable is
populated by git submodule init from the gitmodules(5) file. See
description of update command in git-submodule(1).
which doesn't indicate that any other command apart from
"submodule update" should respect it.
>
>> - switch (sub->update_strategy.type) {
>> - case SM_UPDATE_UNSPECIFIED:
>> - case SM_UPDATE_CHECKOUT:
>> - if (submodule_move_head(ce->name, old_id, new_id, flags))
>> - return o->gently ? -1 :
>> - add_rejected_path(o,
>> ERROR_WOULD_LOSE_SUBMODULE, ce->name);
>> - return 0;
>> - case SM_UPDATE_NONE:
>> - return 0;
>> - case SM_UPDATE_REBASE:
>> - case SM_UPDATE_MERGE:
>> - case SM_UPDATE_COMMAND:
>> - default:
>> - warning(_("submodule update strategy not supported for
>> submodule '%s'"), ce->name);
>> - return -1;
>> - }
>> + if (submodule_move_head(ce->name, old_id, new_id, flags))
>> + return o->gently ? -1 :
>> + add_rejected_path(o,
>> ERROR_WOULD_LOSE_SUBMODULE, ce->name);
>> + return 0;
>
> With this update, we always behave as if update_strategy.type were
> either left unspecified or explicitly set to checkout. Other arms
> in this switch (and the other switch too), especially "none", were
> not expecting a call to submodule_move_head() to be made, but now
> the call is unconditional.
>
Yes. This is because each command (reset/checkout) should provide
one expected behavior. It is not that we can configure reset to omit certain
(tracked) files from being reset?
