Submodule regression in 2.14?

> Hi,
>
> I think we discovered a regression in Git 2.14.1 today.
> It looks like as if "git submodule update --init --recursive" removes
> the "skip submodules" config.
>
> Consider the following steps:
>
>     git clone https://server/repo.git
>     cd repo
>     git config --local submodule.some/other/repo.update none
>     git submodule update --init --recursive
>     git pull --recurse-submodules
>
> With Git 2.14 the last "git pull" will clone the "some/other/repo"
> submodule. This did not happen with Git 2.13.
>
> Bug or feature? I don't have anymore time for Git today. I am happy to
> provide a proper test case tomorrow, though.

> Any chance the "did not happen with 2.13" was not
> freshly cloned but tested on an existing repo? If so a hot
> candidate for suspicion is a93dcb0a56 (Merge branch
> 'bw/submodule-is-active', 2017-03-30), IMHO, just
> gut feeling, though.

> On 16 Aug 2017, at 20:51, Stefan Beller <sbeller@google.com> wrote:
> 
> On Wed, Aug 16, 2017 at 11:35 AM, Lars Schneider
> <larsxschneider@gmail.com> wrote:
>> Hi,
>> 
>> I think we discovered a regression in Git 2.14.1 today.
>> It looks like as if "git submodule update --init --recursive" removes
>> the "skip submodules" config.
>> 
>> Consider the following steps:
>> 
>>    git clone https://server/repo.git
>>    cd repo
>>    git config --local submodule.some/other/repo.update none
>>    git submodule update --init --recursive
>>    git pull --recurse-submodules
>> 
>> With Git 2.14 the last "git pull" will clone the "some/other/repo"
>> submodule. This did not happen with Git 2.13.
>> 
>> Bug or feature? I don't have anymore time for Git today. I am happy to
>> provide a proper test case tomorrow, though.
> 
> $ git log --oneline v2.13.0..v2.14.1 -- git-submodule.sh
> 532139940c add: warn when adding an embedded repository
> (I am confident this is not the suspect, let's keep searching.
> Not a lot happened in submodule land apparently)
> 
> Looking through all commits v2.13..v2.14 doesn't have me
> suspect any of them.
> 
> Any chance the "did not happen with 2.13" was not
> freshly cloned but tested on an existing repo? If so a hot
> candidate for suspicion is a93dcb0a56 (Merge branch
> 'bw/submodule-is-active', 2017-03-30), IMHO, just
> gut feeling, though.
> 
> Oh, wait.
> $ git log --oneline v2.13.0..v2.14.1 -- builtin/pull.c
> c9c63ee558 Merge branch 'sb/pull-rebase-submodule'
> a6d7eb2c7a pull: optionally rebase submodules (remote submodule changes only)
> could also be a culprit. Do you have pull.rebase set?

>
>> Oh, wait.
>> $ git log --oneline v2.13.0..v2.14.1 -- builtin/pull.c
>> c9c63ee558 Merge branch 'sb/pull-rebase-submodule'
>> a6d7eb2c7a pull: optionally rebase submodules (remote submodule changes only)
>> could also be a culprit. Do you have pull.rebase set?
>
> I bisected the problem today and "a6d7eb2c7a pull: optionally rebase submodules
> (remote submodule changes only)" is indeed the culprit.
>
> The commit seems to break the following test case.
>
> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
> index dcac364c5f..24f9729015 100755
> --- a/t/t7400-submodule-basic.sh
> +++ b/t/t7400-submodule-basic.sh
> @@ -1289,4 +1289,19 @@ test_expect_success 'init properly sets the config' '
>         test_must_fail git -C multisuper_clone config --get submodule.sub1.active
>  '
>
> +test_expect_success 'submodule update and git pull with disabled submodule' '
> +       test_when_finished "rm -rf multisuper_clone" &&
> +       pwd=$(pwd) &&
> +       git clone file://"$pwd"/multisuper multisuper_clone &&
> +       (
> +               cd multisuper_clone &&
> +               git config --local submodule.sub0.update none &&
> +               git submodule update --init --recursive &&
> +               git pull --recurse-submodules &&
> +               git submodule status | cut -c 1,43- >actual
> +       ) &&
> +       ls &&
> +       test_cmp expect multisuper_clone/actual
> +'

> +
>  test_done
>
>
> I am not familiar with the code. Does anyone see the problem
> right away?
>
> Thanks,
> Lars
>
>

> 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?

> 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.

>> 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"?

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

> I am not sure if I follow.  Submodules are not trees and one of the
> reasons people may want to separate things into different modules is
> so that they can treat them differently.  If submodules allow you
> a richer set of operations than a tree that is part of a monolithic
> project, is that necessarily a bad thing?

> Jonathan brought up the following very long term vision:
> Eventually the everyday git commands do not treat submodules
> any special than trees, even the submodules git directory
> may be non existent (everything is absorbed into the superproject);
> so it really feels like a monorepo.

> As long as we are talking about idealized future world (well, at
> least an idea of somebody's "ideal", not necessarily shared by
> everybody), I wonder if there is even any need to have commits in
> submodules in such a world.  To realize such a "monorepo" world, you
> might be better off allowing a gitlink in the superproject to
> directly point at a tree object in a submodule repository (making
> them physically a single repository is an optional implementation
> detail I choose to ignore in this discussion).

> On Fri, Aug 18, 2017 at 11:51:13PM -0700, Junio C Hamano wrote:
>> As long as we are talking about idealized future world (well, at
>> least an idea of somebody's "ideal", not necessarily shared by
>> everybody), I wonder if there is even any need to have commits in
>> submodules in such a world.  To realize such a "monorepo" world, you
>> might be better off allowing a gitlink in the superproject to
>> directly point at a tree object in a submodule repository (making
>> them physically a single repository is an optional implementation
>> detail I choose to ignore in this discussion).
>
> IMO this is one step to far. One main use of submodules are shared
> repositories that are used by many superprojects. The reason you want to
> have commits in the submodule are so that you can push them
> independently and all other users can pick up the changes. You could get
> by by Using the superproject commits for the submodule once you push or
> something but those do not necessarily make sense in the context of the
> submodule.
>
> So I think it is important that there are commits in the submodule so
> its history makes sense independently for others.
>
> Or how would you push out the history in the submodule in your idea?
> Maybe I am missing something? What would be your use case with gitlinks
> pointing to trees?

> On Mon, Aug 21, 2017 at 9:05 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> > On Fri, Aug 18, 2017 at 11:51:13PM -0700, Junio C Hamano wrote:
> >> As long as we are talking about idealized future world (well, at
> >> least an idea of somebody's "ideal", not necessarily shared by
> >> everybody), I wonder if there is even any need to have commits in
> >> submodules in such a world.  To realize such a "monorepo" world, you
> >> might be better off allowing a gitlink in the superproject to
> >> directly point at a tree object in a submodule repository (making
> >> them physically a single repository is an optional implementation
> >> detail I choose to ignore in this discussion).
> >
> > IMO this is one step to far. One main use of submodules are shared
> > repositories that are used by many superprojects. The reason you want to
> > have commits in the submodule are so that you can push them
> > independently and all other users can pick up the changes. You could get
> > by by Using the superproject commits for the submodule once you push or
> > something but those do not necessarily make sense in the context of the
> > submodule.
> >
> > So I think it is important that there are commits in the submodule so
> > its history makes sense independently for others.
> >
> > Or how would you push out the history in the submodule in your idea?
> > Maybe I am missing something? What would be your use case with gitlinks
> > pointing to trees?
> 
> Well there are still commits, but in the superproject the UX feels more
> as if the submodules were special trees.

> So if you want to interact with
> the submodule specifically, you could do things like
> 
>     git add /path/inside/sub
>     # works seamlessly from the superproject tree

>     # You feel the superproject is in the way?
>     git worktree add --for-submodule <path/to/sub> ...
>     # The new submodule worktree puts the
>     # submodule only UX first. so it feels like its own
>     # repository, no need for specific flags.

> On Mon, Aug 21, 2017 at 09:42:54AM -0700, Stefan Beller wrote:
>> On Mon, Aug 21, 2017 at 9:05 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
>> > On Fri, Aug 18, 2017 at 11:51:13PM -0700, Junio C Hamano wrote:
>> >> As long as we are talking about idealized future world (well, at
>> >> least an idea of somebody's "ideal", not necessarily shared by
>> >> everybody), I wonder if there is even any need to have commits in
>> >> submodules in such a world.  To realize such a "monorepo" world, you
>> >> might be better off allowing a gitlink in the superproject to
>> >> directly point at a tree object in a submodule repository (making
>> >> them physically a single repository is an optional implementation
>> >> detail I choose to ignore in this discussion).
>> >
>> > IMO this is one step to far. One main use of submodules are shared
>> > repositories that are used by many superprojects. The reason you want to
>> > have commits in the submodule are so that you can push them
>> > independently and all other users can pick up the changes. You could get
>> > by by Using the superproject commits for the submodule once you push or
>> > something but those do not necessarily make sense in the context of the
>> > submodule.
>> >
>> > So I think it is important that there are commits in the submodule so
>> > its history makes sense independently for others.
>> >
>> > Or how would you push out the history in the submodule in your idea?
>> > Maybe I am missing something? What would be your use case with gitlinks
>> > pointing to trees?
>>
>> Well there are still commits, but in the superproject the UX feels more
>> as if the submodules were special trees.
>
> Ah ok then I misunderstood. So they only feel like trees.
>
>> So if you want to interact with
>> the submodule specifically, you could do things like
>>
>>     git add /path/inside/sub
>>     # works seamlessly from the superproject tree
>
> Would that mean that we need to loosen/keep the requirement loose for a
> name from .gitmodules? I am asking because of my series for on-demand
> fetch of renamed submodules. For the full functionality I would require
> a name.
>
> Would that be in a scenario where the user would then e.g. push the
> submodule into the superproject?
>
> Ah wait I misunderstood again. You mean a file in an existing
> submodule right? Not adding submodule from a repository a user moved
> there?

>
>>     git commit --submodule-commit-only
>>     # When the flag is not give, you may get an editor
>>     # asking for two commit messages, (sub+super)
>
> Or maybe something like
>
>     git commit --submodule path/to/submodule

>
> so the user can specify which submodule she wants. I first wrote it
> without the switch but but that collides with listing files which should
> be added. IMO this shorter option is also more intuitive to understand
> what it does (for this case).
>
>>     git fetch --submodule
>>     # When the flag is not given, we'd fetch superproject and
>>     # on-demand
>
> Yes like above we should add the path to the submodule right?

>
>>     # You feel the superproject is in the way?
>>     git worktree add --for-submodule <path/to/sub> ...
>>     # The new submodule worktree puts the
>>     # submodule only UX first. so it feels like its own
>>     # repository, no need for specific flags.
>
> I am not sure I understand this one. What would that do? Put a worktree
> for submodule path/to/sub to ...?

> On Tue, Aug 22, 2017 at 8:33 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> > On Mon, Aug 21, 2017 at 09:42:54AM -0700, Stefan Beller wrote:
> >> On Mon, Aug 21, 2017 at 9:05 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> >> > On Fri, Aug 18, 2017 at 11:51:13PM -0700, Junio C Hamano wrote:
> >>     # You feel the superproject is in the way?
> >>     git worktree add --for-submodule <path/to/sub> ...
> >>     # The new submodule worktree puts the
> >>     # submodule only UX first. so it feels like its own
> >>     # repository, no need for specific flags.
> >
> > I am not sure I understand this one. What would that do? Put a worktree
> > for submodule path/to/sub to ...?
> 
> Yes, and at "..." you would have the UX of the submodule being
> its own repository, no interaction with the superproject.

> On Tue, Aug 22, 2017 at 11:10:52AM -0700, Stefan Beller wrote:
>> On Tue, Aug 22, 2017 at 8:33 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
>> > On Mon, Aug 21, 2017 at 09:42:54AM -0700, Stefan Beller wrote:
>> >> On Mon, Aug 21, 2017 at 9:05 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
>> >> > On Fri, Aug 18, 2017 at 11:51:13PM -0700, Junio C Hamano wrote:
>> >>     # You feel the superproject is in the way?
>> >>     git worktree add --for-submodule <path/to/sub> ...
>> >>     # The new submodule worktree puts the
>> >>     # submodule only UX first. so it feels like its own
>> >>     # repository, no need for specific flags.
>> >
>> > I am not sure I understand this one. What would that do? Put a worktree
>> > for submodule path/to/sub to ...?
>>
>> Yes, and at "..." you would have the UX of the submodule being
>> its own repository, no interaction with the superproject.
>
> Are you sure that is what Junio meant?

> On Tue, Aug 22, 2017 at 11:10:52AM -0700, Stefan Beller wrote:
>> On Tue, Aug 22, 2017 at 8:33 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
>> > On Mon, Aug 21, 2017 at 09:42:54AM -0700, Stefan Beller wrote:
>> >> On Mon, Aug 21, 2017 at 9:05 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
>> >> > On Fri, Aug 18, 2017 at 11:51:13PM -0700, Junio C Hamano wrote:
>> >>     # You feel the superproject is in the way?
>> >>     git worktree add --for-submodule <path/to/sub> ...
>> >>     # The new submodule worktree puts the
>> >>     # submodule only UX first. so it feels like its own
>> >>     # repository, no need for specific flags.
>> >
>> > I am not sure I understand this one. What would that do? Put a worktree
>> > for submodule path/to/sub to ...?
>> 
>> Yes, and at "..." you would have the UX of the submodule being
>> its own repository, no interaction with the superproject.
>
> Are you sure that is what Junio meant?

> Heiko Voigt <hvoigt@hvoigt.net> writes:
> 
> > So I think it is important that there are commits in the submodule so
> > its history makes sense independently for others.
> 
> I was trying to push the "just like trees" to the logical conclusion
> in order to see see if it leads to an absurd conclusions, or some
> useful scenario.  I do not necessarily subscribed to Jonathan's
> "vision" (I do not object to it as one possible mode of operation,
> either).
> 
> > Or how would you push out the history in the submodule in your idea?
> > Maybe I am missing something? What would be your use case with gitlinks
> > pointing to trees?
> 
> Not my idea.  But Stefan's message I was responding to said that
> object database for the superproject is the same as and mixed with
> object databases for the submodules, so it is plausible that push
> always happens at the superproject, and a mechanism to be invented
> (I mentioned the need for it in the message you are responding to)
> to enumerate all the commits bound from submodules to a range of
> superproject's commits would identify these trees to be pushed out.
> 
> In essense, "just like trees" folks want to use the gitlinks in the
> superproject to only specify the tree from the submodule project
> that should sit there.  And my point is that such a world view would
> lead to no need for branches in the submodule repository, no need
> for commits in the submodule repository, and no need for history in
> the submodule repository.  Which may or may not be a bad thing.

> As long as we are talking about idealized future world (well, at
> least an idea of somebody's "ideal", not necessarily shared by
> everybody), I wonder if there is even any need to have commits in
> submodules in such a world.  To realize such a "monorepo" world, you
> might be better off allowing a gitlink in the superproject to
> directly point at a tree object in a submodule repository (making
> them physically a single repository is an optional implementation
> detail I choose to ignore in this discussion).

> On Fri, Aug 18, 2017 at 11:51 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
>> As long as we are talking about idealized future world (well, at
>> least an idea of somebody's "ideal", not necessarily shared by
>> everybody), I wonder if there is even any need to have commits in
>> submodules in such a world.  To realize such a "monorepo" world, you
>> might be better off allowing a gitlink in the superproject to
>> directly point at a tree object in a submodule repository (making
>> them physically a single repository is an optional implementation
>> detail I choose to ignore in this discussion).
>
> Then the sharing between superprojects (e.g. send an Android's linux
> patch upstream or to another distro that also uses a superproject),
> becomes cumbersome as the commit messages are missing and
> potentially not specific to that subtree.

> On 17 Aug 2017, at 23:55, Stefan Beller <sbeller@google.com> wrote:
> 
> On Thu, Aug 17, 2017 at 2:21 PM, Lars Schneider
> <larsxschneider@gmail.com> wrote:
>> 
>>> Oh, wait.
>>> $ git log --oneline v2.13.0..v2.14.1 -- builtin/pull.c
>>> c9c63ee558 Merge branch 'sb/pull-rebase-submodule'
>>> a6d7eb2c7a pull: optionally rebase submodules (remote submodule changes only)
>>> could also be a culprit. Do you have pull.rebase set?
>> 
>> I bisected the problem today and "a6d7eb2c7a pull: optionally rebase submodules
>> (remote submodule changes only)" is indeed the culprit.
>> 
>> The commit seems to break the following test case.
>> 
>> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
>> index dcac364c5f..24f9729015 100755
>> --- a/t/t7400-submodule-basic.sh
>> +++ b/t/t7400-submodule-basic.sh
>> @@ -1289,4 +1289,19 @@ test_expect_success 'init properly sets the config' '
>>        test_must_fail git -C multisuper_clone config --get submodule.sub1.active
>> '
>> 
>> +test_expect_success 'submodule update and git pull with disabled submodule' '
>> +       test_when_finished "rm -rf multisuper_clone" &&
>> +       pwd=$(pwd) &&
>> +       git clone file://"$pwd"/multisuper multisuper_clone &&
>> +       (
>> +               cd multisuper_clone &&
>> +               git config --local submodule.sub0.update none &&
>> +               git submodule update --init --recursive &&
>> +               git pull --recurse-submodules &&
>> +               git submodule status | cut -c 1,43- >actual
>> +       ) &&
>> +       ls &&
>> +       test_cmp expect multisuper_clone/actual
>> +'
> 
> Thanks for providing this test.
> 
> cd trash directory.t7400-submodule-basic/multisuper_clone
> cat .git/config
> [submodule "sub0"]
>  update = none
>  active = true
>  url = file:///.../t/trash directory.t7400-submodule-basic/sub1
> 
> 
> 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).
> 
> The first sentence of .update is misleading IMHO as the
> these settings should strictly apply to the "submodule update"
> command. So "pull --recurse-submodules" ought to ignore it,
> instead the pull can do whatever it wants, namely treat the
> submodule roughly like a tree and either merge/rebase
> inside the submodule as well. The user *asked* for recursive
> pull after all.
> 
> 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?
> 
> When designing these new recursive submodule functionality
> outside the "submodule" command, I'd want submodules
> to behave as much as possible like trees.

>> In the past "submodule.<name>.update=none" was an easy way
>> to selectively disable certain Submodules.
>>
>> How would I do this with Git 2.14?
>
>     submodule.<name>.active = false
>
>> My gut feeling is that all commands should respect the
>> "submodule.<name>.update=none" setting.
>
> Well my gut feeling was that the "update" part of the name
> reponds to the subcommand, not the generic action.
>
> For example when you set update=none, git-status,
> recursive git-diff still reported the submodule.

> From: Lars Schneider <larsxschneider@gmail.com>
>
> Do not override the submodule configuration in the call to update
> the submodules, but give a weaker default.
>
> Reported-by: Lars Schneider <larsxschneider@gmail.com>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>   
> Personally I dislike this patch, but I have no better idea for the time
> being.

>  builtin/pull.c             |  6 ++++--
>  git-submodule.sh           |  7 ++++++-
>  t/t7400-submodule-basic.sh | 22 ++++++++++++++++++++++
>  3 files changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/pull.c b/builtin/pull.c
> index 9b86e519b1..be4f74d764 100644
> --- a/builtin/pull.c
> +++ b/builtin/pull.c
> @@ -553,7 +553,8 @@ static int rebase_submodules(void)
>  	cp.git_cmd = 1;
>  	cp.no_stdin = 1;
>  	argv_array_pushl(&cp.args, "submodule", "update",
> -				   "--recursive", "--rebase", NULL);
> +				   "--recursive", "--default-update",
> +				   "rebase", NULL);
>  
>  	return run_command(&cp);
>  }
> @@ -565,7 +566,8 @@ static int update_submodules(void)
>  	cp.git_cmd = 1;
>  	cp.no_stdin = 1;
>  	argv_array_pushl(&cp.args, "submodule", "update",
> -				   "--recursive", "--checkout", NULL);
> +				   "--recursive", "--default-update",
> +				   "checkout", NULL);
>  
>  	return run_command(&cp);
>  }
> diff --git a/git-submodule.sh b/git-submodule.sh
> index e131760eec..6dbc32e686 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -511,6 +511,7 @@ fetch_in_submodule () (
>  cmd_update()
>  {
>  	# parse $args after "submodule ... update".
> +	default_update="checkout"
>  	while test $# -ne 0
>  	do
>  		case "$1" in
> @@ -552,6 +553,10 @@ cmd_update()
>  		--checkout)
>  			update="checkout"
>  			;;
> +		--default-update)
> +			default_update="$2"
> +			shift
> +			;;
>  		--recommend-shallow)
>  			recommend_shallow="--recommend-shallow"
>  			;;
> @@ -619,7 +624,7 @@ cmd_update()
>  			update_module=$(git config submodule."$name".update)
>  			if test -z "$update_module"
>  			then
> -				update_module="checkout"
> +				update_module="$default_update"
>  			fi
>  		fi
>  
> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
> index dcac364c5f..ff64bf8528 100755
> --- a/t/t7400-submodule-basic.sh
> +++ b/t/t7400-submodule-basic.sh
> @@ -1289,4 +1289,26 @@ test_expect_success 'init properly sets the config' '
>  	test_must_fail git -C multisuper_clone config --get submodule.sub1.active
>  '
>  
> +test_expect_success 'submodule update and git pull with disabled submodule' '
> +	test_when_finished "rm -rf multisuper_clone" &&
> +	pwd=$(pwd) &&
> +	cat <<-\EOF >expect &&
> +	-sub0
> +	 sub1 (test2)
> +	 sub2 (test2)
> +	 sub3 (test2)
> +	 sub4 (test2)
> +	 sub5 (test2)
> +	EOF
> +	git clone file://"$pwd"/multisuper multisuper_clone &&
> +	(
> +		cd multisuper_clone &&
> +		git config --local submodule.sub0.update none &&
> +		git submodule update --init --recursive &&
> +		git pull --recurse-submodules &&
> +		git submodule status | cut -c 1,43- >actual
> +	) &&
> +	test_cmp expect multisuper_clone/actual
> +'
> +
>  test_done

> Stefan Beller <sbeller@google.com> writes:
> 
> > From: Lars Schneider <larsxschneider@gmail.com>
> >
> > Do not override the submodule configuration in the call to update
> > the submodules, but give a weaker default.
> >
> > Reported-by: Lars Schneider <larsxschneider@gmail.com>
> > Signed-off-by: Stefan Beller <sbeller@google.com>
> > ---
> >   
> > Personally I dislike this patch, but I have no better idea for the time
> > being.
> 
> The patch text from a cursory look seems reasonable to me.
> 
> It's not like you have 47 different codepaths that need to pay
> attention to the .update config and they all have to pass the new
> --default-update option, this is merely to fix one of them that
> relates to the problem reported by Lars, and you need a similar fix
> to other 46, right?
> 
> If you want the "--recurse-submodules" thing to always do the
> "weaker default" thing in your project, you can choose not to set
> .update to custom values in any of your submodules, so I do not
> think the reason why you dislike this change is because it would
> affect your use of submodules.
> 
> So I am a bit curious to learn which part of this change you dislike
> and why.

>> So I am a bit curious to learn which part of this change you dislike
>> and why.
>
> I am also curious. Isn't this the same strategy we are using in other
> places?
>

> On 21 Aug 2017, at 18:55, Stefan Beller <sbeller@google.com> wrote:
> 
> On Mon, Aug 21, 2017 at 9:20 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> 
>>> So I am a bit curious to learn which part of this change you dislike
>>> and why.
>> 
>> I am also curious. Isn't this the same strategy we are using in other
>> places?
>> 
> 
> I dislike it because the UX feels crude.  When reading the documentation,
> it seems to me as if submodule.<name> can be one of the following
> 
>    (none, checkout, rebase, merge, !<custom-command>)
> 
> This is perfect for "submodule-update", whose primary goal is
> to update submodules *somehow*. However other commands
> 
>    git rebase --recurse
>    git merge --recurse
>    git checkout --recurse
> 
> have a different primary mode of operation (note how their name
> is one of the modes from the set above), so it may get confusing
> for a user.
> 
> 'none'  and '!<custom-command>' seem like they would be okay
> for any of the commands above but then:
> 
>    git config submodule.<name>.update "!..."
>    git reset --hard --recurse
>    git status
>    # submodule is reported, because "!..." did not 'reset'.
> 
> Anyway. That dislike is just a minor gut feeling about the UX/UI
> being horrible. I wrote the patch to keep the conversation going,
> and if it fixes Lars problem, let's take it for now.

>
>> On 21 Aug 2017, at 18:55, Stefan Beller <sbeller@google.com> wrote:
>>
>> On Mon, Aug 21, 2017 at 9:20 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
>>
>>>> So I am a bit curious to learn which part of this change you dislike
>>>> and why.
>>>
>>> I am also curious. Isn't this the same strategy we are using in other
>>> places?
>>>
>>
>> I dislike it because the UX feels crude.  When reading the documentation,
>> it seems to me as if submodule.<name> can be one of the following
>>
>>    (none, checkout, rebase, merge, !<custom-command>)
>>
>> This is perfect for "submodule-update", whose primary goal is
>> to update submodules *somehow*. However other commands
>>
>>    git rebase --recurse
>>    git merge --recurse
>>    git checkout --recurse
>>
>> have a different primary mode of operation (note how their name
>> is one of the modes from the set above), so it may get confusing
>> for a user.
>>
>> 'none'  and '!<custom-command>' seem like they would be okay
>> for any of the commands above but then:
>>
>>    git config submodule.<name>.update "!..."
>>    git reset --hard --recurse
>>    git status
>>    # submodule is reported, because "!..." did not 'reset'.
>>
>> Anyway. That dislike is just a minor gut feeling about the UX/UI
>> being horrible. I wrote the patch to keep the conversation going,
>> and if it fixes Lars problem, let's take it for now.
>
> Well, I need just a way to disable certain Submodules completely.
> If you show me how "git config --local submodule.sub.active false"
> works then I don't need this patch.
>
> I tried to make it work here:
> https://public-inbox.org/git/89AB8AA3-8E19-46BA-B169-D1EA4CF4ABE7@gmail.com/

> On Mon, Aug 21, 2017 at 10:20 AM, Lars Schneider
> <larsxschneider@gmail.com> wrote:
> >
> >> On 21 Aug 2017, at 18:55, Stefan Beller <sbeller@google.com> wrote:
> >>
> >> On Mon, Aug 21, 2017 at 9:20 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> >>
> >>>> So I am a bit curious to learn which part of this change you dislike
> >>>> and why.
> >>>
> >>> I am also curious. Isn't this the same strategy we are using in other
> >>> places?
> >>>
> >>
> >> I dislike it because the UX feels crude.  When reading the documentation,
> >> it seems to me as if submodule.<name> can be one of the following
> >>
> >>    (none, checkout, rebase, merge, !<custom-command>)
> >>
> >> This is perfect for "submodule-update", whose primary goal is
> >> to update submodules *somehow*. However other commands
> >>
> >>    git rebase --recurse
> >>    git merge --recurse
> >>    git checkout --recurse
> >>
> >> have a different primary mode of operation (note how their name
> >> is one of the modes from the set above), so it may get confusing
> >> for a user.
> >>
> >> 'none'  and '!<custom-command>' seem like they would be okay
> >> for any of the commands above but then:
> >>
> >>    git config submodule.<name>.update "!..."
> >>    git reset --hard --recurse
> >>    git status
> >>    # submodule is reported, because "!..." did not 'reset'.
> >>
> >> Anyway. That dislike is just a minor gut feeling about the UX/UI
> >> being horrible. I wrote the patch to keep the conversation going,
> >> and if it fixes Lars problem, let's take it for now.
> >
> > Well, I need just a way to disable certain Submodules completely.
> > If you show me how "git config --local submodule.sub.active false"
> > works then I don't need this patch.

> >
> > I tried to make it work here:
> > https://public-inbox.org/git/89AB8AA3-8E19-46BA-B169-D1EA4CF4ABE7@gmail.com/
> 
> (A) you need to set expect there as well, to have sub{2,4,5} be expected
> there as well.
> 
> (B) That may hint at another (UX) bug.
> 
> The test case there uses "git submodule update --init".
> The init flag will set all submodules to active.
> 
> Maybe you want
> 
>     git config submodule.active ":(exclude)sub0"
>     git config --add submodule.active ":(exclude)sub2"
>     git config --add submodule.active "."
>     # Read: anything except sub0 and sub2 are interesting
> 
>     git submodule update
>     # no init flag, needed even for new submodules IIUC

>> > I tried to make it work here:
>> > https://public-inbox.org/git/89AB8AA3-8E19-46BA-B169-D1EA4CF4ABE7@gmail.com/
>> 
>> (A) you need to set expect there as well, to have sub{2,4,5} be expected
>> there as well.
>> 
>> (B) That may hint at another (UX) bug.
>> 
>> The test case there uses "git submodule update --init".
>> The init flag will set all submodules to active.
>> 
>> Maybe you want
>> 
>>     git config submodule.active ":(exclude)sub0"
>>     git config --add submodule.active ":(exclude)sub2"
>>     git config --add submodule.active "."
>>     # Read: anything except sub0 and sub2 are interesting
>> 
>>     git submodule update
>>     # no init flag, needed even for new submodules IIUC

> On 21 Aug 2017, at 20:21, Brandon Williams <bmwill@google.com> wrote:
> 
> On 08/21, Stefan Beller wrote:
>> On Mon, Aug 21, 2017 at 10:20 AM, Lars Schneider
>> <larsxschneider@gmail.com> wrote:
>>> 
>>>> On 21 Aug 2017, at 18:55, Stefan Beller <sbeller@google.com> wrote:
>>>> 
>>>> On Mon, Aug 21, 2017 at 9:20 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
>>>> 
>>>>>> So I am a bit curious to learn which part of this change you dislike
>>>>>> and why.
>>>>> 
>>>>> I am also curious. Isn't this the same strategy we are using in other
>>>>> places?
>>>>> 
>>>> 
>>>> I dislike it because the UX feels crude.  When reading the documentation,
>>>> it seems to me as if submodule.<name> can be one of the following
>>>> 
>>>>   (none, checkout, rebase, merge, !<custom-command>)
>>>> 
>>>> This is perfect for "submodule-update", whose primary goal is
>>>> to update submodules *somehow*. However other commands
>>>> 
>>>>   git rebase --recurse
>>>>   git merge --recurse
>>>>   git checkout --recurse
>>>> 
>>>> have a different primary mode of operation (note how their name
>>>> is one of the modes from the set above), so it may get confusing
>>>> for a user.
>>>> 
>>>> 'none'  and '!<custom-command>' seem like they would be okay
>>>> for any of the commands above but then:
>>>> 
>>>>   git config submodule.<name>.update "!..."
>>>>   git reset --hard --recurse
>>>>   git status
>>>>   # submodule is reported, because "!..." did not 'reset'.
>>>> 
>>>> Anyway. That dislike is just a minor gut feeling about the UX/UI
>>>> being horrible. I wrote the patch to keep the conversation going,
>>>> and if it fixes Lars problem, let's take it for now.
>>> 
>>> Well, I need just a way to disable certain Submodules completely.
>>> If you show me how "git config --local submodule.sub.active false"
>>> works then I don't need this patch.
> 
> Yeah if you want to completely disable a submodule (as in not even check
> it out) then setting .active to false would do that.  But as stefan
> pointed out and IIRC 'submodule update --init' with no pathspec sets all
> submodules to be active.  Perhaps it should only init submodules who
> don't already have an explicit active flag set.

>
> OK. I change my scripts to use ".active" and it seems to work nicely.
>
> I noticed one oddity, though:
>
> If I clone a repo using `git clone --recursive <url>` then the local
> Git config of the repo gets the following entry:
>
> [submodule]
>         active = .

> On Tue, Aug 22, 2017 at 7:50 AM, Lars Schneider
> <larsxschneider@gmail.com> wrote:
> >
> > OK. I change my scripts to use ".active" and it seems to work nicely.
> >
> > I noticed one oddity, though:
> >
> > If I clone a repo using `git clone --recursive <url>` then the local
> > Git config of the repo gets the following entry:
> >
> > [submodule]
> >         active = .
> 
> bb62e0a99f (clone: teach --recurse-submodules to optionally take a
> pathspec, 2017-03-17) makes it clear that this is intentional for
> --recurse-submodules, but doesn't exactly state that --recurse will
> behave the same. The idea here is that at clone time you can already
> give
> 
>     git clone --recurse=:(exclude)sub0 <url> <path>
> 
> and have your desired set of submodules there.
> Combined with the changes in the attr system, b0db704652
> (pathspec: allow querying for attributes, 2017-03-13)
> you could make up things like this:
> 
>   $ cat .gitattributes
>   /sub0 label0
>   /sub1
>   /sub2 label1 label2
>   /sub3 label1
>   /platform-specifc-subs/* label1 label2
> 
> and then get a clone via
> 
>     git clone --recurse=:(attr:label2). <url> <path>
> 
> for example. The labeling via the attributes allows for
> complex patterns, but a relatively easy command line, that you
> can share with coworkers.
> 
> > Is this intentional? Something in the git/git test harness seems to prevent
> > that. I was not able to write a test to replicate the issue.
> >
> > Any idea?
> 
> I do not seem to understand the perceived bug?
> The setting of submodule.active=<pathspec> seems intentional to me,
> but how would you not reproduce it? Maybe Brandon has an idea.
> 

> On 18 Aug 2017, at 19:16, Stefan Beller <sbeller@google.com> wrote:
> 
>> In the past "submodule.<name>.update=none" was an easy way
>> to selectively disable certain Submodules.
>> 
>> How would I do this with Git 2.14?
> 
>    submodule.<name>.active = false