Thread (11 messages) flat view 11 messages, 2 authors, 2016-06-15

Re: [PATCH 2/6] Teach remote.c about the remote.default configuration setting.

From: Marc Branchaud <hidden>
Date: 2016-06-15 22:54:14

On 12-07-11 02:21 PM, Junio C Hamano wrote:
marcnarc@xiplink.com writes:
quoted
From: Marc Branchaud <redacted>

The code now has a default_remote_name and an effective_remote_name:

 - default_remote_name is set by remote.default in the config, or is "origin"
   if remote.default doesn't exist ("origin" was the fallback value before
   this change).

 - effective_remote_name is the name of the remote tracked by the current
   branch, or is default_remote_name if the current branch doesn't have a
   remote.

This has a minor side effect on the default behavior of "git push".  Consider
the following sequence of commands:

      git config remote.default foo                 # Set default remote
      git checkout somelocalbranch                  # Does not track a remote
      git remote add origin ssh://example.com/repo  # Add a new "origin"
      git push

Prior to this change, the above "git push" would attempt to push to the new
"origin" repository at ssh://example.com/repo.  Now instead that "git push"
will attempt to push to the repository named "foo".
Hrm, is this a _minor_ side effect?
Well, I thought so...  :)

It's minor because of what you say:
Isn't that a natural and direct consequence of "now you can set
remote.default configuration to name the remote you want to use in
place of traditional 'origin'" feature?  I think changing the
behaviour of "git push" in such a way is the point (may not be the
only but one of the important points) of this series.
I agree.  Phil Hord pointed out the change in behaviour and felt the commit
message should explain it.

Should I just remove "minor"?
quoted
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index cb97cc1..fc17d39 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -27,10 +27,16 @@ documentation for linkgit:git-receive-pack[1].
 OPTIONS[[OPTIONS]]
 ------------------
 <repository>::
-	The "remote" repository that is destination of a push
+	The "remote" repository that is the destination of the push
 	operation.  This parameter can be either a URL
 	(see the section <<URLS,GIT URLS>> below) or the name
 	of a remote (see the section <<REMOTES,REMOTES>> below).
+	If this parameter is omitted, git tries to use the remote
+	associated with the currently checked-out branch.  If there
+	is no remote associated with the current branch, git uses
+	the remote named by the "remote.default" configuration variable.
+	If "remote.default" is not set, git uses the name "origin" even
+	if there is no actual remote named "origin".
This comment applies to other changes to the documentation in this
patch I didn't quote, but I think the phrasing 'even if there is no
acutual remote named "origin"' needs to be rethought, because "we
use it even if there is no such remote determined with this logic"
applies equally to branch.$name.remote, remote.default or built-in
default value of remote.default which happens to be "origin".
I'm sorry, but I'm having trouble understanding what you mean.  I don't see
how the "because ..." part of your sentence suggests what aspect needs
rephrasing.

(BTW, I'm under the impression that when git sets branch.$name.remote it
doesn't ever have to fall back to the default remote name.  That is, commands
that set the value always have the user explicitly, though perhaps
indirectly, specifying the remote.  Did I miss something?)
quoted
diff --git a/remote.c b/remote.c
index 6f371e0..2ebdbbd 100644
--- a/remote.c
+++ b/remote.c
@@ -47,6 +47,7 @@ static int branches_alloc;
 static int branches_nr;
 
 static struct branch *current_branch;
+static const char *default_remote_name;
 static const char *effective_remote_name;
Coming from UNIX background, "effective" gives me an impression that
it is contrasted with "real" (i.e. there is "real remote" for the
branch, but during this particular invocation it is overridden and
"effective remote" is used instead).  Perhaps it is just me.
I did have something like that in mind when I chose "effective":  There's a
default remote name that's normally used, but in the certain circumstances
it's overridden.  Perhaps "effective_default_remote_name" would be better,
though that's getting to be a mouthful (plus it would mean having an
"explicit_effective_default_remote_name", which IMHO is a bit much).
Something along the line of remote-name-to-use, use-remote, might
sound more natural.
current_remote_name?

		M.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help