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

[Junio C Hamano <gitster@pobox.com>]

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

> When git-clone creates an initial branch it was not
> checking the branch.autosetuprebase configuration
> option (which may exist in ~/.gitconfig).
>
> Added function to branch.c to handle autorebase
> configuration setup.
>
> Signed-off-by: Pat Notz <pknotz@sandia.gov>

Thanks.

Your patch still has two codepaths that set the branch.*.merge and other
information and they can diverge with later changes.  You can refactor
them even more to avoid that risk, like the attached patch.

The test is taken from your patch but runs inside a subshell, so that
later tests others may add to the script will be safer from the
environment variable settings and chdir this test does.

-- >8 --

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.

---
 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 c338910..a5f000a 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 use_local_hardlinks = 1;
@@ -553,7 +541,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");
 	}
 
@@ -583,7 +571,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