Re: [PATCH v2 03/11] submodule deinit: use most reliable url

[Stefan Beller <sbeller@google.com>]

On Wed, Mar 8, 2017 at 5:23 PM, Brandon Williams <bmwill@google.com> wrote:
> The user could have configured the submodule to have a different URL
> from the one in the superproject's config.  To account for this read
> what the submodule has configured for remote.origin.url and use that
> instead.

When reading this commit message, I first thought this is an unrelated
bug fix. However it is not unrelated. In a later patch you introduce a mode
where submodule.<name>.url may not exist in .git/config any more,
(there may be a .existence flag instead?) such that url="", which is
obviously a bad UI:

    Submodule '$name' (<empty string>) unregistered for path '$displaypath'"

Like the first patch of this series, we could have a helper function
"git submodule--helper url <name>" that figures out how to get the URL:
1) Look into that GIT_DIR
2) if non-existent check .git/config for the URL or
3) fall back to .gitmodules?

Instead of having such a helper, we directly look into that first option
hoping it exists, however it doesn't have to:

  git submodule init <ps>
  # no command in between
  git submodule deinit <ps>
  # submodule was not cloned yet, but we still test positive for
    > # Remove the .git/config entries (unless the user already did it)
    > if test -n "$(git config --get-regexp submodule."$name\.")"

I am not sure if there is an easy way out here.

Thinking about the name for such a url helper lookup, I wonder if
we want to have

    git submodule--helper show-url <name>

as potentially we end up having this issue for a lot
of different things (e.g. submodule.<name>.shallow = (true,false),
or in case the submodule is cloned you can give the actual depth
as an integral number), so maybe we'd want to introduce one
layer of indirection

    git submodule--helper ask-property \
       (is-active/URL/depth/size/..) <name>

So with that said, I wonder if we also want to ease up:

    GIT_DIR="$(git rev-parse --git-path modules/$name

to be

    GIT_DIR=$(git submodule--helper show-git-dir $name)

>                 then
>                         # Remove the whole section so we have a clean state when
>                         # the user later decides to init this submodule again
> -                       url=$(git config submodule."$name".url)
> +                       url=$(GIT_DIR="$(git rev-parse --git-path modules/$name)" git config remote.origin.url)

In the submodule helper we have get_default_remote(), which we do not
have in shell
(which we seemed to have in shell?), so maybe it's worth not using origin here,
although I guess it is rare that the original remote is named other than origin.

>                         git config --remove-section submodule."$name" 2>/dev/null &&
>                         say "$(eval_gettext "Submodule '\$name' (\$url) unregistered for path '\$displaypath'")"
>                 fi
> --
> 2.12.0.246.ga2ecc84866-goog
>

Thanks,
Stefan