Re: [RFC/PATCH] Introduce remote.pushdefault

[Ramkumar Ramachandra <artagnon@gmail.com>]

Junio C Hamano wrote:
> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>
>> diff --git a/Documentation/config.txt b/Documentation/config.txt
>> index 9b11597..82a4a78 100644
>> --- a/Documentation/config.txt
>> +++ b/Documentation/config.txt
>> @@ -1884,6 +1884,10 @@ receive.updateserverinfo::
>>       If set to true, git-receive-pack will run git-update-server-info
>>       after receiving data from git-push and updating refs.
>>
>> +remote.pushdefault::
>> +     The remote to push to by default.  Overrides the
>> +     branch-specific configuration `branch.<name>.remote`.
>
> It feels unexpected to see "I may have said while on this branch I
> push there and on that branch I push somewhere else, but no, with
> this single configuration I'm invalidating all these previous
> statements, and all pushes go to this new place".
>
> Shouldn't the default be the default that is to be overridden by
> other configuration that is more specific?  That is, "I would
> normally push to this remote and unless I say otherwise that is all
> I have to say, but for this particular branch, I push to somehwere
> else".

Oops, I meant to have it overriden by branch-specific configuration.  Fixed now.

>> diff --git a/builtin/push.c b/builtin/push.c
>> index 42b129d..d447a80 100644
>> --- a/builtin/push.c
>> +++ b/builtin/push.c
>> @@ -322,7 +322,7 @@ static int push_with_options(struct transport *transport, int flags)
>>  static int do_push(const char *repo, int flags)
>>  {
>>       int i, errs;
>> -     struct remote *remote = remote_get(repo);
>> +     struct remote *remote = pushremote_get(repo);
>>       const char **url;
>>       int url_nr;
>>
>> diff --git a/remote.c b/remote.c
>> index e53a6eb..08bb803 100644
>> --- a/remote.c
>> +++ b/remote.c
>> @@ -48,6 +48,7 @@ static int branches_nr;
>>
>>  static struct branch *current_branch;
>>  static const char *default_remote_name;
>> +static const char *pushremote_name;
>>  static int explicit_default_remote_name;
>>
>>  static struct rewrites rewrites;
>> @@ -349,6 +350,14 @@ static int handle_config(const char *key, const char *value, void *cb)
>>       const char *subkey;
>>       struct remote *remote;
>>       struct branch *branch;
>> +     if (!prefixcmp(key,  "remote.")) {
>> +             name = key + 7;
>> +             if (!strcmp(name, "pushdefault")) {
>> +                     if (!value)
>> +                             return config_error_nonbool(key);
>> +                     pushremote_name = xstrdup(value);
>> +             }
>> +     }
>>       if (!prefixcmp(key, "branch.")) {
>>               name = key + 7;
>>               subkey = strrchr(name, '.');
>> @@ -388,8 +397,6 @@ static int handle_config(const char *key, const char *value, void *cb)
>>                       add_instead_of(rewrite, xstrdup(value));
>>               }
>>       }
>> -     if (prefixcmp(key,  "remote."))
>> -             return 0;
>
> Why is this no longer needed?
>
> All the remainder of this function is about "remote.*" config and
> this rejects other keys, like "user.name", etc.

I'm sorry.  I read that as if (!prefixcmp(key, "remote.")), which is
an entirely different thing.  Fixed now.

Thanks.