threads / discuss / 18246

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

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

## tl;dr

4 messages between Mar 10, 2009 and Mar 10, 2009.

replies: 3people: 2as markdown or json

Pat Notz· Mar 10, 2009, 03:26 UTC · lore

[PATCHv5] Make git-clone respect branch.autosetuprebase

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
Pat Notz· Mar 10, 2009, 04:24 UTC · re: Pat Notz · lore

[PATCHv6] Make git-clone respect branch.autosetuprebase

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
Junio C Hamano· Mar 10, 2009, 08:16 UTC · re: Pat Notz · lore
"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);
 }
 
Junio C Hamano· Mar 10, 2009, 08:20 UTC · re: Junio C Hamano · lore

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

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);
 }
 

← back to recent threads