Re: [PATCH 02/15] submodule: don't use submodule_from_name

From: Stefan Beller <sbeller@google.com>
Date: Jul 25, 2017, 23:17 UTC
Message-ID: <CAGZ79kY9Pdk5C8=k-AQpCPwo3q9Jzfg9A93UQxGyyf_OyrMS_Q@mail.gmail.com>
In-Reply-To: <20170725213928.125998-3-bmwill@google.com>

On Tue, Jul 25, 2017 at 2:39 PM, Brandon Williams <bmwill@google.com> wrote:

Show 5 quoted lines

> The function 'submodule_from_name()' is being used incorrectly here as a
> submodule path is being used instead of a submodule name.  Since the
> correct function to use with a path to a submodule is already being used
> ('submodule_from_path()') let's remove the call to
> 'submodule_from_name()'.

This blames to 851e18c385 (submodule: use new config API for worktree configurations, 2015-08-17), but that is a refactoring. The issue of using the path instead of a name was there before that. The actual issue was introduced in 7dce19d374 (fetch/pull: Add the --recurse-submodules option, 2010-11-12).

+     name = ce->name;
+     name_for_path =
unsorted_string_list_lookup(&config_name_for_path, ce->name);
+     if (name_for_path)
+         name = name_for_path->util;

Rereading the archives, there was quite some discussion on the design of these patches, but these lines of code did not get any attention

    https://public-inbox.org/git/4CDB3063.5010801@web.de/

I cc'd Jens in the hope of him having a good memory why he wrote the code that way. :)

Note that this is the last caller of submodule_from_name being removed, so I would expect removal of submodule_from_name from the t/helper/test-submodule-config.c as well as Documentation/technical/api-submodule-config.txt in a later part of this series. (Well technically it could go outside of the series, but in the mean time we'd document and test dead code)

Show 21 quoted lines

> Signed-off-by: Brandon Williams <bmwill@google.com>
> ---
>  submodule.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/submodule.c b/submodule.c
> index 7e87e4698..fd391aea6 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -1177,8 +1177,6 @@ static int get_next_submodule(struct child_process *cp,
>                         continue;
>
>                 submodule = submodule_from_path(&null_oid, ce->name);
> -               if (!submodule)
> -                       submodule = submodule_from_name(&null_oid, ce->name);
>
>                 default_argv = "yes";
>                 if (spf->command_line_option == RECURSE_SUBMODULES_DEFAULT) {
> --
> 2.14.0.rc0.400.g1c36432dff-goog
>