[PATCH] branch: make sure the upstream remote is configured
A command of e.g.
git push --set-upstream /tmp/t master
will call install_branch_config() with a remote name of /tmp/t. This
function will set the 'branch.master.remote' key to, which is
nonsensical as there is no remote by that name.
Instead, make sure that the remote given does exist when writing the
configuration and warn if it does not. In order to distinguish named
remotes, introduce REMOTE_NONE as the default origin for remotes,
which the functions reading from the different sources will
overwrite. Thus, an origin of REMOTE_NONE means it has been created at
run-time in order to push to it.
Signed-off-by: Carlos Martín Nieto c...@elego.de
---
It's somewhat surprising that there didn't seem to be a way to
distinguish named remotes from those created from a command-line path,
but I guess nobody needed to.
branch.c | 11 +++
remote.h | 1 +
t/t5523-push-upstream.sh | 5 +
3 files changed, 17 insertions(+)
diff --git a/branch.c b/branch.c
index c5c6984..cefb8f6 100644
--- a/branch.c
+++ b/branch.c
@@ -53,6 +53,7 @@ void install_branch_config(int flag, const char *local, const
char *origin, cons
int remote_is_branch = !prefixcmp(remote, refs/heads/);
struct strbuf key = STRBUF_INIT;
int rebasing = should_setup_rebase(origin);
+ struct remote *r = remote_get(origin);
if (remote_is_branch
!strcmp(local, shortname)
@@ -62,6 +63,16 @@ void install_branch_config(int flag, const char *local,
const char *origin, cons
return;
}
+ /*
+* Make sure that the remote passed is a configured remote, or
+* we end up setting 'branch.foo.remote = /tmp/t' which is
+* nonsensical.
+*/
+ if (origin strcmp(origin, .) r-origin == REMOTE_NONE) {
+ warning(_(there is no remote named '%s', no upstream
configuration will be set.), origin);
+ return;
+ }
+
strbuf_addf(key, branch.%s.remote, local);
git_config_set(key.buf, origin ? origin : .);
diff --git a/remote.h b/remote.h
index cf56724..92f6e33 100644
--- a/remote.h
+++ b/remote.h
@@ -2,6 +2,7 @@
#define REMOTE_H
enum {
+ REMOTE_NONE,
REMOTE_CONFIG,
REMOTE_REMOTES,
REMOTE_BRANCHES
diff --git a/t/t5523-push-upstream.sh b/t/t5523-push-upstream.sh
index 3683df1..e84c2f8 100755
--- a/t/t5523-push-upstream.sh
+++ b/t/t5523-push-upstream.sh
@@ -71,6 +71,11 @@ test_expect_success 'push -u HEAD' '
check_config headbranch upstream refs/heads/headbranch
'
+test_expect_success 'push -u url' '
+git push -u parent HEAD 2err
+grep no upstream configuration will be set err
+'
+
test_expect_success TTY 'progress messages go to tty' '
ensure_fresh_upstream
--
1.8.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] branch: make sure the upstream remote is configured
On Fri, Jul 26, 2013 at 07:39:37PM +0200, Carlos Martín Nieto wrote:
A command of e.g.
git push --set-upstream /tmp/t master
will call install_branch_config() with a remote name of /tmp/t. This
function will set the 'branch.master.remote' key to, which is
nonsensical as there is no remote by that name.
Is it nonsensical? It does not make sense for the @{upstream} magic
token, because we will not have a branch in tracking branch refs/remotes
to point to. But the configuration would still affect how git pull
chooses a branch to fetch and merge.
I.e., you can currently do:
git push --set-upstream /tmp/t master
git pull ;# pulls from /tmp/t master
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] branch: make sure the upstream remote is configured
On Fri, Jul 26, 2013 at 02:43:11PM -0400, Jeff King wrote:
On Fri, Jul 26, 2013 at 07:39:37PM +0200, Carlos Martín Nieto wrote:
A command of e.g.
git push --set-upstream /tmp/t master
will call install_branch_config() with a remote name of /tmp/t. This
function will set the 'branch.master.remote' key to, which is
nonsensical as there is no remote by that name.
Is it nonsensical? It does not make sense for the @{upstream} magic
token, because we will not have a branch in tracking branch refs/remotes
Eh, I am incapable of typing (and proofreading). That should be not
have a tracking branch in refs/remotes.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] branch: make sure the upstream remote is configured
On Fri, 2013-07-26 at 14:43 -0400, Jeff King wrote:
On Fri, Jul 26, 2013 at 07:39:37PM +0200, Carlos Martín Nieto wrote:
A command of e.g.
git push --set-upstream /tmp/t master
will call install_branch_config() with a remote name of /tmp/t. This
function will set the 'branch.master.remote' key to, which is
nonsensical as there is no remote by that name.
Is it nonsensical? It does not make sense for the @{upstream} magic
token, because we will not have a branch in tracking branch refs/remotes
This was the main point, yes; the only time I've seen it used is by
mistake/misunderstanding, and thinking that you wouldn't want to do
something like what's below.
You are also unable to do this kind of thing through git-branch, and as
it seemed to be an oversight, I wanted to tighten it up.
to point to. But the configuration would still affect how git pull
chooses a branch to fetch and merge.
I.e., you can currently do:
git push --set-upstream /tmp/t master
git pull ;# pulls from /tmp/t master
Interestingly, this actually fetches the right branch from the remote. I
wasn't expecting something like this to work at all.
Somewhat doubtful that this usage is something you'd really want to do,
I see that it does behave properly.
cmn
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] branch: make sure the upstream remote is configured
On Sat, Jul 27, 2013 at 12:29:47AM +0200, Carlos Martín Nieto wrote:
Is it nonsensical? It does not make sense for the @{upstream} magic
token, because we will not have a branch in tracking branch refs/remotes
This was the main point, yes; the only time I've seen it used is by
mistake/misunderstanding, and thinking that you wouldn't want to do
something like what's below.
If that is what you want to prevent, I do not think checking for a named
remote is sufficient. You can also be pushing to a branch on a named
remote that is not part of your fetch refspec, in which case you do not
have a tracking branch. I.e.:
git clone $URL repo.git
cd repo.git
git push --set-upstream HEAD:refs/foo/whatever
For that matter, I wonder what --set-upstream would do if used with
refs/tags/foo. You would not do that in general, but what about:
git push --set-upstream master:master master:v1.0
I didn't test.
to point to. But the configuration would still affect how git pull
chooses a branch to fetch and merge.
I.e., you can currently do:
git push --set-upstream /tmp/t master
git pull ;# pulls from /tmp/t master
Interestingly, this actually fetches the right branch from the remote. I
wasn't expecting something like this to work at all.
Somewhat doubtful that this usage is something you'd really want to do,
I see that it does behave properly.
I do not claim to have used it myself. Tightening the --set-upstream
behavior would not hurt people who want to configure such a thing
manually, and it might catch errors from people doing it accidentally.
So even though the config it generates is not nonsensical, there is a
reasonable chance it was an error, and tightening may make sense. But I
think you would not want the condition to be this is a named remote,
but rather the generated configuration actually has an @{upstream}.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] branch: make sure the upstream remote is configured
On Fri, Jul 26, 2013 at 07:12:11PM -0400, Jeff King wrote: If that is what you want to prevent, I do not think checking for a named remote is sufficient. You can also be pushing to a branch on a named remote that is not part of your fetch refspec, in which case you do not have a tracking branch. I.e.: git clone $URL repo.git cd repo.git git push --set-upstream HEAD:refs/foo/whatever For that matter, I wonder what --set-upstream would do if used with refs/tags/foo. You would not do that in general, but what about: git push --set-upstream master:master master:v1.0 I didn't test. Ah, nevermind. We already catch the case of non-heads (on both the local and remote sides) and abort. So that makes me more confident that your change is a reasonable one; we are already disallowing a subset of what's possible via --set-upstream in the name of preventing weird accidental configurations. This is just fixing another such loophole. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
