[PATCH 0/3] fetch: fix '.' fetching

Hi,

It's quite annoying that 'git fetch' tries to fetch '.' when the upstream
branch is a local one. This patch series fixes that, and while on it, it also
fixes 'git push'.

Felipe Contreras (3):
  fetch: add --allow-local option
  fetch: switch allow-local off by default
  remote: disable allow-local for pushes

 builtin/fetch.c         |  6 +++++-
 git-pull.sh             |  2 +-
 remote.c                | 17 ++++++++++++++---
 remote.h                |  1 +
 t/t5513-fetch-track.sh  | 14 ++++++++++++++
 t/t5528-push-default.sh |  7 +++++++
 6 files changed, 42 insertions(+), 5 deletions(-)

-- 
1.8.3.rc1.579.g184e698

So that it becomes possible to override the behavior when the remote
tracked is '.'; if it is, we default back to 'origin'.

To do this, we need to add a new helper fetchremote_get() that accepts
the boolean to enable/disable this behavior.

The default is 'true' which shouldn't cause any change in behavior.

Signed-off-by: Felipe Contreras <redacted>
---
 builtin/fetch.c |  6 +++++-
 remote.c        | 17 ++++++++++++++---
 remote.h        |  1 +
 3 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 4b6b1df..2efbd7b 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c

@@ -39,6 +39,7 @@ static struct strbuf default_rla = STRBUF_INIT;
 static struct transport *transport;
 static const char *submodule_prefix = "";
 static const char *recurse_submodules_default;
+static int allow_local = 1;
 
 static int option_parse_recurse_submodules(const struct option *opt,
 				   const char *arg, int unset)

@@ -90,6 +91,9 @@ static struct option builtin_fetch_options[] = {
 	{ OPTION_STRING, 0, "recurse-submodules-default",
 		   &recurse_submodules_default, NULL,
 		   N_("default mode for recursion"), PARSE_OPT_HIDDEN },
+	{ OPTION_SET_INT, 0, "allow-local", &allow_local, NULL,
+		   N_("allow fetching from local repository"),
+		   PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, 1 },
 	OPT_END()
 };
 

@@ -1006,7 +1010,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 		result = fetch_multiple(&list);
 	} else if (argc == 0) {
 		/* No arguments -- use default remote */
-		remote = remote_get(NULL);
+		remote = fetchremote_get(NULL, allow_local);
 		result = fetch_one(remote, argc, argv);
 	} else if (multiple) {
 		/* All arguments are assumed to be remotes or groups */

diff --git a/remote.c b/remote.c
index 68eb99b..a7e59ab 100644
--- a/remote.c
+++ b/remote.c

@@ -682,7 +682,7 @@ static int valid_remote_nick(const char *name)
 	return !strchr(name, '/'); /* no slash */
 }
 
-static struct remote *remote_get_1(const char *name, const char *pushremote_name)
+static struct remote *remote_get_1(const char *name, const char *pushremote_name, int allow_local)
 {
 	struct remote *ret;
 	int name_given = 0;

@@ -699,6 +699,11 @@ static struct remote *remote_get_1(const char *name, const char *pushremote_name
 		}
 	}
 
+	if (!allow_local && !strcmp(name, ".")) {
+		name = "origin";
+		name_given = 0;
+	}
+
 	ret = make_remote(name, 0);
 	if (valid_remote_nick(name)) {
 		if (!valid_remote(ret))

@@ -718,13 +723,19 @@ static struct remote *remote_get_1(const char *name, const char *pushremote_name
 struct remote *remote_get(const char *name)
 {
 	read_config();
-	return remote_get_1(name, NULL);
+	return remote_get_1(name, NULL, 1);
 }
 
 struct remote *pushremote_get(const char *name)
 {
 	read_config();
-	return remote_get_1(name, pushremote_name);
+	return remote_get_1(name, pushremote_name, 1);
+}
+
+struct remote *fetchremote_get(const char *name, int allow_local)
+{
+	read_config();
+	return remote_get_1(name, NULL, allow_local);
 }
 
 int remote_is_configured(const char *name)

diff --git a/remote.h b/remote.h
index cf56724..f0d6cf3 100644
--- a/remote.h
+++ b/remote.h

@@ -52,6 +52,7 @@ struct remote {
 
 struct remote *remote_get(const char *name);
 struct remote *pushremote_get(const char *name);
+struct remote *fetchremote_get(const char *name, int allow_local);
 int remote_is_configured(const char *name);
 
 typedef int each_remote_fn(struct remote *remote, void *priv);

-- 
1.8.3.rc1.579.g184e698

And force it on in 'git push' to retain the old behavior.

Signed-off-by: Felipe Contreras <redacted>
---
 builtin/fetch.c        |  2 +-
 git-pull.sh            |  2 +-
 t/t5513-fetch-track.sh | 14 ++++++++++++++
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 2efbd7b..c65c75b 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c

@@ -39,7 +39,7 @@ static struct strbuf default_rla = STRBUF_INIT;
 static struct transport *transport;
 static const char *submodule_prefix = "";
 static const char *recurse_submodules_default;
-static int allow_local = 1;
+static int allow_local = 0;
 
 static int option_parse_recurse_submodules(const struct option *opt,
 				   const char *arg, int unset)

diff --git a/git-pull.sh b/git-pull.sh
index 638aabb..18c3793 100755
--- a/git-pull.sh
+++ b/git-pull.sh

@@ -220,7 +220,7 @@ test true = "$rebase" && {
 	done
 }
 orig_head=$(git rev-parse -q --verify HEAD)
-git fetch $verbosity $progress $dry_run $recurse_submodules --update-head-ok "$@" || exit 1
+git fetch $verbosity $progress $dry_run $recurse_submodules --update-head-ok --allow-local "$@" || exit 1
 test -z "$dry_run" || exit 0
 
 curr_head=$(git rev-parse -q --verify HEAD)

diff --git a/t/t5513-fetch-track.sh b/t/t5513-fetch-track.sh
index 65d1e05..cb46747 100755
--- a/t/t5513-fetch-track.sh
+++ b/t/t5513-fetch-track.sh

@@ -27,4 +27,18 @@ test_expect_success fetch '
 	)
 '
 
+test_expect_success 'fetch no-local' '
+	(
+		test_create_repo another &&
+		cd another &&
+		git remote add origin .. &&
+		echo test > file &&
+		git add . &&
+		git commit -m test &&
+		git checkout -t -b local-tracking master &&
+		git fetch &&
+		git rev-parse --verify refs/remotes/origin/b/one
+	)
+'
+
 test_done

-- 
1.8.3.rc1.579.g184e698

So that 'git push' uses 'origin', instead of '.' by default.

Signed-off-by: Felipe Contreras <redacted>
---
 remote.c                | 2 +-
 t/t5528-push-default.sh | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/remote.c b/remote.c
index a7e59ab..c1458dc 100644
--- a/remote.c
+++ b/remote.c

@@ -729,7 +729,7 @@ struct remote *remote_get(const char *name)
 struct remote *pushremote_get(const char *name)
 {
 	read_config();
-	return remote_get_1(name, pushremote_name, 1);
+	return remote_get_1(name, pushremote_name, 0);
 }
 
 struct remote *fetchremote_get(const char *name, int allow_local)

diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh
index 4736da8..61df2a7 100755
--- a/t/t5528-push-default.sh
+++ b/t/t5528-push-default.sh

@@ -115,4 +115,11 @@ test_expect_success 'push to existing branch, upstream configured with different
 	test_cmp expect-other-name actual-other-name
 '
 
+test_expect_success 'push to existing branch, upstream configured with same name' '
+	git remote add origin repo1 &&
+	git checkout -t -b local-tracking master &&
+	test_commit ten &&
+	test_push_success current local-tracking
+'
+
 test_done

-- 
1.8.3.rc1.579.g184e698

Felipe Contreras wrote:

So that it becomes possible to override the behavior when the remote
tracked is '.'; if it is, we default back to 'origin'.

What is the problem you're trying to solve?  Why do you have
branch.<name>.remote set to '.' in the first place, if you meant
origin?  'git fetch .' currently just updates FETCH_HEAD; while I'm
not sure how that is useful, I still don't understand _why_ you want
to change that behavior.

On Thu, May 16, 2013 at 3:25 AM, Ramkumar Ramachandra
[off-list ref] wrote:

Felipe Contreras wrote:

quoted

So that it becomes possible to override the behavior when the remote
tracked is '.'; if it is, we default back to 'origin'.

What is the problem you're trying to solve?  Why do you have
branch.<name>.remote set to '.' in the first place, if you meant
origin?  'git fetch .' currently just updates FETCH_HEAD; while I'm
not sure how that is useful, I still don't understand _why_ you want
to change that behavior.

% git checkout -t -b devel master

-- 
Felipe Contreras

Felipe Contreras wrote:

% git checkout -t -b devel master

Interesting.  Have you considered changing -t to inherit the parent
branch's remote?  (Would everyone like that?)

On Thu, May 16, 2013 at 4:27 AM, Ramkumar Ramachandra
[off-list ref] wrote:

Felipe Contreras wrote:

quoted

% git checkout -t -b devel master

Interesting.  Have you considered changing -t to inherit the parent
branch's remote?  (Would everyone like that?)

Why would I do that? When I do 'git rebase' I want to rebase on top of
'master', not 'origin/master' (or whatever the upstream of 'master'
is).

-- 
Felipe Contreras

Felipe Contreras wrote:

Why would I do that? When I do 'git rebase' I want to rebase on top of
'master', not 'origin/master' (or whatever the upstream of 'master'
is).

Ah, so you want @{u} to point to refs/heads/master, but want to modify
fetch to act on the hard-coded "origin", not @{u} (wouldn't you like
to be able to configure this?).  Seems a bit yuck overall; I wonder if
there's some other way to achieve what you want.

On Thu, May 16, 2013 at 4:58 AM, Ramkumar Ramachandra
[off-list ref] wrote:

Felipe Contreras wrote:

quoted

Why would I do that? When I do 'git rebase' I want to rebase on top of
'master', not 'origin/master' (or whatever the upstream of 'master'
is).

Ah, so you want @{u} to point to refs/heads/master, but want to modify
fetch to act on the hard-coded "origin", not @{u} (wouldn't you like
to be able to configure this?).

No. What's the point of 'git fetch .'? What does 'git fetch' does when
there's no configured upstream branch? Why doesn't 'git fetch' default
to 'git fetch .' in those cases?

Answer: because 'git fetch .' doesn't make any sense. So if
'branch.HEAD.remote' is '.' it doesn't make sense to do 'git fetch .'.

Seems a bit yuck overall; I wonder if
there's some other way to achieve what you want.

Yeah, add 'branch.A.base' that would be used only by 'git rebase',
which I already suggested before, but I changed my mind.

Fixing 'git fetch' makes much more sense.

-- 
Felipe Contreras

Felipe Contreras wrote:

Answer: because 'git fetch .' doesn't make any sense. So if
'branch.HEAD.remote' is '.' it doesn't make sense to do 'git fetch .'.

I agree that 'git fetch .' is currently not useful (and I am not
against changing its behavior), but my question pertains to why you
are replacing it with the hard-coded "origin".  What happens when I
git branch -t -b devel hot-branch where branch.hot-branch.remote = ram
and not origin?

On Thu, May 16, 2013 at 5:32 AM, Ramkumar Ramachandra
[off-list ref] wrote:

Felipe Contreras wrote:

quoted

Answer: because 'git fetch .' doesn't make any sense. So if
'branch.HEAD.remote' is '.' it doesn't make sense to do 'git fetch .'.

I agree that 'git fetch .' is currently not useful (and I am not
against changing its behavior), but my question pertains to why you
are replacing it with the hard-coded "origin".

'origin' is already hard-coded.

% git clone git://git.kernel.org/pub/scm/git/git.git

What would be the name of the remote? 'origin'.

% git checkout --no-track -b test
% git fetch

What is the remote that is used? 'origin'.

What happens when I
git branch -t -b devel hot-branch where branch.hot-branch.remote = ram
and not origin?

The same thing that happens when you git branch --no-track -b devel hot-branch.

-- 
Felipe Contreras