Re: [PATCH 4/6] remote.c: introduce a way to have different remotes for fetch/push

[Ramkumar Ramachandra <artagnon@gmail.com>]

Jonathan Nieder wrote:
> Junio C Hamano wrote:
>> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>>>> -   struct remote *remote = remote_get(repo);
>>>> +   struct remote *remote = pushremote_get(repo);
>>>
>>> "struct remote" has url and pushurl fields.  What do they mean in the
>>> context of these two accessors?  /me is confused.
>>>
>>> Is the idea that now I should not use pushurl any more, and that I
>>> should use pushremote_get and use url instead?
> [...]
>>               At the programming level, you would still ask what the
>> URL to be pushed to to the remote obtained here, and would use
>> pushurl if defined, or url otherwise.
>
> Ah, I think I see.  It might be more convenient to the caller if
> pushremote_get returned a remote with url set to the pushurl, but
> that would prevent sharing the struct with other callers that want
> that remote for fetching.

We started off with a generic "remote" (for both fetching and
pushing), then added .pushurl on top of this remote.  Now we've
introduced something called a pushremote, a logically distinct remote
from "remote"; pushremote_get() is meant to return this logically
different remote, falling back to the remote_get() codepath if not
present.  This is a perfect migration that trivially preserves
backward compatibility.


> So instead, the idea is something like
>
>         remote: support a different default remote for pushing
>
>         Teach remote_get() to accept an argument FOR_FETCH or FOR_PUSH
>         that determines, when no remote is passed to it, whether to use
>         the default remote for fetching or the default for pushing.
>
>         The default remote for fetching is stored in the static var
>         "default_remote_name", while the default for pushing, if set,
>         is in "default_push_remote_name".
>
>         Currently there is never a different default for pushing set
>         but later patches will change that.
>
> If remote_get() gained a new required parameter, that would force all
> call sites to be examined (even any potential call sites added by new
> patches in flight) and there would no longer be need for the
> remote_get_1() function.

I like this, but let's save it for a future patch as it requires some
extensive refactoring.