Re: [PATCH] submodule: explicitly specify on-demand upon push

From: Glen Choo <chooglen@google.com>
Date: Nov 9, 2022, 18:31 UTC
Message-ID: <kl6l8rkk3rpy.fsf@chooglen-macbookpro.roam.corp.google.com>
In-Reply-To: <20221108214344.272053-1-jonathantanmy@google.com>

Jonathan Tan <jonathantanmy@google.com> writes:

Show 19 quoted lines

>> Not having thought about this a lot, the behavior I might expect is
>> something along the lines of recursively pushing throughout the
>> submodule tree, stopping the recursion as soon as we get to a nested
>> submodule which says "don't push any of my children".
>> 
>> On the other hand, I could sympathize with a compelling argument that
>> the superproject alone should be in charge of determining what gets
>> pushed.
>> 
>> Though TBH, it seems like the former is more convincing. If I depend on
>> an external repository through a submodule, and that repository itself
>> has submodules, it would be nice to configure (once) that I don't want
>> to even try and push any of that repository's children.
>
> I just tried this and the behavior is reasonable except possibly for when
> push.recurseSubmodules=only is configured in a top-level submodule. Let's see
> if other people have something to say. For me, this would also be fine since we
> can just make sure that we don't configure "only" in top-level submodules that
> have their own nested submodules.

For this patch, I think what Jonathan has proposed is preferable because that's close to what "git fetch" [1] does, i.e. when a superproject has parsed a setting for "recurse into submodules" (from either the CLI or config), it passes that value to its submodules via "--recurse-submodules", overriding the submodule's config.

In the longer term though, I think neither "git fetch" nor "git push" (as of this patch) gets it right. I think it should be closer to:

- If the user has passed a "--recurse-submodules" flag via the CLI,
  respect that in the superproject and all submodules regardless of
  nesting. (Make sure not to conflate config values with the CLI option
  in the way "git fetch" does today.)
- Otherwise, each submodule should respect their own config setting if
  it is set.
- Otherwise, each submodule should respect their superproject's config
  setting if it is set.

[1] "git fetch" is somewhat more complicated than this, since it also
  reads values from .gitmodules and passes a "default" value, neither of
  which is relevant here I think. See this ML thread for some previous
  discussion.
  https://lore.kernel.org/git/kl6lbkwa8h5n.fsf@chooglen-macbookpro.roam.corp.google.com/