On Tue, Aug 22, 2023 at 09:28:37PM -0600, Alex Henrie wrote:
> The unusual syntax --recurse-submodules="" (that is,
> --recurse-submodules with an empty string argument) has been an
> undocumented synonym of --recurse-submodules without an argument since
> commit 8f0700dd33 (fetch/pull: Add the 'on-demand' value to the
> --recurse-submodules option, 2011-03-06). Deprecate that syntax to avoid
> confusion with the submodule.recurse config option, where
> submodule.recurse="" is equivalent to --no-recurse-submodules.
>
> The same thing was done for --rebase-merges="" in commit 33561f5170
> (rebase: deprecate --rebase-merges="", 2023-03-25).
Makes sense, and this is certainly in the same spirit as your
33561f5170.
> Signed-off-by: Alex Henrie <alexhenrie24@xxxxxxxxx>
> ---
> submodule-config.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/submodule-config.c b/submodule-config.c
> index 6a48fd12f6..8acb42744d 100644
> --- a/submodule-config.c
> +++ b/submodule-config.c
> @@ -332,11 +332,17 @@ int option_fetch_parse_recurse_submodules(const struct option *opt,
>
> if (unset) {
> *v = RECURSE_SUBMODULES_OFF;
> + } else if (!arg) {
> + *v = RECURSE_SUBMODULES_ON;
> } else {
> - if (arg)
> - *v = parse_fetch_recurse_submodules_arg(opt->long_name, arg);
> - else
> - *v = RECURSE_SUBMODULES_ON;
> + if (!*arg) {
> + warning(_("--recurse-submodules with an empty string "
> + "argument is deprecated and will stop "
> + "working in a future version of Git. Use "
> + "--recurse-submodules without an argument "
> + "instead, which does the same thing."));
This advice says to use `--recurse-submodules` as a non-deprecated
synonym for `--recurse-submodules=""`, but I am not so sure that is
correct advice.
In the pre-image of this patch, having arg be set to the empty string
would cause us to fall into the path that executes
*v = parse_fetch_recurse_submodules_arg(opt->long_name, arg);
which calls `parse_fetch_recurse()` -> `git_parse_maybe_bool()` ->
`git_parse_maybe_bool_text()` which given the empty string will return
0.
So here we'd be doing the equivalent of
*v = RECURSE_SUBMODULES_OFF;
when trying to parse `--recurse-submodules=""`. Should this advice
instead say "[...] Use --no-recurse-submodules without an argument,
which does the same thing"?
Thanks,
Taylor
