[PATCH 10/11] remote: always require at least one url in a remote

When we return a struct from remote_get(), the result _almost_ always has at least one url. In remotes_remote_get_1(), we do this:

        if (name_given && !valid_remote(ret))
                add_url_alias(remote_state, ret, name);
        if (!valid_remote(ret))
                return NULL;

So if the remote doesn't have a url, we give it one based on the name (this is how unconfigured urls are used as remotes). And if that doesn't work, we return NULL.

But there's a catch: valid_remote() checks that we have at least one url _unless_ the remote.*.vcs field is set. This comes from c578f51d52 (Add a config option for remotes to specify a foreign vcs, 2009-11-18), and the whole idea was to support remote helpers that don't have their own url.

However, that mode has been broken since 25d5cc488a (Pass unknown protocols to external protocol handlers, 2009-12-09)! That commit unconditionally looks at the url in get_helper(), causing a segfault with something like:

  git -c remote.foo.vcs=bar fetch foo

We could fix that now, of course. But given that it has been broken for almost 15 years and nobody noticed, there's a better option. This weird "there might not be a url" special case requires checks all over the code base, and it's not clear if there are other similar segfaults lurking. It would be nice if we could drop that special case.

So instead, let's let the "the remote name is the url" code kick in. If you have "remote.foo.vcs", then your url (unless otherwise configured) is "foo". This does have a visible effect compared to what 25d5cc488a was trying to do. The idea back then is that for a remote without a url, we'd run:

   # only one command-line option!
   git-remote-bar foo

whereas with our default url, now we'll run:

  git-remote-bar foo foo

Again, in practice nobody can be relying on this because it has been segfaulting for 15 years. We should consider just removing this "vcs" config option entirely, but that would be a user-visible breakage. So by fixing it this way, we can keep things working that have been working, and simplify away one special case inside our code.

This fixes the segfault from 25d5cc488a (demonstrated by the test), and we can build further cleanups on top.

Signed-off-by: Jeff King <peff@peff.net>
---
 remote.c                  | 2 +-
 t/t5801-remote-helpers.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)