[PATCH 0/3] A better way of handling upstream information in git-branch

DORMANTno replies

6 messages, 2 authors, 2016-06-15 · open the first message on its own page

[PATCH 0/3] A better way of handling upstream information in git-branch

From: Carlos Martín Nieto <hidden>
Date: 2016-06-15 22:54:14

Hello all,

This stems from comments made by Junio and Jonathan about my proposed
changes to --set-upstream.

This should probably have a few tests, but I'd like to hear comments
about the code and documentation first. The third patch is the one I'm
not so confident about. It would be simpler to remove the whole
branch.foo configuration, but that wouldn't be very safe, as we may
have more things there (either the future git or some external tool).

Carlos Martín Nieto (3):
  branch: introduce --set-upstream-to
  branch: suggest how to undo a --set-upstream when given one branch
  branch: add --unset-upstream option

 Documentation/git-branch.txt |   14 +++++++++++-
 builtin/branch.c             |   50 +++++++++++++++++++++++++++++++++++++++---
 2 files changed, 60 insertions(+), 4 deletions(-)

-- 
1.7.10.2.1.g8c77c3c

[PATCH 2/3] branch: suggest how to undo a --set-upstream when given one branch

From: Carlos Martín Nieto <hidden>
Date: 2016-06-15 22:54:14

This interface is error prone, and a better one (--set-upstream-to)
exists. Suggest how to fix a --set-upstream invocation in case the
user only gives one argument, which makes it likely that he meant to
do the opposite, like with

    git branch --set-upstream origin/master

when they meant one of

    git branch --set-upstream origin/master master
    git branch --set-upstream-to origin/master

Signed-off-by: Carlos Martín Nieto <redacted>
---
 builtin/branch.c |   22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/builtin/branch.c b/builtin/branch.c
index c886fc0..5551227 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c

@@ -864,10 +864,32 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		   info and making sure new_upstream is correct */
 		create_branch(head, branch->name, new_upstream, 0, 0, 0, quiet, BRANCH_TRACK_OVERRIDE);
 	} else if (argc > 0 && argc <= 2) {
+		struct branch *branch = branch_get(argv[0]);
+		const char *old_upstream = NULL;
+		int branch_existed = 0;
+
 		if (kinds != REF_LOCAL_BRANCH)
 			die(_("-a and -r options to 'git branch' do not make sense with a branch name"));
+
+		/* Save what argv[0] was pointing to so we can give
+		   the --set-upstream-to hint */
+		if (branch_has_merge_config(branch))
+		  old_upstream = shorten_unambiguous_ref(branch->merge[0]->dst, 0);
+
+		branch_existed = ref_exists(branch->refname);
 		create_branch(head, argv[0], (argc == 2) ? argv[1] : head,
 			      force_create, reflog, 0, quiet, track);
+
+		if (argc == 1) {
+			printf("If you wanted to make '%s' track '%s', do this:\n", head, argv[0]);
+			if (branch_existed)
+				printf(" $ git branch --set-upstream '%s' '%s'\n", argv[0], old_upstream);
+			else
+				printf(" $ git branch -d '%s'\n", argv[0]);
+
+			printf(" $ git branch --set-upstream-to '%s'\n", argv[0]);
+		}
+
 	} else
 		usage_with_options(builtin_branch_usage, options);

-- 
1.7.10.2.1.g8c77c3c

[PATCH 3/3] branch: add --unset-upstream option

From: Carlos Martín Nieto <hidden>
Date: 2016-06-15 22:54:14

We have ways of setting the upstream information, but if we want to
unset it, we need to resort to modifying the configuration manually.

Teach branch an --unset-upstream option that unsets this information.

---

create_branch() uses install_branch_config() which may also set
branch.foo.rebase, so this version might leave some configuration
laying around.

I wonder if deleting the whole branch.foo section would be better. Can
we be sure that nothing else shows up there?

 Documentation/git-branch.txt |    5 +++++
 builtin/branch.c             |   17 ++++++++++++++---
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 0f33fc7..c7cab08 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt

@@ -14,6 +14,7 @@ SYNOPSIS
 	[(--merged | --no-merged | --contains) [<commit>]] [<pattern>...]
 'git branch' [--set-upstream | --track | --no-track] [-l] [-f] <branchname> [<start-point>]
 'git branch' (--set-upstream-to=<upstream> | -u <upstream>) [<branchname>]
+'git branch' --unset-upstream [<branchname>]
 'git branch' (-m | -M) [<oldbranch>] <newbranch>
 'git branch' (-d | -D) [-r] <branchname>...
 'git branch' --edit-description [<branchname>]

@@ -180,6 +181,10 @@ start-point is either a local or remote-tracking branch.
 	considered <branchname>'s upstream branch. If no branch is
 	specified it defaults to the current branch.
 
+--unset-upstream::
+	Remove the upstream information for <branchname>. If no branch
+	is specified it defaults to the current branch.
+
 --edit-description::
 	Open an editor and edit the text to explain what the branch is
 	for, to be used by various other commands (e.g. `request-pull`).

diff --git a/builtin/branch.c b/builtin/branch.c
index 5551227..1d1bf8e 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c

@@ -712,7 +712,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	int delete = 0, rename = 0, force_create = 0, list = 0;
 	int verbose = 0, abbrev = -1, detached = 0;
 	int reflog = 0, edit_description = 0;
-	int quiet = 0;
+	int quiet = 0, unset_upstream = 0;
 	const char *new_upstream = NULL;
 	enum branch_track track;
 	int kinds = REF_LOCAL_BRANCH;

@@ -728,6 +728,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		OPT_SET_INT( 0, "set-upstream",  &track, "change upstream info",
 			BRANCH_TRACK_OVERRIDE),
 		OPT_STRING('u', "set-upstream-to", &new_upstream, "upstream", "change the upstream info"),
+		OPT_BOOLEAN(0, "unset-upstream", &unset_upstream, "Unset the upstream info"),
 		OPT__COLOR(&branch_use_color, "use colored output"),
 		OPT_SET_INT('r', "remotes",     &kinds, "act on remote-tracking branches",
 			REF_REMOTE_BRANCH),

@@ -796,10 +797,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix, options, builtin_branch_usage,
 			     0);
 
-	if (!delete && !rename && !edit_description && !new_upstream && argc == 0)
+	if (!delete && !rename && !edit_description && !new_upstream && !unset_upstream && argc == 0)
 		list = 1;
 
-	if (!!delete + !!rename + !!force_create + !!list + !!new_upstream > 1)
+	if (!!delete + !!rename + !!force_create + !!list + !!new_upstream + !!unset_upstream > 1)
 		usage_with_options(builtin_branch_usage, options);
 
 	if (abbrev == -1)

@@ -863,6 +864,16 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		/* create_branch takes care of setting up the tracking
 		   info and making sure new_upstream is correct */
 		create_branch(head, branch->name, new_upstream, 0, 0, 0, quiet, BRANCH_TRACK_OVERRIDE);
+	} else if (unset_upstream) {
+		struct branch *branch = branch_get(argv[0]);
+		struct strbuf buf = STRBUF_INIT;
+
+		strbuf_addf(&buf, "branch.%s.remote", branch->name);
+		git_config_set_multivar(buf.buf, NULL, NULL, 1);
+		strbuf_reset(&buf);
+		strbuf_addf(&buf, "branch.%s.merge", branch->name);
+		git_config_set_multivar(buf.buf, NULL, NULL, 1);
+		strbuf_release(&buf);
 	} else if (argc > 0 && argc <= 2) {
 		struct branch *branch = branch_get(argv[0]);
 		const char *old_upstream = NULL;

-- 
1.7.10.2.1.g8c77c3c

[PATCH 1/3] branch: introduce --set-upstream-to

From: Carlos Martín Nieto <hidden>
Date: 2016-06-15 22:54:14

The existing --set-uptream option can cause confusion, as it uses the
usual branch convention of assuming a starting point of HEAD if none
is specified, causing

    git branch --set-upstream origin/master

to create a new local branch 'origin/master' that tracks the current
branch. As --set-upstream already exists, we can't simply change its
behaviour. To work around this, introduce --set-upstream-to which
accepts a compulsory argument indicating what the new upstream branch
should be and one optinal argument indicating which branch to change,
defaulting to HEAD.

The new options allows us to type

    git branch --set-upstream-to origin/master

to set the current branch's upstream to be origin's master.
---
 Documentation/git-branch.txt |    9 ++++++++-
 builtin/branch.c             |   15 +++++++++++++--
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 47235be..0f33fc7 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt

@@ -13,6 +13,7 @@ SYNOPSIS
 	[--column[=<options>] | --no-column]
 	[(--merged | --no-merged | --contains) [<commit>]] [<pattern>...]
 'git branch' [--set-upstream | --track | --no-track] [-l] [-f] <branchname> [<start-point>]
+'git branch' (--set-upstream-to=<upstream> | -u <upstream>) [<branchname>]
 'git branch' (-m | -M) [<oldbranch>] <newbranch>
 'git branch' (-d | -D) [-r] <branchname>...
 'git branch' --edit-description [<branchname>]

@@ -48,7 +49,7 @@ branch so that 'git pull' will appropriately merge from
 the remote-tracking branch. This behavior may be changed via the global
 `branch.autosetupmerge` configuration flag. That setting can be
 overridden by using the `--track` and `--no-track` options, and
-changed later using `git branch --set-upstream`.
+changed later using `git branch --set-upstream-to`.
 
 With a `-m` or `-M` option, <oldbranch> will be renamed to <newbranch>.
 If <oldbranch> had a corresponding reflog, it is renamed to match

@@ -173,6 +174,12 @@ start-point is either a local or remote-tracking branch.
 	like `--track` would when creating the branch, except that where
 	branch points to is not changed.
 
+-u <upstream>::
+--set-upstream-to=<upstream>::
+	Set up <branchname>'s tracking information so <upstream> is
+	considered <branchname>'s upstream branch. If no branch is
+	specified it defaults to the current branch.
+
 --edit-description::
 	Open an editor and edit the text to explain what the branch is
 	for, to be used by various other commands (e.g. `request-pull`).

diff --git a/builtin/branch.c b/builtin/branch.c
index 0e060f2..c886fc0 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c

@@ -713,6 +713,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	int verbose = 0, abbrev = -1, detached = 0;
 	int reflog = 0, edit_description = 0;
 	int quiet = 0;
+	const char *new_upstream = NULL;
 	enum branch_track track;
 	int kinds = REF_LOCAL_BRANCH;
 	struct commit_list *with_commit = NULL;

@@ -726,6 +727,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 			BRANCH_TRACK_EXPLICIT),
 		OPT_SET_INT( 0, "set-upstream",  &track, "change upstream info",
 			BRANCH_TRACK_OVERRIDE),
+		OPT_STRING('u', "set-upstream-to", &new_upstream, "upstream", "change the upstream info"),
 		OPT__COLOR(&branch_use_color, "use colored output"),
 		OPT_SET_INT('r', "remotes",     &kinds, "act on remote-tracking branches",
 			REF_REMOTE_BRANCH),

@@ -794,10 +796,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix, options, builtin_branch_usage,
 			     0);
 
-	if (!delete && !rename && !edit_description && argc == 0)
+	if (!delete && !rename && !edit_description && !new_upstream && argc == 0)
 		list = 1;
 
-	if (!!delete + !!rename + !!force_create + !!list > 1)
+	if (!!delete + !!rename + !!force_create + !!list + !!new_upstream > 1)
 		usage_with_options(builtin_branch_usage, options);
 
 	if (abbrev == -1)

@@ -852,6 +854,15 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 			rename_branch(argv[0], argv[1], rename > 1);
 		else
 			usage_with_options(builtin_branch_usage, options);
+	} else if (new_upstream) {
+		struct branch *branch = branch_get(argv[0]);
+
+		if (!ref_exists(branch->refname))
+		  die(_("branch '%s' does not exist"), branch->name);
+
+		/* create_branch takes care of setting up the tracking
+		   info and making sure new_upstream is correct */
+		create_branch(head, branch->name, new_upstream, 0, 0, 0, quiet, BRANCH_TRACK_OVERRIDE);
 	} else if (argc > 0 && argc <= 2) {
 		if (kinds != REF_LOCAL_BRANCH)
 			die(_("-a and -r options to 'git branch' do not make sense with a branch name"));

-- 
1.7.10.2.1.g8c77c3c

Re: [PATCH 1/3] branch: introduce --set-upstream-to

From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:54:14

Hi,

Carlos Martín Nieto wrote:

The existing --set-uptream option can cause confusion, as it uses the
usual branch convention of assuming a starting point of HEAD if none
is specified, causing

    git branch --set-upstream origin/master

to create a new local branch 'origin/master' that tracks the current
branch. As --set-upstream already exists, we can't simply change its
behaviour. To work around this, introduce --set-upstream-to which
accepts a compulsory argument

Thanks.  A part of me really dislikes this --set-upstream-to which
is named more awkwardly than the deprecated mistake it replaces,
though.

Here's a patch on top to play with that names the new option
"--set-upstream=".  Untested.

Signed-off-by: Jonathan Nieder <redacted>
---

diff --git i/Documentation/git-branch.txt w/Documentation/git-branch.txt
index f572913f..57935a64 100644
--- i/Documentation/git-branch.txt
+++ w/Documentation/git-branch.txt

@@ -49,7 +49,7 @@ branch so that 'git pull' will appropriately merge from
 the remote-tracking branch. This behavior may be changed via the global
 `branch.autosetupmerge` configuration flag. That setting can be
 overridden by using the `--track` and `--no-track` options, and
-changed later using `git branch --set-upstream-to`.
+changed later using `git branch --set-upstream`.
 
 With a `-m` or `-M` option, <oldbranch> will be renamed to <newbranch>.
 If <oldbranch> had a corresponding reflog, it is renamed to match

@@ -174,11 +174,13 @@ start-point is either a local or remote-tracking branch.
 	like `--track` would when creating the branch, except that where
 	branch points to is not changed.
 
--u <upstream>::
---set-upstream-to=<upstream>::
+--set-upstream=<upstream>::
 	Set up <branchname>'s tracking information so <upstream> is
 	considered <branchname>'s upstream branch. If no branch is
 	specified it defaults to the current branch.
++
+If no argument is attached, for historical reasons the meaning is
+different.  See above.
 
 --edit-description::
 	Open an editor and edit the text to explain what the branch is

diff --git i/builtin/branch.c w/builtin/branch.c
index c886fc06..0d705790 100644
--- i/builtin/branch.c
+++ w/builtin/branch.c

@@ -669,6 +669,31 @@ static int opt_parse_merge_filter(const struct option *opt, const char *arg, int
 	return 0;
 }
 
+struct set_upstream_params {
+	enum branch_track *track;
+	const char **new_upstream;
+};
+static int parse_opt_set_upstream(const struct option *opt, const char *arg, int unset)
+{
+	struct set_upstream_params *o = opt->value;
+
+	if (unset) {	/* --no-set-upstream */
+		*o->track = BRANCH_TRACK_NEVER;
+		*o->new_upstream = NULL;
+		return 0;
+	}
+
+	*o->track = BRANCH_TRACK_OVERRIDE;
+	if (!arg)	/* --set-upstream <branchname> <start-point> */
+		*o->new_upstream = NULL;
+	else	/* --set-upstream=<upstream> <branchname> */
+		*o->new_upstream = arg;
+	return 0;
+}
+#define OPT_SET_UPSTREAM(s, l, v) \
+	{ OPTION_CALLBACK, (s), (l), (v), "upstream", "change upstream info", \
+	  PARSE_OPT_OPTARG, &parse_opt_set_upstream }
+
 static const char edit_description[] = "BRANCH_DESCRIPTION";
 
 static int edit_branch_description(const char *branch_name)

@@ -716,6 +741,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	const char *new_upstream = NULL;
 	enum branch_track track;
 	int kinds = REF_LOCAL_BRANCH;
+	struct set_upstream_params set_upstream_args = { &track, &new_upstream };
 	struct commit_list *with_commit = NULL;
 
 	struct option options[] = {

@@ -725,9 +751,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		OPT__QUIET(&quiet, "suppress informational messages"),
 		OPT_SET_INT('t', "track",  &track, "set up tracking mode (see git-pull(1))",
 			BRANCH_TRACK_EXPLICIT),
-		OPT_SET_INT( 0, "set-upstream",  &track, "change upstream info",
-			BRANCH_TRACK_OVERRIDE),
-		OPT_STRING('u', "set-upstream-to", &new_upstream, "upstream", "change the upstream info"),
+		OPT_SET_UPSTREAM(0, "set-upstream", &set_upstream_args),
 		OPT__COLOR(&branch_use_color, "use colored output"),
 		OPT_SET_INT('r', "remotes",     &kinds, "act on remote-tracking branches",
 			REF_REMOTE_BRANCH),

Re: [PATCH 2/3] branch: suggest how to undo a --set-upstream when given one branch

From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:54:14

Hi,

Quick nitpicks.

Carlos Martín Nieto wrote:

quoted hunk

--- a/builtin/branch.c
+++ b/builtin/branch.c

@@ -864,10 +864,32 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		   info and making sure new_upstream is correct */
 		create_branch(head, branch->name, new_upstream, 0, 0, 0, quiet, BRANCH_TRACK_OVERRIDE);
 	} else if (argc > 0 && argc <= 2) {
+		struct branch *branch = branch_get(argv[0]);
+		const char *old_upstream = NULL;
+		int branch_existed = 0;
+
 		if (kinds != REF_LOCAL_BRANCH)
 			die(_("-a and -r options to 'git branch' do not make sense with a branch name"));
+
+		/* Save what argv[0] was pointing to so we can give
+		   the --set-upstream-to hint */
+		if (branch_has_merge_config(branch))
+		  old_upstream = shorten_unambiguous_ref(branch->merge[0]->dst, 0);

Whitespace is odd here.  Maybe this case could be factored out as a
new function to make room on the right margin and make cmd_branch()
easier to read straight through.

+
+		branch_existed = ref_exists(branch->refname);
 		create_branch(head, argv[0], (argc == 2) ? argv[1] : head,
 			      force_create, reflog, 0, quiet, track);
+
+		if (argc == 1) {
+			printf("If you wanted to make '%s' track '%s', do this:\n", head, argv[0]);
+			if (branch_existed)
+				printf(" $ git branch --set-upstream '%s' '%s'\n", argv[0], old_upstream);
+			else
+				printf(" $ git branch -d '%s'\n", argv[0]);
+
+			printf(" $ git branch --set-upstream-to '%s'\n", argv[0]);

Message should go on stderr and be guarded with an advice option (see
advice.c).

Like this:

	const char *arg;

	...
	if (argc != 1 || !advice_old_fashioned_set_upstream)
		return 0; /* ok. */

	arg = argv[0];
	advise("If you wanted to make '%s' track '%s', do this:",
							head, arg);
	if (branch_existed)
		advise(" $ git branch --set-upstream-to='%s' '%s'",
			old_upstream, arg);
	else
		advise(" $ git branch -d '%s'", arg);
	advise(" $ git branch --set-upstream-to='%s'", arg);

If an argument contains single-quotes, the quoting will be wrong, but
that's probably not worth worrying about.

Hope that helps,
Jonathan

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help