[PATCHv5] Make git-clone respect branch.autosetuprebase

[PATCHv5] Make git-clone respect branch.autosetuprebase

[Pat Notz]

From: Junio C Hamano <gitster@pobox.com>

When git-clone creates an initial branch it was not checking the
branch.autosetuprebase configuration option (which may exist in
~/.gitconfig).  Refactor the code used by "git branch" to create
a new branch, and use it instead of the insufficiently duplicated code
in builtin-clone.

Minor modification of the patch from Junio Hamano.
---

This version fixes the verbose output to be more human friendly.  Before,
the branch being tracked was printed as 'refs/heads/frotz' regardless of
wether that was a local or remote branch.  Now, a local branch is just
printed as 'frotz' and a remote branch is printed as 'origin/frotz'

 branch.c         |   49 +++++++++++++++++++++++++++++++++----------------
 branch.h         |    7 +++++++
 builtin-clone.c  |   18 +++---------------
 t/t5601-clone.sh |   15 +++++++++++++++
 4 files changed, 58 insertions(+), 31 deletions(-)

diff --git a/branch.c b/branch.c
index 1f00e44..d20fb04 100644
--- a/branch.c
+++ b/branch.c
@@ -32,21 +32,48 @@ static int find_tracked_branch(struct remote *remote, void *priv)
 	return 0;
 }
 
-static int should_setup_rebase(const struct tracking *tracking)
+static int should_setup_rebase(const char *origin)
 {
 	switch (autorebase) {
 	case AUTOREBASE_NEVER:
 		return 0;
 	case AUTOREBASE_LOCAL:
-		return tracking->remote == NULL;
+		return origin == NULL;
 	case AUTOREBASE_REMOTE:
-		return tracking->remote != NULL;
+		return origin != NULL;
 	case AUTOREBASE_ALWAYS:
 		return 1;
 	}
 	return 0;
 }
 
+void install_branch_config(int flag, const char *local, const char *origin, const char *remote)
+{
+	struct strbuf key = STRBUF_INIT;
+	int rebasing = should_setup_rebase(origin);
+
+	strbuf_addf(&key, "branch.%s.remote", local);
+	git_config_set(key.buf, origin ? origin : ".");
+
+	strbuf_reset(&key);
+	strbuf_addf(&key, "branch.%s.merge", local);
+	git_config_set(key.buf, remote);
+
+	if (rebasing) {
+		strbuf_reset(&key);
+		strbuf_addf(&key, "branch.%s.rebase", local);
+		git_config_set(key.buf, "true");
+	}
+
+	if (flag & BRANCH_CONFIG_VERBOSE)
+		printf("Branch %s set up to track %s branch %s %s.\n",
+		       local,
+		       origin ? "remote" : "local",
+		       remote,
+		       rebasing ? "by rebasing" : "by merging");
+	strbuf_release(&key);
+}
+
 /*
  * This is called when new_ref is branched off of orig_ref, and tries
  * to infer the settings for branch.<new_ref>.{remote,merge} from the
@@ -55,7 +82,6 @@ static int should_setup_rebase(const struct tracking *tracking)
 static int setup_tracking(const char *new_ref, const char *orig_ref,
                           enum branch_track track)
 {
-	char key[1024];
 	struct tracking tracking;
 
 	if (strlen(new_ref) > 1024 - 7 - 7 - 1)
@@ -80,19 +106,10 @@ static int setup_tracking(const char *new_ref, const char *orig_ref,
 		return error("Not tracking: ambiguous information for ref %s",
 				orig_ref);
 
-	sprintf(key, "branch.%s.remote", new_ref);
-	git_config_set(key, tracking.remote ?  tracking.remote : ".");
-	sprintf(key, "branch.%s.merge", new_ref);
-	git_config_set(key, tracking.src ? tracking.src : orig_ref);
-	printf("Branch %s set up to track %s branch %s.\n", new_ref,
-		tracking.remote ? "remote" : "local", orig_ref);
-	if (should_setup_rebase(&tracking)) {
-		sprintf(key, "branch.%s.rebase", new_ref);
-		git_config_set(key, "true");
-		printf("This branch will rebase on pull.\n");
-	}
-	free(tracking.src);
+	install_branch_config(BRANCH_CONFIG_VERBOSE, new_ref, tracking.remote,
+			      tracking.src ? tracking.src : orig_ref);
 
+	free(tracking.src);
 	return 0;
 }
 
diff --git a/branch.h b/branch.h
index 9f0c2a2..eed817a 100644
--- a/branch.h
+++ b/branch.h
@@ -21,4 +21,11 @@ void create_branch(const char *head, const char *name, const char *start_name,
  */
 void remove_branch_state(void);
 
+/*
+ * Configure local branch "local" to merge remote branch "remote"
+ * taken from origin "origin".
+ */
+#define BRANCH_CONFIG_VERBOSE 01
+extern void install_branch_config(int flag, const char *local, const char *origin, const char *remote);
+
 #endif
diff --git a/builtin-clone.c b/builtin-clone.c
index 92826cd..687df9a 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -20,6 +20,7 @@
 #include "dir.h"
 #include "pack-refs.h"
 #include "sigchain.h"
+#include "branch.h"
 
 /*
  * Overall FIXMEs:
@@ -350,19 +351,6 @@ static struct ref *write_remote_refs(const struct ref *refs,
 	return local_refs;
 }
 
-static void install_branch_config(const char *local,
-				  const char *origin,
-				  const char *remote)
-{
-	struct strbuf key = STRBUF_INIT;
-	strbuf_addf(&key, "branch.%s.remote", local);
-	git_config_set(key.buf, origin);
-	strbuf_reset(&key);
-	strbuf_addf(&key, "branch.%s.merge", local);
-	git_config_set(key.buf, remote);
-	strbuf_release(&key);
-}
-
 int cmd_clone(int argc, const char **argv, const char *prefix)
 {
 	int is_bundle = 0;
@@ -547,7 +535,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		remote_head = NULL;
 		option_no_checkout = 1;
 		if (!option_bare)
-			install_branch_config("master", option_origin,
+			install_branch_config(0, "master", option_origin,
 					      "refs/heads/master");
 	}
 
@@ -577,7 +565,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 				      head_points_at->peer_ref->name,
 				      reflog_msg.buf);
 
-			install_branch_config(head, option_origin,
+			install_branch_config(0, head, option_origin,
 					      head_points_at->name);
 		}
 	} else if (remote_head) {
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 44793f2..2335d8b 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -159,4 +159,19 @@ test_expect_success 'clone a void' '
 	test_cmp target-6/.git/config target-7/.git/config
 '
 
+test_expect_success 'clone respects global branch.autosetuprebase' '
+	(
+		HOME=$(pwd) &&
+		export HOME &&
+		test_config="$HOME/.gitconfig" &&
+		unset GIT_CONFIG_NOGLOBAL &&
+		git config -f "$test_config" branch.autosetuprebase remote &&
+		rm -fr dst &&
+		git clone src dst &&
+		cd dst &&
+		actual="z$(git config branch.master.rebase)" &&
+		test ztrue = $actual
+	)
+'
+
 test_done
-- 
1.6.2

[PATCHv6] Make git-clone respect branch.autosetuprebase

[Pat Notz]

From: Junio C Hamano <gitster@pobox.com>

When git-clone creates an initial branch it was not checking the
branch.autosetuprebase configuration option (which may exist in
~/.gitconfig).  Refactor the code used by "git branch" to create
a new branch, and use it instead of the insufficiently duplicated code
in builtin-clone.

Minor modification of the patch from Junio Hamano.
---

Ugh.  Another resend... I need to stop working at night.

This version fixes the verbose output to be more human friendly.  Before,
the branch being tracked was printed as 'refs/heads/frotz' regardless of
wether that was a local or remote branch.  Now, a local branch is just
printed as 'frotz' and a remote branch is printed as 'origin/frotz'

 branch.c         |   55 ++++++++++++++++++++++++++++++++++++++---------------
 branch.h         |    7 ++++++
 builtin-clone.c  |   18 ++--------------
 t/t5601-clone.sh |   15 ++++++++++++++
 4 files changed, 64 insertions(+), 31 deletions(-)

diff --git a/branch.c b/branch.c
index 1f00e44..eecda64 100644
--- a/branch.c
+++ b/branch.c
@@ -32,21 +32,54 @@ static int find_tracked_branch(struct remote *remote, void *priv)
 	return 0;
 }
 
-static int should_setup_rebase(const struct tracking *tracking)
+static int should_setup_rebase(const char *origin)
 {
 	switch (autorebase) {
 	case AUTOREBASE_NEVER:
 		return 0;
 	case AUTOREBASE_LOCAL:
-		return tracking->remote == NULL;
+		return origin == NULL;
 	case AUTOREBASE_REMOTE:
-		return tracking->remote != NULL;
+		return origin != NULL;
 	case AUTOREBASE_ALWAYS:
 		return 1;
 	}
 	return 0;
 }
 
+void install_branch_config(int flag, const char *local, const char *origin, const char *remote)
+{
+	struct strbuf key = STRBUF_INIT;
+	int rebasing = should_setup_rebase(origin);
+
+	strbuf_addf(&key, "branch.%s.remote", local);
+	git_config_set(key.buf, origin ? origin : ".");
+
+	strbuf_reset(&key);
+	strbuf_addf(&key, "branch.%s.merge", local);
+	git_config_set(key.buf, remote);
+
+	if (rebasing) {
+		strbuf_reset(&key);
+		strbuf_addf(&key, "branch.%s.rebase", local);
+		git_config_set(key.buf, "true");
+	}
+
+	if (flag & BRANCH_CONFIG_VERBOSE){
+		strbuf_reset(&key);
+		if(origin)
+			strbuf_addf(&key, "%s/%s", origin, remote+11);
+		else
+			strbuf_addf(&key, "%s", remote+11);
+		printf("Branch %s set up to track %s branch %s %s.\n",
+		       local,
+		       origin ? "remote" : "local",
+		       key.buf,
+		       rebasing ? "by rebasing" : "by merging");
+	}
+	strbuf_release(&key);
+}
+
 /*
  * This is called when new_ref is branched off of orig_ref, and tries
  * to infer the settings for branch.<new_ref>.{remote,merge} from the
@@ -55,7 +88,6 @@ static int should_setup_rebase(const struct tracking *tracking)
 static int setup_tracking(const char *new_ref, const char *orig_ref,
                           enum branch_track track)
 {
-	char key[1024];
 	struct tracking tracking;
 
 	if (strlen(new_ref) > 1024 - 7 - 7 - 1)
@@ -80,19 +112,10 @@ static int setup_tracking(const char *new_ref, const char *orig_ref,
 		return error("Not tracking: ambiguous information for ref %s",
 				orig_ref);
 
-	sprintf(key, "branch.%s.remote", new_ref);
-	git_config_set(key, tracking.remote ?  tracking.remote : ".");
-	sprintf(key, "branch.%s.merge", new_ref);
-	git_config_set(key, tracking.src ? tracking.src : orig_ref);
-	printf("Branch %s set up to track %s branch %s.\n", new_ref,
-		tracking.remote ? "remote" : "local", orig_ref);
-	if (should_setup_rebase(&tracking)) {
-		sprintf(key, "branch.%s.rebase", new_ref);
-		git_config_set(key, "true");
-		printf("This branch will rebase on pull.\n");
-	}
-	free(tracking.src);
+	install_branch_config(BRANCH_CONFIG_VERBOSE, new_ref, tracking.remote,
+			      tracking.src ? tracking.src : orig_ref);
 
+	free(tracking.src);
 	return 0;
 }
 
diff --git a/branch.h b/branch.h
index 9f0c2a2..eed817a 100644
--- a/branch.h
+++ b/branch.h
@@ -21,4 +21,11 @@ void create_branch(const char *head, const char *name, const char *start_name,
  */
 void remove_branch_state(void);
 
+/*
+ * Configure local branch "local" to merge remote branch "remote"
+ * taken from origin "origin".
+ */
+#define BRANCH_CONFIG_VERBOSE 01
+extern void install_branch_config(int flag, const char *local, const char *origin, const char *remote);
+
 #endif
diff --git a/builtin-clone.c b/builtin-clone.c
index 92826cd..687df9a 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -20,6 +20,7 @@
 #include "dir.h"
 #include "pack-refs.h"
 #include "sigchain.h"
+#include "branch.h"
 
 /*
  * Overall FIXMEs:
@@ -350,19 +351,6 @@ static struct ref *write_remote_refs(const struct ref *refs,
 	return local_refs;
 }
 
-static void install_branch_config(const char *local,
-				  const char *origin,
-				  const char *remote)
-{
-	struct strbuf key = STRBUF_INIT;
-	strbuf_addf(&key, "branch.%s.remote", local);
-	git_config_set(key.buf, origin);
-	strbuf_reset(&key);
-	strbuf_addf(&key, "branch.%s.merge", local);
-	git_config_set(key.buf, remote);
-	strbuf_release(&key);
-}
-
 int cmd_clone(int argc, const char **argv, const char *prefix)
 {
 	int is_bundle = 0;
@@ -547,7 +535,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		remote_head = NULL;
 		option_no_checkout = 1;
 		if (!option_bare)
-			install_branch_config("master", option_origin,
+			install_branch_config(0, "master", option_origin,
 					      "refs/heads/master");
 	}
 
@@ -577,7 +565,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 				      head_points_at->peer_ref->name,
 				      reflog_msg.buf);
 
-			install_branch_config(head, option_origin,
+			install_branch_config(0, head, option_origin,
 					      head_points_at->name);
 		}
 	} else if (remote_head) {
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 44793f2..2335d8b 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -159,4 +159,19 @@ test_expect_success 'clone a void' '
 	test_cmp target-6/.git/config target-7/.git/config
 '
 
+test_expect_success 'clone respects global branch.autosetuprebase' '
+	(
+		HOME=$(pwd) &&
+		export HOME &&
+		test_config="$HOME/.gitconfig" &&
+		unset GIT_CONFIG_NOGLOBAL &&
+		git config -f "$test_config" branch.autosetuprebase remote &&
+		rm -fr dst &&
+		git clone src dst &&
+		cd dst &&
+		actual="z$(git config branch.master.rebase)" &&
+		test ztrue = $actual
+	)
+'
+
 test_done
-- 
1.6.2

Re: [PATCHv6] Make git-clone respect branch.autosetuprebase

[Junio C Hamano]

"Pat Notz" <pknotz@sandia.gov> writes:

> This version fixes the verbose output to be more human friendly.  Before,
> the branch being tracked was printed as 'refs/heads/frotz' regardless of
> wether that was a local or remote branch.  Now, a local branch is just
> printed as 'frotz' and a remote branch is printed as 'origin/frotz'

Since I've already queued the previous round in 'next', I took the liberty
of making this into an incremental.

I do not think what you did is really correct.  For clone, because we do
not create anything but the default "refs/remotes/<origin>/<branch>"
layout, always stripping out the first 11 bytes "refs/heads/" happens to
give a more intuitive result.  But the helper function is shared with a
more general "git branch --track" that lets you mark the new branch to
track almost anything.  For example, the version from v1.6.0 gives you
these:

    $ git branch --track maint-1.6.0 v1.6.0
    Branch maint-1.6.0 set up to track local branch refs/tags/v1.6.0
    $ git branch --track frotz master
    Branch frotz set up to track local branch refs/heads/master

Arguably, the former should say "track local ref refs/tags/v1.6.0" (or the
command should not even let you "track" a tag, which is supposed to be
immutable), and the latter would be helped by stripping "refs/heads/".
But the point is that stripping first 11 bytes unconditionally would be
wrong if you happen to start tracking a tag whose name is a single byte.

Also I do not think ("%s/%s", origin, remote+11) is correct in general.
You could very well configure your tracking to:

	[remote "origin"]
	fetch = refs/heads/*:refs/remotes/upstream/*

and I think the variable "remote" here refer to the name the ref is known
as in the remote repository, not the name we use to keep a copy of.  IOW,
when we say "Branch frotz tracks remote 'master'", we should say "the
branch known as 'master' at the remote site, no matter what name we happen
to call its copy locally somewhere under our refs/remotes".

In that sense, both the version I queued and the original are lacking
a critical piece of information: which remote we are talking about.

In any case, here is your patch, made into an incremental.  I'll send out
an alternative fix in a separate message.

 branch.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/branch.c b/branch.c
index d20fb04..d586843 100644
--- a/branch.c
+++ b/branch.c
@@ -65,12 +65,18 @@ void install_branch_config(int flag, const char *local, const char *origin, cons
 		git_config_set(key.buf, "true");
 	}
 
-	if (flag & BRANCH_CONFIG_VERBOSE)
+	if (flag & BRANCH_CONFIG_VERBOSE) {
+		strbuf_reset(&key);
+		if (origin)
+			strbuf_addf(&key, "%s/%s", origin, remote+11);
+		else
+			strbuf_addf(&key, "%s", remote+11);
 		printf("Branch %s set up to track %s branch %s %s.\n",
 		       local,
 		       origin ? "remote" : "local",
-		       remote,
+		       key.buf,
 		       rebasing ? "by rebasing" : "by merging");
+	}
 	strbuf_release(&key);
 }
 

[PATCH] Improve "git branch --tracking" output

[Junio C Hamano]

An earlier patch always spelled the full name of the ref that we track
(e.g. "refs/heads/frotz" instead of just "frotz" when we mean the branch
whose name is "frotz").  Worse yet, because we now use the true name of
the ref at the original repository when talk about a tracking branch that
copies from a remote, such a full name alone still does not give enough
information.

This reorganizes the verbose codepath to:

 - differentiate "refs/heads/something" and everything else; we say that
   the branch tracks "branch <something>" if it begins with "refs/heads/",
   and otherwise the branch tracks "ref refs/<someother>/<something>";

 - report the name of the remote when we talk about a tracking branch, by
   saying "branch frotz from origin";

 - not say "by merging" at the end; it is the default and is not worth
   reporting.

Signed-off-by: Junio C Hamano <junio@pobox.com>
---
 branch.c |   23 +++++++++++++++++------
 1 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/branch.c b/branch.c
index d20fb04..5f889fe 100644
--- a/branch.c
+++ b/branch.c
@@ -65,12 +65,23 @@ void install_branch_config(int flag, const char *local, const char *origin, cons
 		git_config_set(key.buf, "true");
 	}
 
-	if (flag & BRANCH_CONFIG_VERBOSE)
-		printf("Branch %s set up to track %s branch %s %s.\n",
-		       local,
-		       origin ? "remote" : "local",
-		       remote,
-		       rebasing ? "by rebasing" : "by merging");
+	if (flag & BRANCH_CONFIG_VERBOSE) {
+		strbuf_reset(&key);
+
+		strbuf_addstr(&key, origin ? "remote" : "local");
+
+		/* Are we tracking a proper "branch"? */
+		if (!prefixcmp(remote, "refs/heads/")) {
+			strbuf_addf(&key, " branch %s", remote + 11);
+			if (origin)
+				strbuf_addf(&key, " from %s", origin);
+		}
+		else
+			strbuf_addf(&key, " ref %s", remote);
+		printf("Branch %s set up to track %s%s.\n",
+		       local, key.buf,
+		       rebasing ? " by rebasing" : "");
+	}
 	strbuf_release(&key);
 }