On Mon, 27 Jul 2009, Junio C Hamano wrote:
> Johan Herland <johan@xxxxxxxxxxx> writes:
>
> > diff --git a/builtin-ls-remote.c b/builtin-ls-remote.c
> > index 78a88f7..4c6fc58 100644
> > --- a/builtin-ls-remote.c
> > +++ b/builtin-ls-remote.c
> > @@ -87,9 +87,9 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
> > }
> > }
> > remote = nongit ? NULL : remote_get(dest);
> > - if (remote && !remote->url_nr)
> > + if (!nongit && !remote)
> > die("remote %s has no configured URL", dest);
>
> It appears to me that remote_get() calls make_remote() that never returns
> NULL, thus this check will never trigger. At least, the die() message
> needs to be updated, since remote_get() may return NULL for some reason
> other than "no configured URL", but the condition the message says is not
> what this code checks.
The previous patch in the series causes remote_get() to return NULL if
the remote configuration is invalid. You're right, however, that the
message should be something more directly accurate.
> It is Ok if the new world order is that the URL does not have to be known
> before transport_get() is called, but if that is the case, it should be
> made clear by removing this check from here (and possibly having a new
> check elsewhere where it really matters, e.g. somewhere in transport
> code).
This spot is trying to make sure that the remote was valid, according to
remote.c
> > - transport = transport_get(remote, remote ? remote->url[0] : dest);
> > + transport = transport_get(remote, remote ? NULL : dest);
> > if (uploadpack != NULL)
> > transport_set_option(transport, TRANS_OPT_UPLOADPACK, uploadpack);
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html