Re: [PATCH v4] clone: update submodule.recurse in config when using --recurse-submodule

From: Mahi Kolla <mahikolla@google.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Mahi Kolla via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org,
	Philippe Blain <levraiphilippeblain@gmail.com>
Subject: Re: [PATCH v4] clone: update submodule.recurse in config when using --recurse-submodule
Date: Tue, 10 Aug 2021 00:26:01 -0700	[thread overview]
Message-ID: <CAN3QUFYPjsvBRGegO-kC7+gcFDczOqQSw-UYphnLHx=6-6kkwA@mail.gmail.com> (raw)
In-Reply-To: <xmqqzgtqe2w6.fsf@gitster.g>

Hi Junio,

Thank you for your feedback!

On Mon, Aug 9, 2021 at 2:15 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Mahi Kolla via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Mahi Kolla <mahikolla@google.com>
> >
> > When running 'git clone --recurse-submodules', developers might expect various other commands such as 'pull' and 'checkout' to also run recursively into submodules. Set 'submodule.recurse' to true when 'git clone' is run with '--recurse-submodules'.
>
> Please wrap overlong lines in your proposed log message to say 70 or
> so columns.
>

Ah, my bad, will do so going forward.

> Some developers might expect, but wouldn't some others want to see
> this not set to true, but want to recurse only into some but not all
> submodules?
>

I definitely agree with this. Currently, the `--recurse-submodules`
option takes in 1 parameter, a pathspec to the submodule they would
like to initialize. `submodule.active` stores this path or `.` if no
path is specified. Accordingly, `submodule.recurse=true` will only
apply to the submodules specified by the user in `submodule.active`.
This way users can ensure commands are run recursively only in
submodules they have listed as active.

> Is it possible to avoid changing the behaviour unconditionally and
> potentially breaking existing users by making it an opt-in feature,
> e.g. "git clone --recurse-submodules" would work as the current
> users would expect, while "git clone --recurse-submodules=sticky"
> would set submodule.recurse to true, or something?

As mentioned, the `submodule.recurse=true` will only apply to active
submodules specified by the user. Setting this config value when the
user runs their initial `git clone` minimizes the number of times a
developer must use the `--recurse-submodule` option on other commands.

However, this is a behavior change that may be surprising for
developers. To ensure a smooth rollout and easy adoption, I think
adding a message using an `advice.*` config setting would be useful.
When a user runs `git clone --recurse-submodules` an advice message
will pop up alerting them `submodule.recurse=true`, what this means
for other commands' functionality, and how to change it back (`git -C
dst-dir config submodule.recurse false`). This also gives the user the
option to turn off the advice message if they don't need it.

I'll also update the appropriate documentation to better explain how
setting submodule.recurse=true effects workflow.

Let me know what you think!

Best,
Mahi Kolla