On Tue, Mar 14, 2017 at 10:46 AM, Junio C Hamano <gits...@pobox.com> wrote:
> Brandon Williams <bmw...@google.com> writes:
>
>> Signed-off-by: Brandon Williams <bmw...@google.com>
>> ---
>> git-submodule.sh | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/git-submodule.sh b/git-submodule.sh
>> index 136e26a2c..ab233712d 100755
>> --- a/git-submodule.sh
>> +++ b/git-submodule.sh
>> @@ -1010,14 +1010,13 @@ cmd_status()
>> do
>> die_if_unmatched "$mode" "$sha1"
>> name=$(git submodule--helper name "$sm_path") || exit
>> - url=$(git config submodule."$name".url)
>> displaypath=$(git submodule--helper relative-path
>> "$prefix$sm_path" "$wt_prefix")
>> if test "$stage" = U
>> then
>> say "U$sha1 $displaypath"
>> continue
>> fi
>> - if test -z "$url" ||
>> + if ! git submodule--helper is-active "$sm_path" ||
>> {
>> ! test -d "$sm_path"/.git &&
>> ! test -f "$sm_path"/.git
>
> The $name is no longer used after this step in cmd_status function,
> as the sole purpose of learning the name from the path was so that
> we can ask if the submodule has .URL defined and the query is done
> by name, not path.
>
> This actually raises a question.
>
> Shouldn't "submodule--helper is-active" ask about submodule while
> identifying the submodule in question by name? Or do all (or most
> of) the callers start from path and ask is-active on them so that it
> is handier to let them ask by path?
A similar observation can be made, when looking at
submodule_from_name and submodule_from_path
(both defined in submodule-config.h)
submodule_from_name has only one real world use case in code,
which I suspect is even a bug.
That line was last touched with 851e18c3859a (submodule: use new config
API for worktree configurations). but the original mechanism comes
from 7dce19d374 (fetch/pull: Add the --recurse-submodules option).
There we take the path as name and if a real name exists, the name
is overwritten with the real name, i.e.
name = name_for_path(path) ? name_for_path(path) : path;
which IMHO is overly accepting and we should just die in case of
!name_for_path.
So to answer your original question, I think the codebase currently
thinks by_path is handier, the name is a mere internal field in
"struct submodule", useful for looking up its git dir.
Thanks,
Stefan
