On 03/08, Stefan Beller wrote:

On Wed, Mar 8, 2017 at 5:23 PM, Brandon Williams [off-list ref] wrote:

quoted

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:

So the later patch doesn't actually omit submodule.<name>.url but we
could end up with this if you init and then deinit without actually
cloning the submodule.  So maybe the best bet is to drop this patch from
the series and we can include one like it in a follow up which uses a
helper function that you suggest.

    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)

quoted

                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.

quoted

                        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

-- 
Brandon Williams