[PATCH] Add push --set-upstream

[PATCH] Add push --set-upstream

[Ilari Liusvaara]

Frequent complaint is lack of easy way to set up upstream (tracking)
references for git pull to work as part of push command. So add switch
--set-upstream (-u) to do just that.

Signed-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
---
This is built on top of master.

 Documentation/git-push.txt |    8 +++++++-
 builtin-push.c             |    1 +
 transport.c                |   35 +++++++++++++++++++++++++++++++++++
 transport.h                |    1 +
 4 files changed, 44 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index e3eb1e8..6c68978 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git push' [--all | --mirror | --tags] [-n | --dry-run] [--receive-pack=<git-receive-pack>]
-	   [--repo=<repository>] [-f | --force] [-v | --verbose]
+	   [--repo=<repository>] [-f | --force] [-v | --verbose] [-u | --set-upstream]
 	   [<repository> <refspec>...]
 
 DESCRIPTION
@@ -122,6 +122,12 @@ nor in any Push line of the corresponding remotes file---see below).
 	the name "origin" is used. For this latter case, this option
 	can be used to override the name "origin". In other words,
 	the difference between these two commands
+
+-u::
+--set-upstream::
+	For every branch that is up to date or successfully pushed, add
+	upstream (tracking) reference for argument-less git pull.
+
 +
 --------------------------
 git push public         #1
diff --git a/builtin-push.c b/builtin-push.c
index 28a26e7..75ddaf4 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -218,6 +218,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		OPT_BOOLEAN( 0 , "thin", &thin, "use thin pack"),
 		OPT_STRING( 0 , "receive-pack", &receivepack, "receive-pack", "receive pack program"),
 		OPT_STRING( 0 , "exec", &receivepack, "receive-pack", "receive pack program"),
+		OPT_BIT('u', "set-upstream", &flags, "Set upstream for git pull", TRANSPORT_PUSH_SET_UPSTREAM),
 		OPT_END()
 	};
 
diff --git a/transport.c b/transport.c
index b5332c0..956d2ed 100644
--- a/transport.c
+++ b/transport.c
@@ -8,6 +8,7 @@
 #include "bundle.h"
 #include "dir.h"
 #include "refs.h"
+#include "branch.h"
 
 /* rsync support */
 
@@ -135,6 +136,33 @@ static void insert_packed_refs(const char *packed_refs, struct ref **list)
 	}
 }
 
+static void set_upstreams(struct transport *trans, struct ref *refs)
+{
+	struct ref *i;
+	for (i = refs; i; i = i->next) {
+		/*
+		 * Check suitability for tracking. Must be successful /
+		 * alreay up-to-date ref create/modify (not delete) and
+		 * both sides must be local branches.
+		 */
+		if (i->status != REF_STATUS_OK &&
+			i->status != REF_STATUS_UPTODATE)
+			continue;
+		if (!i->peer_ref)
+			continue;
+		if (!i->new_sha1 || is_null_sha1(i->new_sha1))
+			continue;
+		if (prefixcmp(i->peer_ref->name, "refs/heads/"))
+			continue;
+		if (prefixcmp(i->name, "refs/heads/"))
+			continue;
+
+		install_branch_config(BRANCH_CONFIG_VERBOSE,
+			i->peer_ref->name + 11, trans->remote->name,
+			i->name);
+	}
+}
+
 static const char *rsync_url(const char *url)
 {
 	return prefixcmp(url, "rsync://") ? skip_prefix(url, "rsync:") : url;
@@ -974,6 +1002,10 @@ int transport_push(struct transport *transport,
 	verify_remote_names(refspec_nr, refspec);
 
 	if (transport->push) {
+		/* Maybe FIXME. But no important transport uses this case. */
+		if (flags & TRANSPORT_PUSH_SET_UPSTREAM)
+			die("This transport does not support using --set-upstream");
+
 		return transport->push(transport, refspec_nr, refspec, flags);
 	} else if (transport->push_refs) {
 		struct ref *remote_refs =
@@ -1002,6 +1034,9 @@ int transport_push(struct transport *transport,
 					verbose | porcelain, porcelain,
 					nonfastforward);
 
+		if (flags & TRANSPORT_PUSH_SET_UPSTREAM)
+			set_upstreams(transport, remote_refs);
+
 		if (!(flags & TRANSPORT_PUSH_DRY_RUN)) {
 			struct ref *ref;
 			for (ref = remote_refs; ref; ref = ref->next)
diff --git a/transport.h b/transport.h
index 97ba251..c4314dd 100644
--- a/transport.h
+++ b/transport.h
@@ -91,6 +91,7 @@ struct transport {
 #define TRANSPORT_PUSH_VERBOSE 16
 #define TRANSPORT_PUSH_PORCELAIN 32
 #define TRANSPORT_PUSH_QUIET 64
+#define TRANSPORT_PUSH_SET_UPSTREAM 128
 
 /* Returns a transport suitable for the url */
 struct transport *transport_get(struct remote *, const char *);
-- 
1.6.6.102.gd6f8f.dirty

Re: [PATCH] Add push --set-upstream

[Matthieu Moy]

Ilari Liusvaara <ilari.liusvaara@elisanet.fi> writes:

> +
> +-u::
> +--set-upstream::
> +	For every branch that is up to date or successfully pushed, add
> +	upstream (tracking) reference for argument-less git pull.

Not just argument-less git pull. git status is also impacted for
example. Actually, we already have documentation for it in git-branch
(--track option), and git-config (branch.<name>.merge configuration
variable), so you should add a pointer to one of them.

How about

--set-upstream::
	For every branch that is up to date or successfully pushed, add
	upstream (tracking) reference, used by argument-less
	linkgit:git-pull[1] and other commands. For more information,
	see 'branch.<name>.merge' in linkgit:git-config[1].

?

> +		OPT_BIT('u', "set-upstream", &flags, "Set upstream for git pull", TRANSPORT_PUSH_SET_UPSTREAM),

I'd be in favor of --track for the option name. Not that it's the best
name ever, but this is really doing the same job as branch --track and
checkout --track, so it should have the same name.

Or the --track option of branch and checkout should be renamed as
--set-upstream, but that seems a lot of trouble for little benefit.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] Add push --set-upstream

[Johannes Schindelin]

Hi,

On Fri, 15 Jan 2010, Matthieu Moy wrote:

> Ilari Liusvaara <ilari.liusvaara@elisanet.fi> writes:
> 
> > +		OPT_BIT('u', "set-upstream", &flags, "Set upstream for git pull", TRANSPORT_PUSH_SET_UPSTREAM),
> 
> I'd be in favor of --track for the option name. Not that it's the best
> name ever, but this is really doing the same job as branch --track and
> checkout --track, so it should have the same name.
> 
> Or the --track option of branch and checkout should be renamed as
> --set-upstream, but that seems a lot of trouble for little benefit.

Junio already complained that it is a bad name, basically because it is 
ambiguous.  If you are really pushing for --track, you will have to think 
of a compelling reasoning.

Ciao,
Dscho

P.S.: FWIW I think --set-upstream is really a good name.

Re: [PATCH] Add push --set-upstream

[Jeff King]

On Fri, Jan 15, 2010 at 06:36:47PM +0200, Ilari Liusvaara wrote:

> Frequent complaint is lack of easy way to set up upstream (tracking)
> references for git pull to work as part of push command. So add switch
> --set-upstream (-u) to do just that.

Huzzah, finally this feature is done right. I even like the name.

>  Documentation/git-push.txt |    8 +++++++-
>  builtin-push.c             |    1 +
>  transport.c                |   35 +++++++++++++++++++++++++++++++++++
>  transport.h                |    1 +
>  4 files changed, 44 insertions(+), 1 deletions(-)

No tests. But since in writing this you have crossed an item off of my
long-term todo, I feel obliged to help out by providing some. :)

The patch below is squash-able, but note that the final test, "git push
-u HEAD" is marked as broken. We should probably support that. I suspect
is is an issue of dereferencing symrefs before doing the
prefixcmp("refs/heads/", ...) but I haven't checked yet.

diff --git a/t/t5523-push-upstream.sh b/t/t5523-push-upstream.sh
new file mode 100755
index 0000000..e977553
--- /dev/null
+++ b/t/t5523-push-upstream.sh
@@ -0,0 +1,48 @@
+#!/bin/sh
+
+test_description='push with --set-upstream'
+. ./test-lib.sh
+
+test_expect_success 'setup bare parent' '
+	git init --bare parent &&
+	git remote add upstream parent
+'
+
+test_expect_success 'setup local commit' '
+	echo content >file &&
+	git add file &&
+	git commit -m one
+'
+
+check_config() {
+	(echo $2; echo $3) >expect.$1
+	(git config branch.$1.remote
+	 git config branch.$1.merge) >actual.$1
+	test_cmp expect.$1 actual.$1
+}
+
+test_expect_success 'push -u master:master' '
+	git push -u upstream master:master &&
+	check_config master upstream refs/heads/master
+'
+
+test_expect_success 'push -u master:other' '
+	git push -u upstream master:other &&
+	check_config master upstream refs/heads/other
+'
+
+test_expect_success 'push -u --all' '
+	git branch all1 &&
+	git branch all2 &&
+	git push -u --all &&
+	check_config all1 upstream refs/heads/all1 &&
+	check_config all2 upstream refs/heads/all2
+'
+
+test_expect_failure 'push -u HEAD' '
+	git checkout -b headbranch &&
+	git push -u upstream HEAD &&
+	check_config headbranch upstream refs/heads/headbranch
+'
+
+test_done

Re: [PATCH] Add push --set-upstream

[Jeff King]

On Fri, Jan 15, 2010 at 12:17:45PM -0500, Jeff King wrote:

> The patch below is squash-able, but note that the final test, "git push
> -u HEAD" is marked as broken. We should probably support that. I suspect
> is is an issue of dereferencing symrefs before doing the
> prefixcmp("refs/heads/", ...) but I haven't checked yet.

The patch below fixes it, but I am not 100% happy with it. Calling
resolve_ref means we actually bother to look up the ref again, which is
wasted effort. The ref struct has a "symref" field which should contain
this information, but for some reason it is not recorded. So we can
probably do better by simply recording the information properly when we
resolve the ref in the first place.

Unfortunately, I don't have time to look at it anymore right now, so it
will have to wait.

diff --git a/t/t5523-push-upstream.sh b/t/t5523-push-upstream.sh
index e977553..d43473f 100755
--- a/t/t5523-push-upstream.sh
+++ b/t/t5523-push-upstream.sh
@@ -39,7 +39,7 @@ test_expect_success 'push -u --all' '
 	check_config all2 upstream refs/heads/all2
 '
 
-test_expect_failure 'push -u HEAD' '
+test_expect_success 'push -u HEAD' '
 	git checkout -b headbranch &&
 	git push -u upstream HEAD &&
 	check_config headbranch upstream refs/heads/headbranch
diff --git a/transport.c b/transport.c
index 956d2ed..01ff364 100644
--- a/transport.c
+++ b/transport.c
@@ -140,6 +140,7 @@ static void set_upstreams(struct transport *trans, struct ref *refs)
 {
 	struct ref *i;
 	for (i = refs; i; i = i->next) {
+		const char *branch;
 		/*
 		 * Check suitability for tracking. Must be successful /
 		 * alreay up-to-date ref create/modify (not delete) and
@@ -152,14 +153,20 @@ static void set_upstreams(struct transport *trans, struct ref *refs)
 			continue;
 		if (!i->new_sha1 || is_null_sha1(i->new_sha1))
 			continue;
-		if (prefixcmp(i->peer_ref->name, "refs/heads/"))
-			continue;
 		if (prefixcmp(i->name, "refs/heads/"))
 			continue;
 
+		if (!prefixcmp(i->peer_ref->name, "refs/heads/"))
+			branch = i->peer_ref->name;
+		else {
+			unsigned char sha1[20];
+			branch = resolve_ref(i->peer_ref->name, sha1, 1, NULL);
+			if (!branch || prefixcmp(branch, "refs/heads/"))
+				continue;
+		}
+
 		install_branch_config(BRANCH_CONFIG_VERBOSE,
-			i->peer_ref->name + 11, trans->remote->name,
-			i->name);
+			branch + 11, trans->remote->name, i->name);
 	}
 }
 

Re: [PATCH] Add push --set-upstream

[Junio C Hamano]

Very competently done.  Will look forward to see the conclusion of the
follow-up discussions I haven't read but see in my mailreader.

Thanks.

Re: [PATCH] Add push --set-upstream

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Ilari Liusvaara <ilari.liusvaara@elisanet.fi> writes:
>
>> +
>> +-u::
>> +--set-upstream::
>> +	For every branch that is up to date or successfully pushed, add
>> +	upstream (tracking) reference for argument-less git pull.
>
> Not just argument-less git pull. git status is also impacted for
> example. Actually, we already have documentation for it in git-branch
> (--track option), and git-config (branch.<name>.merge configuration
> variable), so you should add a pointer to one of them.
>
> How about
>
> --set-upstream::
> 	For every branch that is up to date or successfully pushed, add
> 	upstream (tracking) reference, used by argument-less
> 	linkgit:git-pull[1] and other commands. For more information,
> 	see 'branch.<name>.merge' in linkgit:git-config[1].

Yeah, if we talked only about the configuration variable it wouldn't help
users, but this description is reayy good.

> Or the --track option of branch and checkout should be renamed as
> --set-upstream, but that seems a lot of trouble for little benefit.

It might make sense to do so in the longer term UI clean-up, but I think
that is largely an independent topic.  By not introducing new places the
confusing --track is used, we are at least not making things worse.

Re: [PATCH] Add push --set-upstream

[Ilari Liusvaara]

On Fri, Jan 15, 2010 at 12:17:45PM -0500, Jeff King wrote:
> On Fri, Jan 15, 2010 at 06:36:47PM +0200, Ilari Liusvaara wrote:
> 
> No tests. But since in writing this you have crossed an item off of my
> long-term todo, I feel obliged to help out by providing some. :)
 
Signoff for those tests? Or do they need it anyway?

<Snip testscript> 

Re: [PATCH] Add push --set-upstream

[Jeff King]

On Sat, Jan 16, 2010 at 12:06:58AM +0200, Ilari Liusvaara wrote:

> On Fri, Jan 15, 2010 at 12:17:45PM -0500, Jeff King wrote:
> > On Fri, Jan 15, 2010 at 06:36:47PM +0200, Ilari Liusvaara wrote:
> > 
> > No tests. But since in writing this you have crossed an item off of my
> > long-term todo, I feel obliged to help out by providing some. :)
>  
> Signoff for those tests? Or do they need it anyway?

Sorry, yes:

Signed-off-by: Jeff King <peff@peff.net>

-Peff