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

> 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