Re: [RFC/PATCH] Introduce remote.pushdefault

Ramkumar Ramachandra [off-list ref] writes:

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".

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 a bit confused....

 	name = key + 7;
 	if (*name == '/') {
 		warning("Config remote shorthand cannot begin with '/': %s",

Junio C Hamano wrote:

Ramkumar Ramachandra [off-list ref] writes:

quoted

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.

quoted

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.

Sorry about the horribly late response- I just got around to
re-rolling this, and had a doubt.

Junio C Hamano [off-list ref] wrote:

Ramkumar Ramachandra [off-list ref] writes:

quoted

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".

I'm a little confused as to where this configuration variable will be
useful.  On a fresh clone from Github, I get branch.master.remote
configured to "origin".  How will adding remote.pushdefault have any
impact, unless I explicitly remove this branch-specific remote
configuration?  Besides, without branch.<name>.remote configured, I
can't even pull and expect changes to be merged.  So, really: what is
the use of remote.pushdefault?

I'm dropping this patch, and just going with branch.<name>.pushremote,
unless you convince me otherwise.

On Sun, Mar 17, 2013 at 06:00:08AM +0530, Ramkumar Ramachandra wrote:

quoted

quoted

+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".

I'm a little confused as to where this configuration variable will be
useful.  On a fresh clone from Github, I get branch.master.remote
configured to "origin".  How will adding remote.pushdefault have any
impact, unless I explicitly remove this branch-specific remote
configuration?  Besides, without branch.<name>.remote configured, I
can't even pull and expect changes to be merged.  So, really: what is
the use of remote.pushdefault?

I'm dropping this patch, and just going with branch.<name>.pushremote,
unless you convince me otherwise.

That is why I described the scheme I did in [1]. It uses the following
two general rules:

  1. Per-branch config trumps repo-wide config.

  2. Push-specific config (e.g., "remote.pushdefault") trumps
     non-specific config (e.g., "remote.default") for pushing.

So the push lookup list is (in order of precedence):

  1. branch.*.pushremote
  2. remote.pushdefault
  3. branch.*.remote
  4. remote.default
  5. origin

and it solves Junio's issue because the way to say "override my
remote.pushdefault for this branch" is not to set "branch.*.remote", but
to set "branch.*.pushremote".

-Peff

[1] http://article.gmane.org/gmane.comp.version-control.git/215751

Jeff King wrote:

So the push lookup list is (in order of precedence):

  1. branch.*.pushremote
  2. remote.pushdefault
  3. branch.*.remote
  4. remote.default
  5. origin

and it solves Junio's issue because the way to say "override my
remote.pushdefault for this branch" is not to set "branch.*.remote", but
to set "branch.*.pushremote".

Right, thanks for clearing that up Jeff.  I'll re-roll with
remote.pushdefault overriding branch.<name>.remote.

Ram