[PATCH] branch: optionally setup branch.*.merge from upstream local branches

[PATCH] branch: optionally setup branch.*.merge from upstream local branches

[Jay Soffian]

git branch / git checkout -b now honor --track when the upstream branch
is local. Previously --track was silently ignored for local upstream
branches.

The configuration setting branch.autosetupmerge can now be set to
"always" which is equivalent to using --track. Setting
branch.autosetupmerge to boolean true will retains the former behavior
of only setting up branch.*.merge for remote upstream branches.

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
On Feb 16, 2008 6:45 AM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi,
> 
> On Fri, 15 Feb 2008, Junio C Hamano wrote:
> 
> > This area was last cleaned up in 6f084a5 (branch --track: code cleanup
> > and saner handling of local branches).  I do not know if the original
> > intention of the code was to allow a hack like this to work, or it is
> > just an unintended accident that it happens to work.  Dscho, any ideas?
> 
> AFAIR the problem was that you were (rightfully) annoyed when you were
> setting up your local branches, and all of a sudden, they were set up with
> loads of tracking information, cluttering your config.
> 
> Basically, that is the reason why we disallowed tracking information to be
> set up for local branching.

Hopefully this is an acceptable compromise? Junio suggested making this 
implicit:

   [remote "."]
      fetch = refs/*:refs/*

But I thought that might affect other areas, and I don't think this is too bad.

 branch.c           |   12 ++++++++++--
 branch.h           |    8 +++++++-
 builtin-branch.c   |   11 ++++++++---
 builtin-checkout.c |   15 ++++++++++-----
 4 files changed, 35 insertions(+), 11 deletions(-)

diff --git a/branch.c b/branch.c
index 1fc8788..24e98dd 100644
--- a/branch.c
+++ b/branch.c
@@ -71,7 +71,7 @@ static int setup_tracking(const char *new_ref, const char *orig_ref)
 
 void create_branch(const char *head,
 		   const char *name, const char *start_name,
-		   int force, int reflog, int track)
+		   int force, int reflog, enum branch_track track)
 {
 	struct ref_lock *lock;
 	struct commit *commit;
@@ -130,7 +130,15 @@ void create_branch(const char *head,
 	   automatically merges from there.  So far, this is only done for
 	   remotes registered via .git/config.  */
 	if (real_ref && track)
-		setup_tracking(name, real_ref);
+		if (setup_tracking(name, real_ref) == 1 && track == BRANCH_TRACK_ALWAYS) {
+			char key[1024];
+			sprintf(key, "branch.%s.remote", name);
+			git_config_set(key, ".");
+			sprintf(key, "branch.%s.merge", name);
+			git_config_set(key, real_ref);
+			printf("Branch %s set up with upstream branch %s.\n",
+			       name, real_ref);
+		}
 
 	if (write_ref_sha1(lock, sha1, msg) < 0)
 		die("Failed to write ref: %s.", strerror(errno));
diff --git a/branch.h b/branch.h
index d30abe0..771d6ce 100644
--- a/branch.h
+++ b/branch.h
@@ -1,6 +1,12 @@
 #ifndef BRANCH_H
 #define BRANCH_H
 
+enum branch_track {
+	BRANCH_TRACK_FALSE = 0,
+	BRANCH_TRACK_TRUE = 1,
+	BRANCH_TRACK_ALWAYS = 2,
+};
+
 /* Functions for acting on the information about branches. */
 
 /*
@@ -13,7 +19,7 @@
  * branch for (if any).
  */
 void create_branch(const char *head, const char *name, const char *start_name,
-		   int force, int reflog, int track);
+		   int force, int reflog, enum branch_track track);
 
 /*
  * Remove information about the state of working on the current
diff --git a/builtin-branch.c b/builtin-branch.c
index 5094e0d..231bff4 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -30,7 +30,7 @@ static const char * const builtin_branch_usage[] = {
 static const char *head;
 static unsigned char head_sha1[20];
 
-static int branch_track = 1;
+static enum branch_track branch_track = BRANCH_TRACK_FALSE;
 
 static int branch_use_color;
 static char branch_colors[][COLOR_MAXLEN] = {
@@ -77,6 +77,10 @@ static int git_branch_config(const char *var, const char *value)
 		return 0;
 	}
 	if (!strcmp(var, "branch.autosetupmerge")) {
+		if (value && !strcasecmp(value, "always")) {
+			branch_track = BRANCH_TRACK_ALWAYS;
+			return 0;
+		}
 		branch_track = git_config_bool(var, value);
 		return 0;
 	}
@@ -420,14 +424,15 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 {
 	int delete = 0, rename = 0, force_create = 0;
 	int verbose = 0, abbrev = DEFAULT_ABBREV, detached = 0;
-	int reflog = 0, track;
+	int reflog = 0;
+	enum branch_track track;
 	int kinds = REF_LOCAL_BRANCH;
 	struct commit_list *with_commit = NULL;
 
 	struct option options[] = {
 		OPT_GROUP("Generic options"),
 		OPT__VERBOSE(&verbose),
-		OPT_BOOLEAN( 0 , "track",  &track, "set up tracking mode (see git-pull(1))"),
+		OPT_SET_INT( 0 , "track",  &track, "set up tracking mode (see git-pull(1))", BRANCH_TRACK_ALWAYS),
 		OPT_BOOLEAN( 0 , "color",  &branch_use_color, "use colored output"),
 		OPT_SET_INT('r', NULL,     &kinds, "act on remote-tracking branches",
 			REF_REMOTE_BRANCH),
diff --git a/builtin-checkout.c b/builtin-checkout.c
index 0d19835..d77ee9c 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -186,7 +186,7 @@ struct checkout_opts {
 
 	char *new_branch;
 	int new_branch_log;
-	int track;
+	enum branch_track track;
 };
 
 struct branch_info {
@@ -521,13 +521,18 @@ static int switch_branches(struct checkout_opts *opts,
 	return post_checkout_hook(old.commit, new->commit, 1);
 }
 
-static int branch_track = 0;
+static enum branch_track branch_track = BRANCH_TRACK_FALSE;
 
 static int git_checkout_config(const char *var, const char *value)
 {
-	if (!strcmp(var, "branch.autosetupmerge"))
+	if (!strcmp(var, "branch.autosetupmerge")) {
+		if (value && !strcasecmp(value, "always")) {
+			branch_track = BRANCH_TRACK_ALWAYS;
+			return 0;
+		}
 		branch_track = git_config_bool(var, value);
-
+		return 0;
+	}
 	return git_default_config(var, value);
 }
 
@@ -542,7 +547,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		OPT__QUIET(&opts.quiet),
 		OPT_STRING('b', NULL, &opts.new_branch, "new branch", "branch"),
 		OPT_BOOLEAN('l', NULL, &opts.new_branch_log, "log for new branch"),
-		OPT_BOOLEAN( 0 , "track", &opts.track, "track"),
+		OPT_SET_INT( 0 , "track",  &opts.track, "track", BRANCH_TRACK_ALWAYS),
 		OPT_BOOLEAN('f', NULL, &opts.force, "force"),
 		OPT_BOOLEAN('m', NULL, &opts.merge, "merge"),
 		OPT_END(),
-- 
1.5.4.2.183.g9fe5b

Re: [PATCH] branch: optionally setup branch.*.merge from upstream local branches

[Johannes Schindelin]

Hi,

On Mon, 18 Feb 2008, Jay Soffian wrote:

> diff --git a/branch.h b/branch.h
> index d30abe0..771d6ce 100644
> --- a/branch.h
> +++ b/branch.h
> @@ -1,6 +1,12 @@
>  #ifndef BRANCH_H
>  #define BRANCH_H
>  
> +enum branch_track {
> +	BRANCH_TRACK_FALSE = 0,
> +	BRANCH_TRACK_TRUE = 1,

BRANCH_TRACK_REMOTES would be a better name here.  And you do not need the 
"= 1" and "= 2".

> +	BRANCH_TRACK_ALWAYS = 2,
> +};
> +
>  /* Functions for acting on the information about branches. */
>  
>  /*
> diff --git a/builtin-branch.c b/builtin-branch.c
> index 5094e0d..231bff4 100644
> --- a/builtin-branch.c
> +++ b/builtin-branch.c
> @@ -30,7 +30,7 @@ static const char * const builtin_branch_usage[] = {
>  static const char *head;
>  static unsigned char head_sha1[20];
>  
> -static int branch_track = 1;
> +static enum branch_track branch_track = BRANCH_TRACK_FALSE;

That is a clear regression.

> @@ -77,6 +77,10 @@ static int git_branch_config(const char *var, const char *value)
>  		return 0;
>  	}
>  	if (!strcmp(var, "branch.autosetupmerge")) {
> +		if (value && !strcasecmp(value, "always")) {
> +			branch_track = BRANCH_TRACK_ALWAYS;
> +			return 0;
> +		}
>  		branch_track = git_config_bool(var, value);
>  		return 0;
>  	}

You have this in builtin-branch.c and builtin-checkout.c.  Duplicated 
code.  IMHO it is time to move this into the git_default_config() function 
(with "branch_track" being renamed to "git_branch_track", and moved to 
environment.c).

Personally, I have no problem with typing "git merge <branch>" in your 
workflow.  I would even avoid saying "git pull" for obviously-local 
branches, because I would have forgotten which branch it tracked 
originally.

But hey, if you really want to... ;-)

Ciao,
Dscho

Re: [PATCH] branch: optionally setup branch.*.merge from upstream local branches

[Jay Soffian]

On Feb 18, 2008 7:14 AM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

> On Mon, 18 Feb 2008, Jay Soffian wrote:
>
> > +enum branch_track {
> > +     BRANCH_TRACK_FALSE = 0,
> > +     BRANCH_TRACK_TRUE = 1,
>
> BRANCH_TRACK_REMOTES would be a better name here.  And you do not need the
> "= 1" and "= 2".

I was just following along. Plenty of enum examples in the current code
do this. Am I missing something subtle about when assigning explicit
values should be done?

> >
> > -static int branch_track = 1;
> > +static enum branch_track branch_track = BRANCH_TRACK_FALSE;
>
> That is a clear regression.

Perhaps. It's consistent with builtin-checkout.c though (which was
initializing it to 0). Who to believe?

> > @@ -77,6 +77,10 @@ static int git_branch_config(const char *var, const char *value)
> >               return 0;
> >       }
> >       if (!strcmp(var, "branch.autosetupmerge")) {
> > +             if (value && !strcasecmp(value, "always")) {
> > +                     branch_track = BRANCH_TRACK_ALWAYS;
> > +                     return 0;
> > +             }
> >               branch_track = git_config_bool(var, value);
> >               return 0;
> >       }
>
> You have this in builtin-branch.c and builtin-checkout.c.  Duplicated
> code.  IMHO it is time to move this into the git_default_config() function
> (with "branch_track" being renamed to "git_branch_track", and moved to
> environment.c).

Mkay, builtin-checkout.c author didn't do it, so...

> Personally, I have no problem with typing "git merge <branch>" in your
> workflow.  I would even avoid saying "git pull" for obviously-local
> branches, because I would have forgotten which branch it tracked
> originally.

Um, well, apply this patch, set branch.autosetupmerge=always and then
branch.*.merge will tell you which branch it tracked originally. :-)

Aside, then how do you figure out the upstream branch is if you've
forgotten?

j.

[PATCH] branch: optionally setup branch.*.merge from upstream local branches

[Jay Soffian]

git branch / git checkout -b now honor --track when the upstream branch
is local. Previously --track was silently ignored for local upstream
branches.

The configuration setting branch.autosetupmerge can now be set to
"always" which is equivalent to using --track. Setting
branch.autosetupmerge to boolean true will retains the former behavior
of only setting up branch.*.merge for remote upstream branches.

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
Incorporated Johannes' feedback and updated documentation as well.

 Documentation/config.txt       |   13 ++++++++-----
 Documentation/git-branch.txt   |   24 ++++++++++++------------
 Documentation/git-checkout.txt |   25 ++++++++++++-------------
 branch.c                       |   12 ++++++++++--
 branch.h                       |    2 +-
 builtin-branch.c               |   13 ++++---------
 builtin-checkout.c             |   13 ++++---------
 cache.h                        |    8 ++++++++
 config.c                       |    8 ++++++++
 environment.c                  |    1 +
 10 files changed, 68 insertions(+), 51 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index fb6dae0..f15fe0a 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -378,11 +378,14 @@ apply.whitespace::
 	as the '--whitespace' option. See linkgit:git-apply[1].
 
 branch.autosetupmerge::
-	Tells `git-branch` and `git-checkout` to setup new branches
-	so that linkgit:git-pull[1] will appropriately merge from that
-	remote branch.  Note that even if this option is not set,
-	this behavior can be chosen per-branch using the `--track`
-	and `--no-track` options.  This option defaults to true.
+	Tells `git-branch` and `git-checkout` to setup `branch.*.remote`
+	and `branch.*.merge` for new branches so that linkgit:git-pull[1]
+	will appropriately merge from that upstream branch. Note that even
+	if this option is not set, this behavior can be chosen per-branch
+	using the `--track`	and `--no-track` options. This option defaults
+	to true, which will setup tracking for remote branches. Set it to
+	`always` to automatically setup the aforementioned options for local
+	upstream branches as well.
 
 branch.<name>.remote::
 	When in branch <name>, it tells `git fetch` which remote to fetch.
diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 7e8874a..ce2fc64 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -105,20 +105,20 @@ OPTIONS
 	Display the full sha1s in output listing rather than abbreviating them.
 
 --track::
-	Set up configuration so that git-pull will automatically
-	retrieve data from the remote branch.  Use this if you always
-	pull from the same remote branch into the new branch, or if you
-	don't want to use "git pull <repository> <refspec>" explicitly.
-	This behavior is the default.  Set the
-	branch.autosetupmerge configuration variable to false if you
-	want git-checkout and git-branch to always behave as if
-	'--no-track' were given.
+	Set up configuration so that git-pull will
+	automatically retrieve data from the upstream branch.  Use this if
+	you always pull from the same upstream branch into the new branch,
+	or if you don't want to use "git pull <repository> <refspec>"
+	explicitly. This behavior is the default for remote branches.
+	Set the branch.autosetupmerge configuration variable to false if you
+	want git-checkout and git-branch to always behave as if '--no-track'
+	were given. Set it to 'always' if you want git-checkout and git-branch
+	to always behave as if '--track' were given.
 
 --no-track::
-	When a branch is created off a remote branch,
-	set up configuration so that git-pull will not retrieve data
-	from the remote branch, ignoring the branch.autosetupmerge
-	configuration variable.
+	Ignore the branch.autosetupmerge configuration variable. When
+	using git pull with the new branch, a <refspec> will have to be
+	given explicitly.
 
 <branchname>::
 	The name of the branch to create or delete.
diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index b4cfa04..ad2ab51 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -48,21 +48,20 @@ OPTIONS
 	may restrict the characters allowed in a branch name.
 
 --track::
-	When -b is given and a branch is created off a remote branch,
-	set up configuration so that git-pull will automatically
-	retrieve data from the remote branch.  Use this if you always
-	pull from the same remote branch into the new branch, or if you
-	don't want to use "git pull <repository> <refspec>" explicitly.
-	This behavior is the default.  Set the
-	branch.autosetupmerge configuration variable to false if you
-	want git-checkout and git-branch to always behave as if
-	'--no-track' were given.
+	When -b is given set up configuration so that git-pull will
+	automatically retrieve data from the upstream branch.  Use this if
+	you always pull from the same upstream branch into the new branch,
+	or if you don't want to use "git pull <repository> <refspec>"
+	explicitly. This behavior is the default for remote branches.
+	Set the branch.autosetupmerge configuration variable to false if you
+	want git-checkout and git-branch to always behave as if '--no-track'
+	were given. Set it to 'always' if you want git-checkout and git-branch
+	to always behave as if '--track' were given.
 
 --no-track::
-	When -b is given and a branch is created off a remote branch,
-	set up configuration so that git-pull will not retrieve data
-	from the remote branch, ignoring the branch.autosetupmerge
-	configuration variable.
+	Ignore the branch.autosetupmerge configuration variable. When
+	using git pull with the new branch, a <refspec> will have to be
+	given explicitly.
 
 -l::
 	Create the new branch's reflog.  This activates recording of
diff --git a/branch.c b/branch.c
index 1fc8788..24e98dd 100644
--- a/branch.c
+++ b/branch.c
@@ -71,7 +71,7 @@ static int setup_tracking(const char *new_ref, const char *orig_ref)
 
 void create_branch(const char *head,
 		   const char *name, const char *start_name,
-		   int force, int reflog, int track)
+		   int force, int reflog, enum branch_track track)
 {
 	struct ref_lock *lock;
 	struct commit *commit;
@@ -130,7 +130,15 @@ void create_branch(const char *head,
 	   automatically merges from there.  So far, this is only done for
 	   remotes registered via .git/config.  */
 	if (real_ref && track)
-		setup_tracking(name, real_ref);
+		if (setup_tracking(name, real_ref) == 1 && track == BRANCH_TRACK_ALWAYS) {
+			char key[1024];
+			sprintf(key, "branch.%s.remote", name);
+			git_config_set(key, ".");
+			sprintf(key, "branch.%s.merge", name);
+			git_config_set(key, real_ref);
+			printf("Branch %s set up with upstream branch %s.\n",
+			       name, real_ref);
+		}
 
 	if (write_ref_sha1(lock, sha1, msg) < 0)
 		die("Failed to write ref: %s.", strerror(errno));
diff --git a/branch.h b/branch.h
index d30abe0..9f0c2a2 100644
--- a/branch.h
+++ b/branch.h
@@ -13,7 +13,7 @@
  * branch for (if any).
  */
 void create_branch(const char *head, const char *name, const char *start_name,
-		   int force, int reflog, int track);
+		   int force, int reflog, enum branch_track track);
 
 /*
  * Remove information about the state of working on the current
diff --git a/builtin-branch.c b/builtin-branch.c
index 7e99103..3b53bb7 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -30,8 +30,6 @@ static const char * const builtin_branch_usage[] = {
 static const char *head;
 static unsigned char head_sha1[20];
 
-static int branch_track = 1;
-
 static int branch_use_color = -1;
 static char branch_colors[][COLOR_MAXLEN] = {
 	"\033[m",	/* reset */
@@ -76,10 +74,6 @@ static int git_branch_config(const char *var, const char *value)
 		color_parse(value, var, branch_colors[slot]);
 		return 0;
 	}
-	if (!strcmp(var, "branch.autosetupmerge")) {
-		branch_track = git_config_bool(var, value);
-		return 0;
-	}
 	return git_color_default_config(var, value);
 }
 
@@ -420,14 +414,15 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 {
 	int delete = 0, rename = 0, force_create = 0;
 	int verbose = 0, abbrev = DEFAULT_ABBREV, detached = 0;
-	int reflog = 0, track;
+	int reflog = 0;
+	enum branch_track track;
 	int kinds = REF_LOCAL_BRANCH;
 	struct commit_list *with_commit = NULL;
 
 	struct option options[] = {
 		OPT_GROUP("Generic options"),
 		OPT__VERBOSE(&verbose),
-		OPT_BOOLEAN( 0 , "track",  &track, "set up tracking mode (see git-pull(1))"),
+		OPT_SET_INT( 0 , "track",  &track, "set up tracking mode (see git-pull(1))", BRANCH_TRACK_ALWAYS),
 		OPT_BOOLEAN( 0 , "color",  &branch_use_color, "use colored output"),
 		OPT_SET_INT('r', NULL,     &kinds, "act on remote-tracking branches",
 			REF_REMOTE_BRANCH),
@@ -458,7 +453,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	if (branch_use_color == -1)
 		branch_use_color = git_use_color_default;
 
-	track = branch_track;
+	track = git_branch_track;
 	argc = parse_options(argc, argv, options, builtin_branch_usage, 0);
 	if (!!delete + !!rename + !!force_create > 1)
 		usage_with_options(builtin_branch_usage, options);
diff --git a/builtin-checkout.c b/builtin-checkout.c
index 0d19835..cf26e10 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -186,7 +186,7 @@ struct checkout_opts {
 
 	char *new_branch;
 	int new_branch_log;
-	int track;
+	enum branch_track track;
 };
 
 struct branch_info {
@@ -521,13 +521,8 @@ static int switch_branches(struct checkout_opts *opts,
 	return post_checkout_hook(old.commit, new->commit, 1);
 }
 
-static int branch_track = 0;
-
 static int git_checkout_config(const char *var, const char *value)
 {
-	if (!strcmp(var, "branch.autosetupmerge"))
-		branch_track = git_config_bool(var, value);
-
 	return git_default_config(var, value);
 }
 
@@ -542,7 +537,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		OPT__QUIET(&opts.quiet),
 		OPT_STRING('b', NULL, &opts.new_branch, "new branch", "branch"),
 		OPT_BOOLEAN('l', NULL, &opts.new_branch_log, "log for new branch"),
-		OPT_BOOLEAN( 0 , "track", &opts.track, "track"),
+		OPT_SET_INT( 0 , "track",  &opts.track, "track", BRANCH_TRACK_ALWAYS),
 		OPT_BOOLEAN('f', NULL, &opts.force, "force"),
 		OPT_BOOLEAN('m', NULL, &opts.merge, "merge"),
 		OPT_END(),
@@ -553,7 +548,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 
 	git_config(git_checkout_config);
 
-	opts.track = branch_track;
+	opts.track = git_branch_track;
 
 	argc = parse_options(argc, argv, options, checkout_usage, 0);
 	if (argc) {
@@ -582,7 +577,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		argc--;
 	}
 
-	if (!opts.new_branch && (opts.track != branch_track))
+	if (!opts.new_branch && (opts.track != git_branch_track))
 		die("git checkout: --track and --no-track require -b");
 
 	if (opts.force && opts.merge)
diff --git a/cache.h b/cache.h
index 2b59c44..90b3a9e 100644
--- a/cache.h
+++ b/cache.h
@@ -393,6 +393,14 @@ enum safe_crlf {
 
 extern enum safe_crlf safe_crlf;
 
+enum branch_track {
+	BRANCH_TRACK_REMOTES_FALSE = 0,
+	BRANCH_TRACK_REMOTES_TRUE,
+	BRANCH_TRACK_ALWAYS,
+};
+
+extern enum branch_track git_branch_track;
+
 #define GIT_REPO_VERSION 0
 extern int repository_format_version;
 extern int check_repository_format(void);
diff --git a/config.c b/config.c
index 8064cae..556b2ec 100644
--- a/config.c
+++ b/config.c
@@ -464,6 +464,14 @@ int git_default_config(const char *var, const char *value)
 		whitespace_rule_cfg = parse_whitespace_rule(value);
 		return 0;
 	}
+	if (!strcmp(var, "branch.autosetupmerge")) {
+		if (value && !strcasecmp(value, "always")) {
+			git_branch_track = BRANCH_TRACK_ALWAYS;
+			return 0;
+		}
+		git_branch_track = git_config_bool(var, value);
+		return 0;
+	}
 
 	/* Add other config variables here and to Documentation/config.txt. */
 	return 0;
diff --git a/environment.c b/environment.c
index 3527f16..24334a2 100644
--- a/environment.c
+++ b/environment.c
@@ -37,6 +37,7 @@ const char *excludes_file;
 int auto_crlf = 0;	/* 1: both ways, -1: only when adding git objects */
 enum safe_crlf safe_crlf = SAFE_CRLF_WARN;
 unsigned whitespace_rule_cfg = WS_DEFAULT_RULE;
+enum branch_track git_branch_track = BRANCH_TRACK_REMOTES_TRUE;
 
 /* This is set by setup_git_dir_gently() and/or git_default_config() */
 char *git_work_tree_cfg;
-- 
1.5.4.2.203.gf8d86

Re: [PATCH] branch: optionally setup branch.*.merge from upstream local branches

[Johannes Schindelin]

Hi,

On Mon, 18 Feb 2008, Jay Soffian wrote:

> On Feb 18, 2008 7:14 AM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> 
> > On Mon, 18 Feb 2008, Jay Soffian wrote:
> >
> > > -static int branch_track = 1;
> > > +static enum branch_track branch_track = BRANCH_TRACK_FALSE;
> >
> > That is a clear regression.
> 
> Perhaps. It's consistent with builtin-checkout.c though (which was 
> initializing it to 0). Who to believe?

Well, in the way _you_ implemented it, it is a clear regression, since 
_your_ patch changes the 1 to a 0.

> > Personally, I have no problem with typing "git merge <branch>" in your 
> > workflow.  I would even avoid saying "git pull" for obviously-local 
> > branches, because I would have forgotten which branch it tracked 
> > originally.
> 
> Um, well, apply this patch, set branch.autosetupmerge=always and then 
> branch.*.merge will tell you which branch it tracked originally. :-)

That's what I meant.  Rather than _looking it up_, and then saying "git 
pull", I'd do "git merge <branch>" _directly_.  One command.  Not two.

> Aside, then how do you figure out the upstream branch is if you've 
> forgotten?

Well, that's easy.  I would look at the output of "git show-branch --all".

But then, I usually do not care too much which branch I branched off, but 
which branch to merge with/rebase on.

Ciao,
Dscho

Re: [PATCH] branch: optionally setup branch.*.merge from upstream local branches

[Johannes Schindelin]

Hi,

sorry, I overlooked that earlier:

On Mon, 18 Feb 2008, Jay Soffian wrote:

> diff --git a/branch.c b/branch.c
> index 1fc8788..24e98dd 100644
> --- a/branch.c
> +++ b/branch.c
> @@ -130,7 +130,15 @@ void create_branch(const char *head,
>  	   automatically merges from there.  So far, this is only done for
>  	   remotes registered via .git/config.  */
>  	if (real_ref && track)
> -		setup_tracking(name, real_ref);
> +		if (setup_tracking(name, real_ref) == 1 && track == BRANCH_TRACK_ALWAYS) {

This line is too long.

> +			char key[1024];
> +			sprintf(key, "branch.%s.remote", name);

To stay safe, you should use snprintf() and test the return value.

> +			git_config_set(key, ".");
> +			sprintf(key, "branch.%s.merge", name);

Same here.

Thanks,
Dscho

[PATCH] branch: optionally setup branch.*.merge from upstream local branches

[Jay Soffian]

git branch / git checkout -b now honor --track when the upstream branch
is local. Previously --track was silently ignored for local upstream
branches.

The configuration setting branch.autosetupmerge can now be set to
"always" which is equivalent to using --track. Setting
branch.autosetupmerge to boolean true will retains the former behavior
of only setting up branch.*.merge for remote upstream branches.

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
 Incorporated Johannes' second round of feedback. This should be all for awhile.

 Documentation/config.txt       |   13 ++++++++-----
 Documentation/git-branch.txt   |   24 ++++++++++++------------
 Documentation/git-checkout.txt |   25 ++++++++++++-------------
 branch.c                       |   19 ++++++++++++-------
 branch.h                       |    2 +-
 builtin-branch.c               |   13 ++++---------
 builtin-checkout.c             |   13 ++++---------
 cache.h                        |    8 ++++++++
 config.c                       |    8 ++++++++
 environment.c                  |    1 +
 10 files changed, 70 insertions(+), 56 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index fb6dae0..f15fe0a 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -378,11 +378,14 @@ apply.whitespace::
 	as the '--whitespace' option. See linkgit:git-apply[1].
 
 branch.autosetupmerge::
-	Tells `git-branch` and `git-checkout` to setup new branches
-	so that linkgit:git-pull[1] will appropriately merge from that
-	remote branch.  Note that even if this option is not set,
-	this behavior can be chosen per-branch using the `--track`
-	and `--no-track` options.  This option defaults to true.
+	Tells `git-branch` and `git-checkout` to setup `branch.*.remote`
+	and `branch.*.merge` for new branches so that linkgit:git-pull[1]
+	will appropriately merge from that upstream branch. Note that even
+	if this option is not set, this behavior can be chosen per-branch
+	using the `--track`	and `--no-track` options. This option defaults
+	to true, which will setup tracking for remote branches. Set it to
+	`always` to automatically setup the aforementioned options for local
+	upstream branches as well.
 
 branch.<name>.remote::
 	When in branch <name>, it tells `git fetch` which remote to fetch.
diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 7e8874a..ce2fc64 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -105,20 +105,20 @@ OPTIONS
 	Display the full sha1s in output listing rather than abbreviating them.
 
 --track::
-	Set up configuration so that git-pull will automatically
-	retrieve data from the remote branch.  Use this if you always
-	pull from the same remote branch into the new branch, or if you
-	don't want to use "git pull <repository> <refspec>" explicitly.
-	This behavior is the default.  Set the
-	branch.autosetupmerge configuration variable to false if you
-	want git-checkout and git-branch to always behave as if
-	'--no-track' were given.
+	Set up configuration so that git-pull will
+	automatically retrieve data from the upstream branch.  Use this if
+	you always pull from the same upstream branch into the new branch,
+	or if you don't want to use "git pull <repository> <refspec>"
+	explicitly. This behavior is the default for remote branches.
+	Set the branch.autosetupmerge configuration variable to false if you
+	want git-checkout and git-branch to always behave as if '--no-track'
+	were given. Set it to 'always' if you want git-checkout and git-branch
+	to always behave as if '--track' were given.
 
 --no-track::
-	When a branch is created off a remote branch,
-	set up configuration so that git-pull will not retrieve data
-	from the remote branch, ignoring the branch.autosetupmerge
-	configuration variable.
+	Ignore the branch.autosetupmerge configuration variable. When
+	using git pull with the new branch, a <refspec> will have to be
+	given explicitly.
 
 <branchname>::
 	The name of the branch to create or delete.
diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index b4cfa04..ad2ab51 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -48,21 +48,20 @@ OPTIONS
 	may restrict the characters allowed in a branch name.
 
 --track::
-	When -b is given and a branch is created off a remote branch,
-	set up configuration so that git-pull will automatically
-	retrieve data from the remote branch.  Use this if you always
-	pull from the same remote branch into the new branch, or if you
-	don't want to use "git pull <repository> <refspec>" explicitly.
-	This behavior is the default.  Set the
-	branch.autosetupmerge configuration variable to false if you
-	want git-checkout and git-branch to always behave as if
-	'--no-track' were given.
+	When -b is given set up configuration so that git-pull will
+	automatically retrieve data from the upstream branch.  Use this if
+	you always pull from the same upstream branch into the new branch,
+	or if you don't want to use "git pull <repository> <refspec>"
+	explicitly. This behavior is the default for remote branches.
+	Set the branch.autosetupmerge configuration variable to false if you
+	want git-checkout and git-branch to always behave as if '--no-track'
+	were given. Set it to 'always' if you want git-checkout and git-branch
+	to always behave as if '--track' were given.
 
 --no-track::
-	When -b is given and a branch is created off a remote branch,
-	set up configuration so that git-pull will not retrieve data
-	from the remote branch, ignoring the branch.autosetupmerge
-	configuration variable.
+	Ignore the branch.autosetupmerge configuration variable. When
+	using git pull with the new branch, a <refspec> will have to be
+	given explicitly.
 
 -l::
 	Create the new branch's reflog.  This activates recording of
diff --git a/branch.c b/branch.c
index 1fc8788..64f0a4a 100644
--- a/branch.c
+++ b/branch.c
@@ -37,7 +37,7 @@ static int find_tracked_branch(struct remote *remote, void *priv)
  * to infer the settings for branch.<new_ref>.{remote,merge} from the
  * config.
  */
-static int setup_tracking(const char *new_ref, const char *orig_ref)
+static int setup_tracking(const char *new_ref, const char *orig_ref, int always)
 {
 	char key[1024];
 	struct tracking tracking;
@@ -49,8 +49,12 @@ static int setup_tracking(const char *new_ref, const char *orig_ref)
 	memset(&tracking, 0, sizeof(tracking));
 	tracking.spec.dst = (char *)orig_ref;
 	if (for_each_remote(find_tracked_branch, &tracking) ||
-			!tracking.matches)
-		return 1;
+			!tracking.matches) {
+		if (!always)
+			return 1;
+		tracking.matches = 1;
+		tracking.src = xstrdup(orig_ref);
+	}
 
 	if (tracking.matches > 1)
 		return error("Not tracking: ambiguous information for ref %s",
@@ -62,8 +66,9 @@ static int setup_tracking(const char *new_ref, const char *orig_ref)
 		sprintf(key, "branch.%s.merge", new_ref);
 		git_config_set(key, tracking.src);
 		free(tracking.src);
-		printf("Branch %s set up to track remote branch %s.\n",
-			       new_ref, orig_ref);
+		printf("Branch %s set up to track %s branch %s.\n",
+			new_ref, tracking.remote ? "remote" : "local", 
+			orig_ref);
 	}
 
 	return 0;
@@ -71,7 +76,7 @@ static int setup_tracking(const char *new_ref, const char *orig_ref)
 
 void create_branch(const char *head,
 		   const char *name, const char *start_name,
-		   int force, int reflog, int track)
+		   int force, int reflog, enum branch_track track)
 {
 	struct ref_lock *lock;
 	struct commit *commit;
@@ -130,7 +135,7 @@ void create_branch(const char *head,
 	   automatically merges from there.  So far, this is only done for
 	   remotes registered via .git/config.  */
 	if (real_ref && track)
-		setup_tracking(name, real_ref);
+		setup_tracking(name, real_ref, (track == BRANCH_TRACK_ALWAYS));
 
 	if (write_ref_sha1(lock, sha1, msg) < 0)
 		die("Failed to write ref: %s.", strerror(errno));
diff --git a/branch.h b/branch.h
index d30abe0..9f0c2a2 100644
--- a/branch.h
+++ b/branch.h
@@ -13,7 +13,7 @@
  * branch for (if any).
  */
 void create_branch(const char *head, const char *name, const char *start_name,
-		   int force, int reflog, int track);
+		   int force, int reflog, enum branch_track track);
 
 /*
  * Remove information about the state of working on the current
diff --git a/builtin-branch.c b/builtin-branch.c
index 7e99103..3b53bb7 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -30,8 +30,6 @@ static const char * const builtin_branch_usage[] = {
 static const char *head;
 static unsigned char head_sha1[20];
 
-static int branch_track = 1;
-
 static int branch_use_color = -1;
 static char branch_colors[][COLOR_MAXLEN] = {
 	"\033[m",	/* reset */
@@ -76,10 +74,6 @@ static int git_branch_config(const char *var, const char *value)
 		color_parse(value, var, branch_colors[slot]);
 		return 0;
 	}
-	if (!strcmp(var, "branch.autosetupmerge")) {
-		branch_track = git_config_bool(var, value);
-		return 0;
-	}
 	return git_color_default_config(var, value);
 }
 
@@ -420,14 +414,15 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 {
 	int delete = 0, rename = 0, force_create = 0;
 	int verbose = 0, abbrev = DEFAULT_ABBREV, detached = 0;
-	int reflog = 0, track;
+	int reflog = 0;
+	enum branch_track track;
 	int kinds = REF_LOCAL_BRANCH;
 	struct commit_list *with_commit = NULL;
 
 	struct option options[] = {
 		OPT_GROUP("Generic options"),
 		OPT__VERBOSE(&verbose),
-		OPT_BOOLEAN( 0 , "track",  &track, "set up tracking mode (see git-pull(1))"),
+		OPT_SET_INT( 0 , "track",  &track, "set up tracking mode (see git-pull(1))", BRANCH_TRACK_ALWAYS),
 		OPT_BOOLEAN( 0 , "color",  &branch_use_color, "use colored output"),
 		OPT_SET_INT('r', NULL,     &kinds, "act on remote-tracking branches",
 			REF_REMOTE_BRANCH),
@@ -458,7 +453,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	if (branch_use_color == -1)
 		branch_use_color = git_use_color_default;
 
-	track = branch_track;
+	track = git_branch_track;
 	argc = parse_options(argc, argv, options, builtin_branch_usage, 0);
 	if (!!delete + !!rename + !!force_create > 1)
 		usage_with_options(builtin_branch_usage, options);
diff --git a/builtin-checkout.c b/builtin-checkout.c
index 0d19835..cf26e10 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -186,7 +186,7 @@ struct checkout_opts {
 
 	char *new_branch;
 	int new_branch_log;
-	int track;
+	enum branch_track track;
 };
 
 struct branch_info {
@@ -521,13 +521,8 @@ static int switch_branches(struct checkout_opts *opts,
 	return post_checkout_hook(old.commit, new->commit, 1);
 }
 
-static int branch_track = 0;
-
 static int git_checkout_config(const char *var, const char *value)
 {
-	if (!strcmp(var, "branch.autosetupmerge"))
-		branch_track = git_config_bool(var, value);
-
 	return git_default_config(var, value);
 }
 
@@ -542,7 +537,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		OPT__QUIET(&opts.quiet),
 		OPT_STRING('b', NULL, &opts.new_branch, "new branch", "branch"),
 		OPT_BOOLEAN('l', NULL, &opts.new_branch_log, "log for new branch"),
-		OPT_BOOLEAN( 0 , "track", &opts.track, "track"),
+		OPT_SET_INT( 0 , "track",  &opts.track, "track", BRANCH_TRACK_ALWAYS),
 		OPT_BOOLEAN('f', NULL, &opts.force, "force"),
 		OPT_BOOLEAN('m', NULL, &opts.merge, "merge"),
 		OPT_END(),
@@ -553,7 +548,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 
 	git_config(git_checkout_config);
 
-	opts.track = branch_track;
+	opts.track = git_branch_track;
 
 	argc = parse_options(argc, argv, options, checkout_usage, 0);
 	if (argc) {
@@ -582,7 +577,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		argc--;
 	}
 
-	if (!opts.new_branch && (opts.track != branch_track))
+	if (!opts.new_branch && (opts.track != git_branch_track))
 		die("git checkout: --track and --no-track require -b");
 
 	if (opts.force && opts.merge)
diff --git a/cache.h b/cache.h
index 2b59c44..90b3a9e 100644
--- a/cache.h
+++ b/cache.h
@@ -393,6 +393,14 @@ enum safe_crlf {
 
 extern enum safe_crlf safe_crlf;
 
+enum branch_track {
+	BRANCH_TRACK_REMOTES_FALSE = 0,
+	BRANCH_TRACK_REMOTES_TRUE,
+	BRANCH_TRACK_ALWAYS,
+};
+
+extern enum branch_track git_branch_track;
+
 #define GIT_REPO_VERSION 0
 extern int repository_format_version;
 extern int check_repository_format(void);
diff --git a/config.c b/config.c
index 8064cae..556b2ec 100644
--- a/config.c
+++ b/config.c
@@ -464,6 +464,14 @@ int git_default_config(const char *var, const char *value)
 		whitespace_rule_cfg = parse_whitespace_rule(value);
 		return 0;
 	}
+	if (!strcmp(var, "branch.autosetupmerge")) {
+		if (value && !strcasecmp(value, "always")) {
+			git_branch_track = BRANCH_TRACK_ALWAYS;
+			return 0;
+		}
+		git_branch_track = git_config_bool(var, value);
+		return 0;
+	}
 
 	/* Add other config variables here and to Documentation/config.txt. */
 	return 0;
diff --git a/environment.c b/environment.c
index 3527f16..24334a2 100644
--- a/environment.c
+++ b/environment.c
@@ -37,6 +37,7 @@ const char *excludes_file;
 int auto_crlf = 0;	/* 1: both ways, -1: only when adding git objects */
 enum safe_crlf safe_crlf = SAFE_CRLF_WARN;
 unsigned whitespace_rule_cfg = WS_DEFAULT_RULE;
+enum branch_track git_branch_track = BRANCH_TRACK_REMOTES_TRUE;
 
 /* This is set by setup_git_dir_gently() and/or git_default_config() */
 char *git_work_tree_cfg;
-- 
1.5.4.2.203.gf8d86

Re: [PATCH] branch: optionally setup branch.*.merge from upstream local branches

[Johannes Schindelin]

Hi,

On Mon, 18 Feb 2008, Jay Soffian wrote:

>  Incorporated Johannes' second round of feedback.

Not exactly.

> diff --git a/branch.c b/branch.c
> index 1fc8788..64f0a4a 100644
> --- a/branch.c
> +++ b/branch.c
> @@ -37,7 +37,7 @@ static int find_tracked_branch(struct remote *remote, void *priv)
>   * to infer the settings for branch.<new_ref>.{remote,merge} from the
>   * config.
>   */
> -static int setup_tracking(const char *new_ref, const char *orig_ref)
> +static int setup_tracking(const char *new_ref, const char *orig_ref, int always)
>  {
>  	char key[1024];
>  	struct tracking tracking;
> @@ -49,8 +49,12 @@ static int setup_tracking(const char *new_ref, const char *orig_ref)
>  	memset(&tracking, 0, sizeof(tracking));
>  	tracking.spec.dst = (char *)orig_ref;
>  	if (for_each_remote(find_tracked_branch, &tracking) ||
> -			!tracking.matches)
> -		return 1;
> +			!tracking.matches) {
> +		if (!always)
> +			return 1;
> +		tracking.matches = 1;
> +		tracking.src = xstrdup(orig_ref);
> +	}
>  

This looks completely different than what I commented on.  And my comments 
suggested a different solution.

Unfortunately, I will not have time for the rest of the day to review that 
new thing (it is not at all obvious for me why this works as intended, and 
does not break anything else).

But you can use the time to write some tests ;-)

Ciao,
Dscho

Re: [PATCH] branch: optionally setup branch.*.merge from upstream local branches

[Jay Soffian]

On Feb 18, 2008 9:05 AM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Mon, 18 Feb 2008, Jay Soffian wrote:
>
> >  Incorporated Johannes' second round of feedback.
>
> Not exactly.

Heh...

> This looks completely different than what I commented on.  And my comments
> suggested a different solution.

Thought process went like this:

- fix sprintf() to use snprintf()
- notice setup_tracking() uses sprintf() and think about fixing it too
- realize setup_tracking() does bounds checking first and its use of sprintf()
  is therefor safe.
- go to copy the bounds checking code from setup_tracking() to the area you
  commented on.
- realized duplicating the code was dumb and I could just re-use what was
  already in setup_tracking() via what was just sent.

> But you can use the time to write some tests ;-)

Sure I can. I really hope after all this work the patch gets accepted.

j.

Re: [PATCH] branch: optionally setup branch.*.merge from upstream local branches

[Johannes Schindelin]

Hi,

On Mon, 18 Feb 2008, Jay Soffian wrote:

> diff --git a/branch.c b/branch.c
> index 1fc8788..64f0a4a 100644
> --- a/branch.c
> +++ b/branch.c
> @@ -49,8 +49,12 @@ static int setup_tracking(const char *new_ref, const char *orig_ref)
>  	memset(&tracking, 0, sizeof(tracking));
>  	tracking.spec.dst = (char *)orig_ref;
>  	if (for_each_remote(find_tracked_branch, &tracking) ||
> -			!tracking.matches)
> -		return 1;
> +			!tracking.matches) {
> +		if (!always)
> +			return 1;
> +		tracking.matches = 1;
> +		tracking.src = xstrdup(orig_ref);
> +	}

I think you need to split this into

	if (for_each_remote(...))
		return 1;
	if (!tracking.matches && !always)
			return 1;

because an error in for_each_remote() is a proper error, and should not be 
ignored (speaking of which, the return value is unchecked...).  And then, 
later,

        if (tracking.matches == 1) {
                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);
+               git_config_set(key, tracking.src ? tracking.src : orig_ref);
                free(tracking.src);

> @@ -130,7 +135,7 @@ void create_branch(const char *head,
>  	   automatically merges from there.  So far, this is only done for
>  	   remotes registered via .git/config.  */
>  	if (real_ref && track)
> -		setup_tracking(name, real_ref);
> +		setup_tracking(name, real_ref, (track == BRANCH_TRACK_ALWAYS));

It is a matter of taste, I guess, but I would remove the parens around the 
boolean expression.

FWIW a very good argument in favour of your patch is IMHO that now, 
"--track" works as a user would expect, even for local branch-offs.

And of course that you were very responsive, and really put in a lot of 
effort.

Thanks,
Dscho

Re: [PATCH] branch: optionally setup branch.*.merge from upstream local branches

[Mike Hommey]

On Mon, Feb 18, 2008 at 01:29:55PM +0000, Johannes Schindelin wrote:
> Hi,
> 
> sorry, I overlooked that earlier:
> 
> On Mon, 18 Feb 2008, Jay Soffian wrote:
> 
> > diff --git a/branch.c b/branch.c
> > index 1fc8788..24e98dd 100644
> > --- a/branch.c
> > +++ b/branch.c
> > @@ -130,7 +130,15 @@ void create_branch(const char *head,
> >  	   automatically merges from there.  So far, this is only done for
> >  	   remotes registered via .git/config.  */
> >  	if (real_ref && track)
> > -		setup_tracking(name, real_ref);
> > +		if (setup_tracking(name, real_ref) == 1 && track == BRANCH_TRACK_ALWAYS) {
> 
> This line is too long.
> 
> > +			char key[1024];
> > +			sprintf(key, "branch.%s.remote", name);
> 
> To stay safe, you should use snprintf() and test the return value.

Or use strbufs.

Mike

Re: [PATCH] branch: optionally setup branch.*.merge from upstream local branches

[Junio C Hamano]

Jay Soffian <jaysoffian@gmail.com> writes:

> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index fb6dae0..f15fe0a 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -378,11 +378,14 @@ apply.whitespace::
>  	as the '--whitespace' option. See linkgit:git-apply[1].
>  
>  branch.autosetupmerge::
> ...
> +	Tells `git-branch` and `git-checkout` to setup `branch.*.remote`
> +	and `branch.*.merge` for new branches so that linkgit:git-pull[1]
> +	will appropriately merge from that upstream branch. Note that even
> +	if this option is not set, this behavior can be chosen per-branch
> +	using the `--track`	and `--no-track` options. This option defaults

You have an unintended tab there.

> +	to true, which will setup tracking for remote branches. Set it to
> +	`always` to automatically setup the aforementioned options for local
> +	upstream branches as well.

I somehow find it easier to read if the default is always
described at the end of the paragraph, i.e. "Setting this to
value X does foo, Y does bar, Z does baz. Defaults to X.", but
that may be just me.

> diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
> index 7e8874a..ce2fc64 100644
> --- a/Documentation/git-branch.txt
> +++ b/Documentation/git-branch.txt
> @@ -105,20 +105,20 @@ OPTIONS
>  	Display the full sha1s in output listing rather than abbreviating them.
>  
>  --track::
> ...
> +	Set up configuration so that git-pull will
> +	automatically retrieve data from the upstream branch.  Use this if
> +	you always pull from the same upstream branch into the new branch,
> +	or if you don't want to use "git pull <repository> <refspec>"
> +	explicitly. This behavior is the default for remote branches.
> +	Set the branch.autosetupmerge configuration variable to false if you
> +	want git-checkout and git-branch to always behave as if '--no-track'
> +	were given. Set it to 'always' if you want git-checkout and git-branch
> +	to always behave as if '--track' were given.

Re-wrapping the
paragraph at an unusual place.  It is Ok to do so if it is done
to keep the unchanged part intact to minimize the diff text, but
otherwise, please don't.

You reworded "remote branch" to "upstream branch" here and other
places.  I think that makes the intention easier to understand.
It is "the branch this new branch intends to build on top of",
so the earlier text should have said "remote tracking branch"
but the intention has always been to mean "on top of whom will I
be building".

This is not a flaw in your patch, but I suspect "or if you
don't want to" should have been "and if you don't want to".

"This behaviour is the default for remote branches" feels a bit
odd.  It confuses the first time reader as if you are saying "if
you are creating a remote branch with "git-branch" command, this
behaviour is the default", which is not what you are saying.  I
think "... the default when forking off of a remote tracking
branch" is what you meant.

>  --no-track::
> -	When a branch is created off a remote branch,
> -	set up configuration so that git-pull will not retrieve data
> -	from the remote branch, ignoring the branch.autosetupmerge
> -	configuration variable.
> +	Ignore the branch.autosetupmerge configuration variable. When
> +	using git pull with the new branch, a <refspec> will have to be
> +	given explicitly.

This rewrite probably has made the description easier to understand.

> diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
> index b4cfa04..ad2ab51 100644
> --- a/Documentation/git-checkout.txt
> +++ b/Documentation/git-checkout.txt
> @@ -48,21 +48,20 @@ OPTIONS
>  	may restrict the characters allowed in a branch name.
>  
>  --track::
> ...
> +	When -b is given set up configuration so that git-pull will

s/given/&,/;

> +	automatically retrieve data from the upstream branch.  Use this if
> +	you always pull from the same upstream branch into the new branch,
> +	or if you don't want to use "git pull <repository> <refspec>"
> +	explicitly. This behavior is the default for remote branches.

The same comment on "or if you don't want to" applies.

> @@ -49,8 +49,12 @@ static int setup_tracking(const char *new_ref, const char *orig_ref)
>  	memset(&tracking, 0, sizeof(tracking));
>  	tracking.spec.dst = (char *)orig_ref;
>  	if (for_each_remote(find_tracked_branch, &tracking) ||
> -			!tracking.matches)
> -		return 1;
> +			!tracking.matches) {
> +		if (!always)
> +			return 1;
> +		tracking.matches = 1;
> +		tracking.src = xstrdup(orig_ref);
> +	}

I suspect this is a clever fallback to pretend as if you have
[remote "."] fetch=refs/*:refs/* when nothing else matches, but
I think it needs an explanation why this is a good idea and
would not break others.  Also I suspect this should be moved
after you see for-each-remote returns without error and did not
find any match.

> @@ -71,7 +76,7 @@ static int setup_tracking(const char *new_ref, const char *orig_ref)
>  
>  void create_branch(const char *head,
>  		   const char *name, const char *start_name,
> -		   int force, int reflog, int track)
> +		   int force, int reflog, enum branch_track track)
>  {
>  	struct ref_lock *lock;
>  	struct commit *commit;
> @@ -130,7 +135,7 @@ void create_branch(const char *head,
>  	   automatically merges from there.  So far, this is only done for
>  	   remotes registered via .git/config.  */
>  	if (real_ref && track)
> -		setup_tracking(name, real_ref);
> +		setup_tracking(name, real_ref, (track == BRANCH_TRACK_ALWAYS));

It probably is better to pass 'track' itself and have the
special case logic inside setup_tracking() instead.  Maybe we
would want to add new tracking mode that needs different
matching logic there later.

The move to default_config() is probably a good idea.

> diff --git a/cache.h b/cache.h
> index 2b59c44..90b3a9e 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -393,6 +393,14 @@ enum safe_crlf {
>  
>  extern enum safe_crlf safe_crlf;
>  
> +enum branch_track {
> +	BRANCH_TRACK_REMOTES_FALSE = 0,
> +	BRANCH_TRACK_REMOTES_TRUE,
> +	BRANCH_TRACK_ALWAYS,
> +};

Perhaps BRANCH_TRACK_{NEVER,REMOTE,ALWAYS} would be easier
(although I cannot decide if s/NEVER/FALSE/ would be better)?

[PATCH] branch: optionally setup branch.*.merge from upstream local branches

[Jay Soffian]

git branch / git checkout -b now honor --track when the upstream branch
is local. Previously --track was silently ignored for local upstream
branches.

The configuration setting branch.autosetupmerge can now be set to
"always" which is equivalent to using --track. Setting
branch.autosetupmerge to boolean true will retains the former behavior
of only setting up branch.*.merge for remote upstream branches.

Includes test cases for the new functionality.

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
Changes from last iteration:

- Reworked the documentation. I incorporated some of Junio's suggestions, but
  I made a lot of other clarifications (IMO...). Let me know what you think.

- Added a suggested test case to t7201-co and some of my own to t3200-branch.

- Renamed the enum values per Junio.

- Incorporated suggestions from Junio and Johannes re:setup_tracking().

- The code/doc/and test case changes are now all in this commit together (I
  don't think it's too much for review...?)

Thanks for the feedback so far.

 Documentation/config.txt       |   15 ++++++---
 Documentation/git-branch.txt   |   69 +++++++++++++++++-----------------------
 Documentation/git-checkout.txt |   61 ++++++++++++++++++-----------------
 branch.c                       |   20 +++++++----
 branch.h                       |    2 +-
 builtin-branch.c               |   14 +++-----
 builtin-checkout.c             |   13 ++-----
 cache.h                        |    8 +++++
 config.c                       |    8 +++++
 environment.c                  |    1 +
 t/t3200-branch.sh              |   32 ++++++++++++++++--
 t/t7201-co.sh                  |    8 +++++
 12 files changed, 147 insertions(+), 104 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index fb6dae0..fd4904f 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -378,11 +378,16 @@ apply.whitespace::
 	as the '--whitespace' option. See linkgit:git-apply[1].
 
 branch.autosetupmerge::
-	Tells `git-branch` and `git-checkout` to setup new branches
-	so that linkgit:git-pull[1] will appropriately merge from that
-	remote branch.  Note that even if this option is not set,
-	this behavior can be chosen per-branch using the `--track`
-	and `--no-track` options.  This option defaults to true.
+	When setting up new branch <name> from existing branch
+	<starting-point>, tells linkgit:git-branch[1] and
+	linkgit:git-checkout[1] to configure branch.<name>.remote and
+	branch.<name>.merge for use with linkgit:git-fetch[1] and
+	linkgit:git-pull[1]. There are three valid settings: `false` -- no
+	automatic setup; `true` -- automatic setup when <starting-poiint> is
+	a remote branch; `always` -- automatic setup when <starting-point>
+	is either a local or remote branch. The `--track` and `--no-track`
+	options to linkgit:git-branch[1] and linkgit:git-checkout[1] always
+	override this setting. The default is `true`.
 
 branch.<name>.remote::
 	When in branch <name>, it tells `git fetch` which remote to fetch.
diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 7e8874a..3ad9dc0 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -11,9 +11,9 @@ SYNOPSIS
 'git-branch' [--color | --no-color] [-r | -a]
 	   [-v [--abbrev=<length> | --no-abbrev]]
 	   [--contains <commit>]
-'git-branch' [--track | --no-track] [-l] [-f] <branchname> [<start-point>]
-'git-branch' (-m | -M) [<oldbranch>] <newbranch>
-'git-branch' (-d | -D) [-r] <branchname>...
+'git-branch' [--track | --no-track] [-l] [-f] <newbranch> [<start-point>]
+'git-branch' (-m | -M) [<branch>] <newbranch>
+'git-branch' (-d | -D) [-r] <branch>...
 
 DESCRIPTION
 -----------
@@ -25,29 +25,25 @@ With `--contains <commit>`, shows only the branches that
 contains the named commit (in other words, the branches whose
 tip commits are descendant of the named commit).
 
-In its second form, a new branch named <branchname> will be created.
-It will start out with a head equal to the one given as <start-point>.
-If no <start-point> is given, the branch will be created with a head
-equal to that of the currently checked out branch.
+In its second form, a new branch named <newbranch> will be created.
+It will start out with a head equal to <start-point>. If no
+<start-point> is given, it is assumed to be the currently checked
+out branch. The new branch may optionally be configured for use with
+linkgit:git-pull[1] -- see `--track` and `--no-track` in the OPTIONS
+section below.
 
 Note that this will create the new branch, but it will not switch the
 working tree to it; use "git checkout <newbranch>" to switch to the
-new branch.
+new branch. (As a shortcut consider using "git checkout [--track | 
+--notrack ] -b <newbranch>" instead.)
 
-When a local branch is started off a remote branch, git sets up the
-branch so that linkgit:git-pull[1] will appropriately merge from that
-remote branch.  If this behavior is not desired, it is possible to
-disable it using the global `branch.autosetupmerge` configuration
-flag.  That setting can be overridden by using the `--track`
-and `--no-track` options.
-
-With a '-m' or '-M' option, <oldbranch> will be renamed to <newbranch>.
-If <oldbranch> had a corresponding reflog, it is renamed to match
+With a '-m' or '-M' option, <branch> will be renamed to <newbranch>.
+If <branch> had a corresponding reflog, it is renamed to match
 <newbranch>, and a reflog entry is created to remember the branch
 renaming. If <newbranch> exists, -M must be used to force the rename
 to happen.
 
-With a `-d` or `-D` option, `<branchname>` will be deleted.  You may
+With a `-d` or `-D` option, `<branch>` will be deleted.  You may
 specify more than one branch for deletion.  If the branch currently
 has a reflog then the reflog will also be deleted.
 
@@ -69,7 +65,7 @@ OPTIONS
 -l::
 	Create the branch's reflog.  This activates recording of
 	all changes made to the branch ref, enabling use of date
-	based sha1 expressions such as "<branchname>@\{yesterday}".
+	based sha1 expressions such as "<branch>@\{yesterday}".
 
 -f::
 	Force the creation of a new branch even if it means deleting
@@ -105,38 +101,31 @@ OPTIONS
 	Display the full sha1s in output listing rather than abbreviating them.
 
 --track::
-	Set up configuration so that git-pull will automatically
-	retrieve data from the remote branch.  Use this if you always
-	pull from the same remote branch into the new branch, or if you
-	don't want to use "git pull <repository> <refspec>" explicitly.
-	This behavior is the default.  Set the
-	branch.autosetupmerge configuration variable to false if you
-	want git-checkout and git-branch to always behave as if
-	'--no-track' were given.
+	Set up the new branch's configuration so that linkgit:git-pull[1]
+	will appropriately merge from the start point when run without
+	arguments while in the new branch. `--track` is the default behavior
+	when the start point is a remote branch. See the documentation for
+	the `branch.autosetupmerge` configuration variable in
+	linkgit:git-config[1] for how to modify the default.
 
 --no-track::
-	When a branch is created off a remote branch,
-	set up configuration so that git-pull will not retrieve data
-	from the remote branch, ignoring the branch.autosetupmerge
-	configuration variable.
+	Ignore the `branch.autosetupmerge` configuration variable. The new
+	branch will not be setup for automatic merging.
 
-<branchname>::
-	The name of the branch to create or delete.
+<newbranch>::
+	The name of the branch to create or rename to.
 	The new branch name must pass all checks defined by
 	linkgit:git-check-ref-format[1].  Some of these checks
 	may restrict the characters allowed in a branch name.
 
 <start-point>::
 	The new branch will be created with a HEAD equal to this.  It may
-	be given as a branch name, a commit-id, or a tag.  If this option
-	is omitted, the current branch is assumed.
+	be given as a branch name, a commit-id, or a tag.  Defaults
+	to the current HEAD.
 
-<oldbranch>::
-	The name of an existing branch to rename.
+<branch>::
+	The name of an existing branch to rename or delete.
 
-<newbranch>::
-	The new name for an existing branch. The same restrictions as for
-	<branchname> applies.
 
 
 Examples
diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index b4cfa04..fd862b4 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -8,25 +8,27 @@ git-checkout - Checkout and switch to a branch
 SYNOPSIS
 --------
 [verse]
-'git-checkout' [-q] [-f] [[--track | --no-track] -b <new_branch> [-l]] [-m] [<branch>]
+'git-checkout' [-q] [-f] [-m] <branch>
+'git-checkout' [-q] [-f] [-m] [-l] [--track | --no-track] -b <newbranch> [<start-point>]
 'git-checkout' [<tree-ish>] <paths>...
 
 DESCRIPTION
 -----------
 
-When <paths> are not given, this command switches branches by
-updating the index and working tree to reflect the specified
-branch, <branch>, and updating HEAD to be <branch> or, if
-specified, <new_branch>.  Using -b will cause <new_branch> to
-be created; in this case you can use the --track or --no-track
-options, which will be passed to `git branch`.
+It its first form this command switches branches by updating the
+index and working tree to reflect <branch> and updating HEAD to
+<branch>.
 
-When <paths> are given, this command does *not* switch
+In its second form this command optionally switches to <start-point>
+by updating the index and working tree to reflect <start-point>.
+<newbranch> is then created and HEAD is updated to it. The new
+branch may optionally be configured for use with linkgit:git-pull[1]
+-- see `--track` and `--no-track` in the OPTIONS section below.
+
+In its third form, this command does *not* switch
 branches.  It updates the named paths in the working tree from
 the index file (i.e. it runs `git-checkout-index -f -u`), or
-from a named commit.  In
-this case, the `-f` and `-b` options are meaningless and giving
-either of them results in an error.  <tree-ish> argument can be
+from a named commit. The optional <tree-ish> argument can be
 used to specify a specific tree-ish (i.e. commit, tag or tree)
 to update the index for the given paths before updating the
 working tree.
@@ -42,27 +44,23 @@ OPTIONS
 	from HEAD.  This is used to throw away local changes.
 
 -b::
-	Create a new branch named <new_branch> and start it at
-	<branch>.  The new branch name must pass all checks defined
-	by linkgit:git-check-ref-format[1].  Some of these checks
-	may restrict the characters allowed in a branch name.
+	Create a new branch named <newbranch> and start it at the optional
+	<start-point> (the currently checked out branch if not given). The
+	new branch name must pass all checks defined by
+	linkgit:git-check-ref-format[1]. Some of these checks may restrict
+	the characters allowed in a branch name.
 
 --track::
-	When -b is given and a branch is created off a remote branch,
-	set up configuration so that git-pull will automatically
-	retrieve data from the remote branch.  Use this if you always
-	pull from the same remote branch into the new branch, or if you
-	don't want to use "git pull <repository> <refspec>" explicitly.
-	This behavior is the default.  Set the
-	branch.autosetupmerge configuration variable to false if you
-	want git-checkout and git-branch to always behave as if
-	'--no-track' were given.
+	Set up the new branch's configuration so that linkgit:git-pull[1]
+	will appropriately merge from the start point when run without
+	arguments while in the new branch. `--track` is the default behavior
+	when the start point is a remote branch. See the documentation for
+	the `branch.autosetupmerge` configuration variable in
+	linkgit:git-config[1] for how to modify the default.
 
 --no-track::
-	When -b is given and a branch is created off a remote branch,
-	set up configuration so that git-pull will not retrieve data
-	from the remote branch, ignoring the branch.autosetupmerge
-	configuration variable.
+	Ignore the `branch.autosetupmerge` configuration variable. The new
+	branch will not be setup for automatic merging.
 
 -l::
 	Create the new branch's reflog.  This activates recording of
@@ -83,9 +81,14 @@ paths are left unmerged, and you need to resolve the conflicts
 and mark the resolved paths with `git add` (or `git rm` if the merge
 should result in deletion of the path).
 
-<new_branch>::
+<newbranch>::
 	Name for the new branch.
 
+<start-point>::
+	The new branch will be created with a HEAD equal to this.  It may
+	be given as a branch name, a commit-id, or a tag.  Defaults
+	to the current HEAD.
+
 <branch>::
 	Branch to checkout; may be any object ID that resolves to a
 	commit.  Defaults to HEAD.
diff --git a/branch.c b/branch.c
index 1fc8788..9d7585e 100644
--- a/branch.c
+++ b/branch.c
@@ -37,7 +37,8 @@ static int find_tracked_branch(struct remote *remote, void *priv)
  * to infer the settings for branch.<new_ref>.{remote,merge} from the
  * config.
  */
-static int setup_tracking(const char *new_ref, const char *orig_ref)
+static int setup_tracking(const char *new_ref, const char *orig_ref,
+                          enum branch_track track)
 {
 	char key[1024];
 	struct tracking tracking;
@@ -48,10 +49,14 @@ static int setup_tracking(const char *new_ref, const char *orig_ref)
 
 	memset(&tracking, 0, sizeof(tracking));
 	tracking.spec.dst = (char *)orig_ref;
-	if (for_each_remote(find_tracked_branch, &tracking) ||
-			!tracking.matches)
+	if (for_each_remote(find_tracked_branch, &tracking))
 		return 1;
 
+	if (!tracking.matches && track == BRANCH_TRACK_ALWAYS) {
+		tracking.matches = 1;
+		tracking.src = xstrdup(orig_ref);
+	}
+
 	if (tracking.matches > 1)
 		return error("Not tracking: ambiguous information for ref %s",
 				orig_ref);
@@ -62,8 +67,9 @@ static int setup_tracking(const char *new_ref, const char *orig_ref)
 		sprintf(key, "branch.%s.merge", new_ref);
 		git_config_set(key, tracking.src);
 		free(tracking.src);
-		printf("Branch %s set up to track remote branch %s.\n",
-			       new_ref, orig_ref);
+		printf("Branch %s set up to track %s branch %s.\n",
+			new_ref, tracking.remote ? "remote" : "local",
+			orig_ref);
 	}
 
 	return 0;
@@ -71,7 +77,7 @@ static int setup_tracking(const char *new_ref, const char *orig_ref)
 
 void create_branch(const char *head,
 		   const char *name, const char *start_name,
-		   int force, int reflog, int track)
+		   int force, int reflog, enum branch_track track)
 {
 	struct ref_lock *lock;
 	struct commit *commit;
@@ -130,7 +136,7 @@ void create_branch(const char *head,
 	   automatically merges from there.  So far, this is only done for
 	   remotes registered via .git/config.  */
 	if (real_ref && track)
-		setup_tracking(name, real_ref);
+		setup_tracking(name, real_ref, track);
 
 	if (write_ref_sha1(lock, sha1, msg) < 0)
 		die("Failed to write ref: %s.", strerror(errno));
diff --git a/branch.h b/branch.h
index d30abe0..9f0c2a2 100644
--- a/branch.h
+++ b/branch.h
@@ -13,7 +13,7 @@
  * branch for (if any).
  */
 void create_branch(const char *head, const char *name, const char *start_name,
-		   int force, int reflog, int track);
+		   int force, int reflog, enum branch_track track);
 
 /*
  * Remove information about the state of working on the current
diff --git a/builtin-branch.c b/builtin-branch.c
index 7e99103..bcc2354 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -30,8 +30,6 @@ static const char * const builtin_branch_usage[] = {
 static const char *head;
 static unsigned char head_sha1[20];
 
-static int branch_track = 1;
-
 static int branch_use_color = -1;
 static char branch_colors[][COLOR_MAXLEN] = {
 	"\033[m",	/* reset */
@@ -76,10 +74,6 @@ static int git_branch_config(const char *var, const char *value)
 		color_parse(value, var, branch_colors[slot]);
 		return 0;
 	}
-	if (!strcmp(var, "branch.autosetupmerge")) {
-		branch_track = git_config_bool(var, value);
-		return 0;
-	}
 	return git_color_default_config(var, value);
 }
 
@@ -420,14 +414,16 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 {
 	int delete = 0, rename = 0, force_create = 0;
 	int verbose = 0, abbrev = DEFAULT_ABBREV, detached = 0;
-	int reflog = 0, track;
+	int reflog = 0;
+	enum branch_track track;
 	int kinds = REF_LOCAL_BRANCH;
 	struct commit_list *with_commit = NULL;
 
 	struct option options[] = {
 		OPT_GROUP("Generic options"),
 		OPT__VERBOSE(&verbose),
-		OPT_BOOLEAN( 0 , "track",  &track, "set up tracking mode (see git-pull(1))"),
+		OPT_SET_INT( 0 , "track",  &track, "set up tracking mode (see git-pull(1))",
+			BRANCH_TRACK_ALWAYS),
 		OPT_BOOLEAN( 0 , "color",  &branch_use_color, "use colored output"),
 		OPT_SET_INT('r', NULL,     &kinds, "act on remote-tracking branches",
 			REF_REMOTE_BRANCH),
@@ -458,7 +454,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	if (branch_use_color == -1)
 		branch_use_color = git_use_color_default;
 
-	track = branch_track;
+	track = git_branch_track;
 	argc = parse_options(argc, argv, options, builtin_branch_usage, 0);
 	if (!!delete + !!rename + !!force_create > 1)
 		usage_with_options(builtin_branch_usage, options);
diff --git a/builtin-checkout.c b/builtin-checkout.c
index 0d19835..cf26e10 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -186,7 +186,7 @@ struct checkout_opts {
 
 	char *new_branch;
 	int new_branch_log;
-	int track;
+	enum branch_track track;
 };
 
 struct branch_info {
@@ -521,13 +521,8 @@ static int switch_branches(struct checkout_opts *opts,
 	return post_checkout_hook(old.commit, new->commit, 1);
 }
 
-static int branch_track = 0;
-
 static int git_checkout_config(const char *var, const char *value)
 {
-	if (!strcmp(var, "branch.autosetupmerge"))
-		branch_track = git_config_bool(var, value);
-
 	return git_default_config(var, value);
 }
 
@@ -542,7 +537,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		OPT__QUIET(&opts.quiet),
 		OPT_STRING('b', NULL, &opts.new_branch, "new branch", "branch"),
 		OPT_BOOLEAN('l', NULL, &opts.new_branch_log, "log for new branch"),
-		OPT_BOOLEAN( 0 , "track", &opts.track, "track"),
+		OPT_SET_INT( 0 , "track",  &opts.track, "track", BRANCH_TRACK_ALWAYS),
 		OPT_BOOLEAN('f', NULL, &opts.force, "force"),
 		OPT_BOOLEAN('m', NULL, &opts.merge, "merge"),
 		OPT_END(),
@@ -553,7 +548,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 
 	git_config(git_checkout_config);
 
-	opts.track = branch_track;
+	opts.track = git_branch_track;
 
 	argc = parse_options(argc, argv, options, checkout_usage, 0);
 	if (argc) {
@@ -582,7 +577,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		argc--;
 	}
 
-	if (!opts.new_branch && (opts.track != branch_track))
+	if (!opts.new_branch && (opts.track != git_branch_track))
 		die("git checkout: --track and --no-track require -b");
 
 	if (opts.force && opts.merge)
diff --git a/cache.h b/cache.h
index 2b59c44..3c17e2c 100644
--- a/cache.h
+++ b/cache.h
@@ -393,6 +393,14 @@ enum safe_crlf {
 
 extern enum safe_crlf safe_crlf;
 
+enum branch_track {
+	BRANCH_TRACK_NEVER = 0,
+	BRANCH_TRACK_REMOTE,
+	BRANCH_TRACK_ALWAYS,
+};
+
+extern enum branch_track git_branch_track;
+
 #define GIT_REPO_VERSION 0
 extern int repository_format_version;
 extern int check_repository_format(void);
diff --git a/config.c b/config.c
index 8064cae..556b2ec 100644
--- a/config.c
+++ b/config.c
@@ -464,6 +464,14 @@ int git_default_config(const char *var, const char *value)
 		whitespace_rule_cfg = parse_whitespace_rule(value);
 		return 0;
 	}
+	if (!strcmp(var, "branch.autosetupmerge")) {
+		if (value && !strcasecmp(value, "always")) {
+			git_branch_track = BRANCH_TRACK_ALWAYS;
+			return 0;
+		}
+		git_branch_track = git_config_bool(var, value);
+		return 0;
+	}
 
 	/* Add other config variables here and to Documentation/config.txt. */
 	return 0;
diff --git a/environment.c b/environment.c
index 3527f16..6739a3f 100644
--- a/environment.c
+++ b/environment.c
@@ -37,6 +37,7 @@ const char *excludes_file;
 int auto_crlf = 0;	/* 1: both ways, -1: only when adding git objects */
 enum safe_crlf safe_crlf = SAFE_CRLF_WARN;
 unsigned whitespace_rule_cfg = WS_DEFAULT_RULE;
+enum branch_track git_branch_track = BRANCH_TRACK_REMOTE;
 
 /* This is set by setup_git_dir_gently() and/or git_default_config() */
 char *git_work_tree_cfg;
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index d21081d..94e4dc6 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -15,6 +15,9 @@ test_expect_success \
     'echo Hello > A &&
      git update-index --add A &&
      git-commit -m "Initial commit." &&
+     echo World >> A &&
+     git update-index --add A &&
+     git-commit -m "Second commit." &&
      HEAD=$(git rev-parse --verify HEAD)'
 
 test_expect_success \
@@ -161,7 +164,7 @@ test_expect_success 'avoid ambiguous track' '
 '
 
 test_expect_success 'test overriding tracking setup via --no-track' \
-    'git config branch.autosetupmerge true &&
+    'git config branch.autosetupmerge always &&
      git config remote.local.url . &&
      git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
      (git show-ref -q refs/remotes/local/master || git-fetch local) &&
@@ -171,7 +174,9 @@ test_expect_success 'test overriding tracking setup via --no-track' \
      ! test "$(git config branch.my2.merge)" = refs/heads/master'
 
 test_expect_success 'no tracking without .fetch entries' \
-    'git branch --track my6 s &&
+    'git config branch.autosetupmerge true &&
+     git branch my6 s &&
+     git config branch.automsetupmerge false &&
      test -z "$(git config branch.my6.remote)" &&
      test -z "$(git config branch.my6.merge)"'
 
@@ -192,14 +197,33 @@ test_expect_success 'test deleting branch without config' \
     'git branch my7 s &&
      test "$(git branch -d my7 2>&1)" = "Deleted branch my7."'
 
-# Keep this test last, as it changes the current branch
+test_expect_success 'test tracking without .fetch entries w/--track given' \
+    'git branch --track my8 &&
+     test "$(git config branch.my8.remote)" &&
+     test "$(git config branch.my8.merge)"'
+
+test_expect_success \
+    'no tracking entries from non-branch HEAD even w/autosetupmerge=always)' \
+    'git config branch.autosetupmerge always &&
+     git branch my9 HEAD^ &&
+     git config branch.autosetupmerge false &&
+     test -z "$(git config branch.my9.remote)" &&
+     test -z "$(git config branch.my9.merge)"'
+
+test_expect_success \
+    'no tracking entries from non-branch HEAD even w/--track' \
+    'git branch --track my10 HEAD^ &&
+     test -z "$(git config branch.my10.remote)" &&
+     test -z "$(git config branch.my10.merge)"'
+
+# Keep this test last, as they change the current branch
 cat >expect <<EOF
 0000000000000000000000000000000000000000 $HEAD $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150200 +0000	branch: Created from master
 EOF
 test_expect_success \
     'git checkout -b g/h/i -l should create a branch and a log' \
 	'GIT_COMMITTER_DATE="2005-05-26 23:30" \
-     git-checkout -b g/h/i -l master &&
+     git checkout -b g/h/i -l master &&
 	 test -f .git/refs/heads/g/h/i &&
 	 test -f .git/logs/refs/heads/g/h/i &&
 	 diff expect .git/logs/refs/heads/g/h/i'
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 06959d9..384e3be 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -279,4 +279,12 @@ test_expect_success 'checkout with ambiguous tag/branch names' '
 
 '
 
+test_expect_success \
+    'checkout w/--track sets up tracking w/autosetupmerge=false' '
+    git config branch.autosetupmerge false &&
+    git checkout -f master && git clean -f &&
+    git checkout --track -b track &&
+    test "$(git config branch.track.remote)" &&
+    test "$(git config branch.track.merge)"'
+
 test_done
-- 
1.5.4.2.203.gf8d86

Re: [PATCH] branch: optionally setup branch.*.merge from upstream local branches

[Jay Soffian]

On Feb 18, 2008 9:07 PM, Jay Soffian <jaysoffian@gmail.com> wrote:
> diff --git a/branch.c b/branch.c
> index 1fc8788..9d7585e 100644
> --- a/branch.c
> +++ b/branch.c
> @@ -37,7 +37,8 @@ static int find_tracked_branch(struct remote *remote, void *priv)
>   * to infer the settings for branch.<new_ref>.{remote,merge} from the
>   * config.
>   */
> -static int setup_tracking(const char *new_ref, const char *orig_ref)
> +static int setup_tracking(const char *new_ref, const char *orig_ref,
> +                          enum branch_track track)
>  {
>         char key[1024];
>         struct tracking tracking;
> @@ -48,10 +49,14 @@ static int setup_tracking(const char *new_ref, const char *orig_ref)
>
>         memset(&tracking, 0, sizeof(tracking));
>         tracking.spec.dst = (char *)orig_ref;
> -       if (for_each_remote(find_tracked_branch, &tracking) ||
> -                       !tracking.matches)
> +       if (for_each_remote(find_tracked_branch, &tracking))
>                 return 1;
>
> +       if (!tracking.matches && track == BRANCH_TRACK_ALWAYS) {
> +               tracking.matches = 1;
> +               tracking.src = xstrdup(orig_ref);
> +       }
> +

Well that's obviously wrong (though it causes no problems, setup_tracking()
would return 0 when it should return 1, but its return value is currently
ignored). I changed it to:

	if (!tracking.matches) {
		if (track != BRANCH_TRACK_ALWAYS)
			return 1;
		tracking.matches = 1;
		tracking.src = xstrdup(orig_ref);
	}

j.

Re: [PATCH] branch: optionally setup branch.*.merge from upstream local branches

[Junio C Hamano]

Jay Soffian <jaysoffian@gmail.com> writes:

> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index fb6dae0..fd4904f 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -378,11 +378,16 @@ apply.whitespace::
>  	as the '--whitespace' option. See linkgit:git-apply[1].
>  
>  branch.autosetupmerge::
> ...
> +	When setting up new branch <name> from existing branch
> +	<starting-point>, tells linkgit:git-branch[1] and
> +	linkgit:git-checkout[1] to configure branch.<name>.remote and
> +	branch.<name>.merge for use with linkgit:git-fetch[1] and
> +	linkgit:git-pull[1]. There are three valid settings: `false` -- no
> +	automatic setup; `true` -- automatic setup when <starting-poiint> is
> +	a remote branch; `always` -- automatic setup when <starting-point>
> +	is either a local or remote branch. The `--track` and `--no-track`
> +	options to linkgit:git-branch[1] and linkgit:git-checkout[1] always
> +	override this setting. The default is `true`.

Three issues.

 (1) "tells X and Y to configure" is bad.  We may later want to
     add a command that forks a new branch from an existing one,
     and you sure do want it to honor this variable.  Better not
     list the commands but tell the reader that it applies to
     the act of forking a new branch based on an existing one.

 (2) "configure X and Y" is bad.  We may later want to add new
     configuration variables to facilitate marking a branch that
     builds on top of another in addition to the existing remote
     and merge, and you sure do want them to be also set up
     appropriately.

 (3) "for use with X and Y" is bad.  We may later want to add
     new commands that you would use to work with a branch that
     builds on top of another, and you sure do want them to
     honor the configuration variables this automatically sets
     up.

So, how about...

        When creating a new branch starting at an existing 'upstream'
        branch, the new branch can be marked as building on top of the
        'upstream' branch, by setting a few configuration variables
        (e.g. branch.<name>.remote and branch.<name>.merge).  This can
        be explicitly asked for by giving `--track` (and turned off by
        giving `--no-track`) option to commands that create a new branch
        (e.g.  linkgit:git-branch[1]); this variable is consulted when
        neither option is given.  
+
When set to `true`, act as if `--track` is given only if 'upstream' is a
remote tracking branch.  When set to `always`, act as if `--track` is
given even if 'upstream' is a local branch.  When set to `false`, no
such extra configuration is made.  Defaults to `true`.

> diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
> index 7e8874a..3ad9dc0 100644
> --- a/Documentation/git-branch.txt
> +++ b/Documentation/git-branch.txt
> @@ -11,9 +11,9 @@ SYNOPSIS
>  'git-branch' [--color | --no-color] [-r | -a]
>  	   [-v [--abbrev=<length> | --no-abbrev]]
>  	   [--contains <commit>]
> -'git-branch' [--track | --no-track] [-l] [-f] <branchname> [<start-point>]
> -'git-branch' (-m | -M) [<oldbranch>] <newbranch>
> -'git-branch' (-d | -D) [-r] <branchname>...
> +'git-branch' [--track | --no-track] [-l] [-f] <newbranch> [<start-point>]
> +'git-branch' (-m | -M) [<branch>] <newbranch>
> +'git-branch' (-d | -D) [-r] <branch>...

Looks more-or-less unnecessary churn to me.  If you have to rename the
placeholders, I would say:

	git branch <branch> [<start-point>]
        git branch -m [<old>] <new>
        git branch -d <branch>

would be short, sweet and sufficient.

>  DESCRIPTION
>  -----------
> @@ -25,29 +25,25 @@ With `--contains <commit>`, shows only the branches that
>  contains the named commit (in other words, the branches whose
>  tip commits are descendant of the named commit).
>  
> -In its second form, a new branch named <branchname> will be created.
> -It will start out with a head equal to the one given as <start-point>.
> -If no <start-point> is given, the branch will be created with a head
> -equal to that of the currently checked out branch.
> +In its second form, a new branch named <newbranch> will be created.
> +It will start out with a head equal to <start-point>. If no
> +<start-point> is given, it is assumed to be the currently checked
> +out branch. The new branch may optionally be configured for use with
> +linkgit:git-pull[1] -- see `--track` and `--no-track` in the OPTIONS
> +section below.

Drop all the changes except for the last sentence.

>  Note that this will create the new branch, but it will not switch the
>  working tree to it; use "git checkout <newbranch>" to switch to the
> -new branch.
> +new branch. (As a shortcut consider using "git checkout [--track | 
> +--notrack ] -b <newbranch>" instead.)

Hinting an alternative is good but is too verbose.

	"git checkout -b" can be used to create and switch in one step.

would be short, sweet and sufficient.

> -When a local branch is started off a remote branch, git sets up the
> -branch so that linkgit:git-pull[1] will appropriately merge from that
> -remote branch.  If this behavior is not desired, it is possible to
> -disable it using the global `branch.autosetupmerge` configuration
> -flag.  That setting can be overridden by using the `--track`
> -and `--no-track` options.

Why remove this?

> @@ -105,38 +101,31 @@ OPTIONS
>  	Display the full sha1s in output listing rather than abbreviating them.
>  
>  --track::
> -	Set up configuration so that git-pull will automatically
> -	retrieve data from the remote branch.  Use this if you always
> -	pull from the same remote branch into the new branch, or if you
> -	don't want to use "git pull <repository> <refspec>" explicitly.
> -	This behavior is the default.  Set the
> -	branch.autosetupmerge configuration variable to false if you
> -	want git-checkout and git-branch to always behave as if
> -	'--no-track' were given.
> +	Set up the new branch's configuration so that linkgit:git-pull[1]
> +	will appropriately merge from the start point when run without

"from the start point" is misleading.

> +	arguments while in the new branch. `--track` is the default behavior
> +	when the start point is a remote branch. See the documentation for
> +	the `branch.autosetupmerge` configuration variable in
> +	linkgit:git-config[1] for how to modify the default.

How about

	When <start-point> is a name of an existing branch (either local
	or remote tracking branch), treat the <start-point> branch as
	the upstream branch for the new branch (i.e. the new branch is
	marked to build on top of the branch) by setting a few extra
	configuration variables to cause "git pull" without repository
	and refspec parameters to fetch and merge (or rebase) from the
	upstream branch.  By default, this happens when forking from a
	remote tracking branch without this option.  This default can be 
	modified by the `branch.autosetupmerge` configuration variable.

By the way, I think the existing code is buggy.  When you say:

	$ git branch --track new $(git rev-parse --verify HEAD~12)

the command should barf, saying "Starting point is not a branch; it
is impossible to set up a tracking information", perhaps without even
creating the "new" branch.  Instead, it silently creates a new branch.

>  --no-track::
> +	Ignore the `branch.autosetupmerge` configuration variable. The new
> +	branch will not be setup for automatic merging.

This is much better than the original text.

>  <start-point>::
>  	The new branch will be created with a HEAD equal to this.  It may
> -	be given as a branch name, a commit-id, or a tag.  If this option
> -	is omitted, the current branch is assumed.
> +	be given as a branch name, a commit-id, or a tag.  Defaults
> +	to the current HEAD.

I do not think this is what _you_ want (although I do not personally
care).  Defaulting to HEAD means <start-point> is not a name of the
branch in such a case.  I suspect that you would want this to create a
new branch that follows your current branch (assuming your HEAD is not
detached):

	$ git branch --track new

wouldn't you?

> diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
> index b4cfa04..fd862b4 100644
> --- a/Documentation/git-checkout.txt
> +++ b/Documentation/git-checkout.txt
> @@ -8,25 +8,27 @@ git-checkout - Checkout and switch to a branch
>  SYNOPSIS
>  --------
>  [verse]
> -'git-checkout' [-q] [-f] [[--track | --no-track] -b <new_branch> [-l]] [-m] [<branch>]
> +'git-checkout' [-q] [-f] [-m] <branch>
> +'git-checkout' [-q] [-f] [-m] [-l] [--track | --no-track] -b <newbranch> [<start-point>]
>  'git-checkout' [<tree-ish>] <paths>...
>
>  DESCRIPTION
>  -----------
>  
> -When <paths> are not given, this command switches branches by
> -updating the index and working tree to reflect the specified
> -branch, <branch>, and updating HEAD to be <branch> or, if
> -specified, <new_branch>.  Using -b will cause <new_branch> to
> -be created; in this case you can use the --track or --no-track
> -options, which will be passed to `git branch`.
> +It its first form this command switches branches by updating the
> +index and working tree to reflect <branch> and updating HEAD to
> +<branch>.

Now this is getting tiresome.  The rewrite might be a good change for
readability but this does not have anything to do with --track.  The
review process of such a rewrite should be done as an independent
topic.

I give up (for now).

Re: [PATCH] branch: optionally setup branch.*.merge from upstream local branches

[Alex Riesen]

Jay Soffian, Tue, Feb 19, 2008 03:07:12 +0100:
> diff --git a/cache.h b/cache.h
> index 2b59c44..3c17e2c 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -393,6 +393,14 @@ enum safe_crlf {
>  
>  extern enum safe_crlf safe_crlf;
>  
> +enum branch_track {
> +	BRANCH_TRACK_NEVER = 0,

enums start at 0 anyway, don't they?

> +	BRANCH_TRACK_REMOTE,
> +	BRANCH_TRACK_ALWAYS,
> +};

Re: [PATCH] branch: optionally setup branch.*.merge from upstream local branches

[Johannes Schindelin]

Hi,

On Mon, 18 Feb 2008, Jay Soffian wrote:

> On Feb 18, 2008 9:07 PM, Jay Soffian <jaysoffian@gmail.com> wrote:
> > diff --git a/branch.c b/branch.c
> > index 1fc8788..9d7585e 100644
> > --- a/branch.c
> > +++ b/branch.c
> > @@ -37,7 +37,8 @@ static int find_tracked_branch(struct remote *remote, void *priv)
> >   * to infer the settings for branch.<new_ref>.{remote,merge} from the
> >   * config.
> >   */
> > -static int setup_tracking(const char *new_ref, const char *orig_ref)
> > +static int setup_tracking(const char *new_ref, const char *orig_ref,
> > +                          enum branch_track track)
> >  {
> >         char key[1024];
> >         struct tracking tracking;
> > @@ -48,10 +49,14 @@ static int setup_tracking(const char *new_ref, const char *orig_ref)
> >
> >         memset(&tracking, 0, sizeof(tracking));
> >         tracking.spec.dst = (char *)orig_ref;
> > -       if (for_each_remote(find_tracked_branch, &tracking) ||
> > -                       !tracking.matches)
> > +       if (for_each_remote(find_tracked_branch, &tracking))
> >                 return 1;
> >
> > +       if (!tracking.matches && track == BRANCH_TRACK_ALWAYS) {
> > +               tracking.matches = 1;
> > +               tracking.src = xstrdup(orig_ref);
> > +       }
> > +
> 
> Well that's obviously wrong (though it causes no problems, setup_tracking()
> would return 0 when it should return 1, but its return value is currently
> ignored). I changed it to:
> 
> 	if (!tracking.matches) {
> 		if (track != BRANCH_TRACK_ALWAYS)
> 			return 1;
> 		tracking.matches = 1;
> 		tracking.src = xstrdup(orig_ref);
> 	}

Ah, yes.  But I still maintain that xstrdup()ing orig_ref only to free it 
later is ugly.  Why not have the "tracking.src ? tracking.src : orig_ref" 
as I suggested?  The free() obviously can stay, since it will say 
"free(NULL)" in the case of tracking.matches == 0.

Ciao,
Dscho

Re: [PATCH] branch: optionally setup branch.*.merge from upstream local branches

[Jay Soffian]

On Feb 19, 2008 12:52 AM, Junio C Hamano <gitster@pobox.com> wrote:

> Three issues.
>
>  (1) "tells X and Y to configure" is bad.  We may later want to
>      add a command that forks a new branch from an existing one,
>      and you sure do want it to honor this variable.  Better not
>      list the commands but tell the reader that it applies to
>      the act of forking a new branch based on an existing one.
>
>  (2) "configure X and Y" is bad.  We may later want to add new
>      configuration variables to facilitate marking a branch that
>      builds on top of another in addition to the existing remote
>      and merge, and you sure do want them to be also set up
>      appropriately.
>
>  (3) "for use with X and Y" is bad.  We may later want to add
>      new commands that you would use to work with a branch that
>      builds on top of another, and you sure do want them to
>      honor the configuration variables this automatically sets
>      up.
>
> So, how about...
>
>         When creating a new branch starting at an existing 'upstream'
>         branch, the new branch can be marked as building on top of the
>         'upstream' branch, by setting a few configuration variables
>         (e.g. branch.<name>.remote and branch.<name>.merge).  This can
>         be explicitly asked for by giving `--track` (and turned off by
>         giving `--no-track`) option to commands that create a new branch
>         (e.g.  linkgit:git-branch[1]); this variable is consulted when
>         neither option is given.

I disagree. I think explicit is better than implicit and if a new
command or config variable is added, then the docs should be similarly
updated.

> Looks more-or-less unnecessary churn to me.

I was trying to make git-branch(1) and git-checkout(1) consistent.

> > -When a local branch is started off a remote branch, git sets up the
> > -branch so that linkgit:git-pull[1] will appropriately merge from that
> > -remote branch.  If this behavior is not desired, it is possible to
> > -disable it using the global `branch.autosetupmerge` configuration
> > -flag.  That setting can be overridden by using the `--track`
> > -and `--no-track` options.
>
> Why remove this?

Because --track and --no-track are documented in the OPTIONS section, so there
is no reason to document them up top as well. None of the other options [-l],
[-q], [-f] are documented in the intro.

> By the way, I think the existing code is buggy.  When you say:
>
>         $ git branch --track new $(git rev-parse --verify HEAD~12)
>
> the command should barf, saying "Starting point is not a branch; it
> is impossible to set up a tracking information", perhaps without even
> creating the "new" branch.  Instead, it silently creates a new branch.

That's what it did before I started touching any code. Perhaps it
should be a separate cleanup patch.

> >  <start-point>::
> >       The new branch will be created with a HEAD equal to this.  It may
> > -     be given as a branch name, a commit-id, or a tag.  If this option
> > -     is omitted, the current branch is assumed.
> > +     be given as a branch name, a commit-id, or a tag.  Defaults
> > +     to the current HEAD.
>
> I do not think this is what _you_ want (although I do not personally
> care).  Defaulting to HEAD means <start-point> is not a name of the
> branch in such a case.

Okay, that's a subtle difference. The existing documentation conflates (I think)
"HEAD" with "current branch". I see now they are different.

> Now this is getting tiresome.  The rewrite might be a good change for
> readability but this does not have anything to do with --track.  The
> review process of such a rewrite should be done as an independent
> topic.
>
> I give up (for now).

<grumble>
I will rip all the documentation changes out of this patch and do them
separately at a later time if at all. The current documentation has lots of
areas for improvement. It's hard to touch it w/o realizing you've made a clean
spot and then wanting to rewrite whole swaths. I happen to think I'm a decent
technical writer (no, it's not my day job) and I thought the changes I was
making, from the perspective of a new set of eyes, made things more clear. I
give up on that for now.

	
I too find it tiresome trying to get this patch into git. As a newbie
contributor, I'm beginning to find this very discouraging. I appreciate that
changes need a lot of scrutiny, but if the same amount of scrutiny were applied
to the existing code it would never get through.

Which is not to say that I don't appreciate your feedback and work on git.
</grumble>
j.

Re: [PATCH] branch: optionally setup branch.*.merge from upstream local branches

[Jay Soffian]

On Feb 19, 2008 5:55 AM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>
> On Mon, 18 Feb 2008, Jay Soffian wrote:
>
> >
> >       if (!tracking.matches) {
> >               if (track != BRANCH_TRACK_ALWAYS)
> >                       return 1;
> >               tracking.matches = 1;
> >               tracking.src = xstrdup(orig_ref);
> >       }
>
> Ah, yes.  But I still maintain that xstrdup()ing orig_ref only to free it
> later is ugly.  Why not have the "tracking.src ? tracking.src : orig_ref"
> as I suggested?

Sorry, I'd missed that. Good idea, will do.

j.

Re: [PATCH] branch: optionally setup branch.*.merge from upstream local branches

[Jay Soffian]

On Feb 19, 2008 2:44 AM, Alex Riesen <raa.lkml@gmail.com> wrote:
> Jay Soffian, Tue, Feb 19, 2008 03:07:12 +0100:
> > +enum branch_track {
> > +     BRANCH_TRACK_NEVER = 0,
>
> enums start at 0 anyway, don't they?

I don't know, but guys, give me a break on the enums already. What's
the preferred syntax already because the existing code is not
consistent:

enum color_branch {
	COLOR_BRANCH_RESET = 0,
	COLOR_BRANCH_PLAIN = 1,
	COLOR_BRANCH_REMOTE = 2,
	COLOR_BRANCH_LOCAL = 3,
	COLOR_BRANCH_CURRENT = 4,
};

enum {
	TAGS_UNSET = 0,
	TAGS_DEFAULT = 1,
	TAGS_SET = 2
};

enum update_mode { BOTH = 0, WORKING_DIRECTORY, INDEX } *modes;

enum exist_status {
	index_nonexistent = 0,
	index_directory,
	index_gitdir,
};

enum CAPABILITY {
	NOLOGIN = 0,
	UIDPLUS,
	LITERALPLUS,
	NAMESPACE,
};

j.

Re: [PATCH] branch: optionally setup branch.*.merge from upstream local branches

[Johannes Schindelin]

Hi,

On Tue, 19 Feb 2008, Jay Soffian wrote:

> On Feb 19, 2008 2:44 AM, Alex Riesen <raa.lkml@gmail.com> wrote:
> > Jay Soffian, Tue, Feb 19, 2008 03:07:12 +0100:
> > > +enum branch_track {
> > > +     BRANCH_TRACK_NEVER = 0,
> >
> > enums start at 0 anyway, don't they?
> 
> I don't know, but guys, give me a break on the enums already.

Heh.

Don't be discouraged.  I think you help educating me here, too:

> What's the preferred syntax already because the existing code is not 
> consistent: [...]

I had the impression that the first was set to "= 0", and the rest was not 
explicitely set.  But I was obviously wrong.

Ciao,
Dscho

Re: [PATCH] branch: optionally setup branch.*.merge from upstream local branches

[Jay Soffian]

On Feb 19, 2008 5:55 AM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>
> Ah, yes.  But I still maintain that xstrdup()ing orig_ref only to free it
> later is ugly.  Why not have the "tracking.src ? tracking.src : orig_ref"
> as I suggested?  The free() obviously can stay, since it will say
> "free(NULL)" in the case of tracking.matches == 0.

How about this?

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)
		return error("Tracking not set up: name too long: %s",
				new_ref);

	memset(&tracking, 0, sizeof(tracking));
	tracking.spec.dst = (char *)orig_ref;
	if (for_each_remote(find_tracked_branch, &tracking))
		return 1;

	if (!tracking.matches && track != BRANCH_TRACK_ALWAYS)
		return 1;

	if (tracking.matches > 1)
		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);
	free(tracking.src);
	printf("Branch %s set up to track %s branch %s.\n", new_ref,
		tracking.remote ? "remote" : "local", orig_ref);

	return 0;
}

j.

Re: [PATCH] branch: optionally setup branch.*.merge from upstream local branches

[Johannes Schindelin]

Hi,

On Tue, 19 Feb 2008, Jay Soffian wrote:

> On Feb 19, 2008 5:55 AM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> >
> > Ah, yes.  But I still maintain that xstrdup()ing orig_ref only to free 
> > it later is ugly.  Why not have the "tracking.src ? tracking.src : 
> > orig_ref" as I suggested?  The free() obviously can stay, since it 
> > will say "free(NULL)" in the case of tracking.matches == 0.
> 
> How about this?
> 
> 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)
> 		return error("Tracking not set up: name too long: %s",
> 				new_ref);
> 
> 	memset(&tracking, 0, sizeof(tracking));
> 	tracking.spec.dst = (char *)orig_ref;
> 	if (for_each_remote(find_tracked_branch, &tracking))
> 		return 1;
> 
> 	if (!tracking.matches && track != BRANCH_TRACK_ALWAYS)
> 		return 1;
> 
> 	if (tracking.matches > 1)
> 		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);
> 	free(tracking.src);
> 	printf("Branch %s set up to track %s branch %s.\n", new_ref,
> 		tracking.remote ? "remote" : "local", orig_ref);
> 
> 	return 0;
> }

I like it.

Ciao,
Dscho

Re: [PATCH] branch: optionally setup branch.*.merge from upstream local branches

[Alex Riesen]

Jay Soffian, Tue, Feb 19, 2008 14:49:17 +0100:
> On Feb 19, 2008 2:44 AM, Alex Riesen <raa.lkml@gmail.com> wrote:
> > Jay Soffian, Tue, Feb 19, 2008 03:07:12 +0100:
> > > +enum branch_track {
> > > +     BRANCH_TRACK_NEVER = 0,
> >
> > enums start at 0 anyway, don't they?
> 
> I don't know, but guys, give me a break on the enums already. What's
> the preferred syntax already because the existing code is not
> consistent:

Well, it could also mean that there is no rules yet, and you can
do the next sane thing of your choice.

> enum color_branch {
> 	COLOR_BRANCH_RESET = 0,
> 	COLOR_BRANCH_PLAIN = 1,
> 	COLOR_BRANCH_REMOTE = 2,
> 	COLOR_BRANCH_LOCAL = 3,
> 	COLOR_BRANCH_CURRENT = 4,
> };
>
> enum {
> 	TAGS_UNSET = 0,
> 	TAGS_DEFAULT = 1,
> 	TAGS_SET = 2
> };

Which is just as useless. It is not useful even for documentation,
as the verbatim values are not used anywere else (i.e. they are not
a part of file format, where they could stand out as the numbers
listed).

It looks like someone was just too paranoid or suffered a mental
damage from too long exposure to proprietary code.

> enum update_mode { BOTH = 0, WORKING_DIRECTORY, INDEX } *modes;
> 
> enum exist_status {
> 	index_nonexistent = 0,
> 	index_directory,
> 	index_gitdir,
> };
> 
> enum CAPABILITY {
> 	NOLOGIN = 0,
> 	UIDPLUS,
> 	LITERALPLUS,
> 	NAMESPACE,
> };

All the same...

Re: [PATCH] branch: optionally setup branch.*.merge from upstream local branches

[Junio C Hamano]

Alex Riesen <raa.lkml@gmail.com> writes:

> Well, it could also mean that there is no rules yet, and you can
> do the next sane thing of your choice.
>
>> enum color_branch {
>> 	COLOR_BRANCH_RESET = 0,
>> 	COLOR_BRANCH_PLAIN = 1,
>> 	COLOR_BRANCH_REMOTE = 2,
>> 	COLOR_BRANCH_LOCAL = 3,
>> 	COLOR_BRANCH_CURRENT = 4,
>> };

This enum is used as an index into branch_colors[] array.  Of
course, by omitting everything you will get the default "start
with 0, incrementing by 1" which will be the right assignment
anyway.  But we would want to leave a clue for people who would
want to touch this later that individual values have some
meaning, more than just that they have to be distinct.

>> enum {
>> 	TAGS_UNSET = 0,
>> 	TAGS_DEFAULT = 1,
>> 	TAGS_SET = 2
>> };

This one can be made unspecified or even shuffled, because
nobody does:

	int function_that_acts_on_tag_setting(int tag) {
                if (!tag) {
                        ... do something ...
                }
                if (TAGS_DEFAULT <= tag) {
                        ... do something else ...
                }
                return some_array[tag];
	}

So there is no reason to spell out any of the values.

>> enum update_mode { BOTH = 0, WORKING_DIRECTORY, INDEX } *modes;

Likewise.

>> enum exist_status {
>> 	index_nonexistent = 0,
>> 	index_directory,
>> 	index_gitdir,
>> };

Likewise, modulo that making "nonexistent" explicitly to 0 is a
very sensible thing whoever wrote that code has done.  This is
used as a type of directory_exists_in_index() function, and
callers can say:

       	if (!directory_exists_in_index(dirname)) {
        	... ah, there is no such directory ...
	} else {
        	... something exists, but I do not care what kind ...
	}

So spelling out that "nonexistent MUST BE 0" (even though C
language will give value 0 to it anyway) is a good convention.

>> enum CAPABILITY {
>> 	NOLOGIN = 0,
>> 	UIDPLUS,
>> 	LITERALPLUS,
>> 	NAMESPACE,
>> };

This seems to be meant to match the order in the corresponding
cap_list[] array, so this cannot be reshuffled (iow, it is
similar to color_branch).

If you do not need to have any specific value assigned nor order
among the enum tokens, (i.e. somebody later can add a new enum
anywhere in the decl or even reshuffle the existing ones without
breaking your code), it is a good idea to hint that fact by not
having any "= value" in the decl.  If you do rely on specific
one being zero (see "exist_status" example above), it is better
to spell it out that it has to be zero even if it is the first
entry to hint that fact.  Of course, if you need a set of values
that are not sequential, you would need to spell out each and
every one of them.

Re: [PATCH] branch: optionally setup branch.*.merge from upstream local branches

[Junio C Hamano]

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

> Alex Riesen <raa.lkml@gmail.com> writes:
>
>> Well, it could also mean that there is no rules yet, and you can
>> do the next sane thing of your choice.
>>
>>> enum color_branch {
>>> 	COLOR_BRANCH_RESET = 0,
>>> 	COLOR_BRANCH_PLAIN = 1,
>>> 	COLOR_BRANCH_REMOTE = 2,
>>> 	COLOR_BRANCH_LOCAL = 3,
>>> 	COLOR_BRANCH_CURRENT = 4,
>>> };
>
> This enum is used as an index into branch_colors[] array.  ...
> ...  But we would want to leave a clue for people who would
> want to touch this later that individual values have some
> meaning, more than just that they have to be distinct.
> ...
>>> enum CAPABILITY {
>>> 	NOLOGIN = 0,
>>> 	UIDPLUS,
>>> 	LITERALPLUS,
>>> 	NAMESPACE,
>>> };
>
> This seems to be meant to match the order in the corresponding
> cap_list[] array, so this cannot be reshuffled (iow, it is
> similar to color_branch).

Side note.  My preference for enum that indexes an array is to
use the latter, not spelling everything out like "color_branch"
does.  The first one being 0 and everybody else increments by 1
is a good enough clue that you cannot arbitrarily reshuffle them
(as opposed to everbody left to the default).  After being
warned with the clue, somebody who is adding new elements to the
array and defining a symbolic index to the element will know to
append to the list, not just insert into an arbitrary place.
Spelling out all the rest explicitly like "color_branch" one
does is wasteful (new entries would need to carry "= (n+1)"),
pointless (you cannot write anything but "= (n+1)" because it is
an array index), and misleading (it makes you wonder if the
specific values have meaning other than being used for an array
index).