[PATCH] push: Learn to set up branch tracking with '--track'

[PATCH] push: Learn to set up branch tracking with '--track'

[Johannes Schindelin]

When pushing a branch to a remote repository that the remote side did
not know beforehand, it is often handy to set up the branch tracking
such that

	$ git checkout xyz
	$ git push --track origin xyz:abc
	$ git pull

will pull the branch 'abc' from the remote 'origin' into the branch
'xyz'.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	This is a companion patch to the one I sent earlier:

	http://article.gmane.org/gmane.comp.version-control.git/13735

 Documentation/git-push.txt |    7 ++++++-
 builtin-push.c             |   42 ++++++++++++++++++++++++++++++++++++++++++
 t/t5516-fetch-push.sh      |   11 +++++++++++
 3 files changed, 59 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 7d1eced..fa1d54c 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git push' [--all | --mirror] [--dry-run] [--tags] [--receive-pack=<git-receive-pack>]
-	   [--repo=<repository>] [-f | --force] [-v | --verbose]
+	   [--repo=<repository>] [-f | --force] [-v | --verbose] [-t | --track]
 	   [<repository> <refspec>...]
 
 DESCRIPTION
@@ -126,6 +126,11 @@ useful if you write an alias or script around 'git-push'.
 	transfer spends extra cycles to minimize the number of
 	objects to be sent and meant to be used on slower connection.
 
+-t::
+--track::
+	Set up tracking information for the pushed branches, so that
+	'git pull' will remember the indicated mapping.
+
 -v::
 --verbose::
 	Run verbosely.
diff --git a/builtin-push.c b/builtin-push.c
index 122fdcf..9fd445d 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -16,6 +16,7 @@ static const char * const push_usage[] = {
 
 static int thin;
 static const char *receivepack;
+static int track;
 
 static const char **refspec;
 static int refspec_nr;
@@ -48,6 +49,41 @@ static void set_refspecs(const char **refs, int nr)
 	}
 }
 
+static void setup_tracking(const char *url)
+{
+	struct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT;
+	int i;
+
+	for (i = 0; i < refspec_nr; i++) {
+		const char *branch = refspec[i], *colon;
+
+		/* skip non-branches */
+		if (!prefixcmp("refs/", branch)) {
+			if (prefixcmp("refs/heads/", branch))
+				continue;
+			branch += strlen("refs/heads/");
+		}
+		colon = strchrnul(branch, ':');
+
+		strbuf_reset(&buf);
+		strbuf_addf(&buf, "branch.%.*s.remote",
+				(int)(colon - branch), branch);
+		git_config_set(buf.buf, url);
+
+		strbuf_reset(&buf);
+		strbuf_addf(&buf, "branch.%.*s.merge",
+				(int)(colon - branch), branch);
+		strbuf_reset(&buf2);
+		strbuf_addf(&buf2, "%s%s",
+			*colon && !prefixcmp("refs/heads/", colon + 1) ?
+			"" : "refs/heads/",
+			*colon ? colon + 1 : branch);
+		git_config_set(buf.buf, buf2.buf);
+	}
+	strbuf_release(&buf);
+	strbuf_release(&buf2);
+}
+
 static int do_push(const char *repo, int flags)
 {
 	int i, errs;
@@ -96,6 +132,8 @@ static int do_push(const char *repo, int flags)
 		if (flags & TRANSPORT_PUSH_VERBOSE)
 			fprintf(stderr, "Pushing to %s\n", remote->url[i]);
 		err = transport_push(transport, refspec_nr, refspec, flags);
+		if (!err && track)
+			setup_tracking(transport->url);
 		err |= transport_disconnect(transport);
 
 		if (!err)
@@ -126,11 +164,15 @@ 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_BOOLEAN('t', "track", &track, "set up branch tracking information"),
 		OPT_END()
 	};
 
 	argc = parse_options(argc, argv, options, push_usage, 0);
 
+	if (track && !tags && !argc)
+		die ("Need explicit arguments for branch tracking");
+
 	if (tags)
 		add_refspec("refs/tags/*");
 
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 4426df9..e18b2f6 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -573,4 +573,15 @@ test_expect_success 'push with branches containing #' '
 	git checkout master
 '
 
+test_expect_success 'push --track' '
+
+	git push --track testrepo master &&
+	test ! -z "$(git ls-remote testrepo master)" &&
+	test "testrepo" = $(git config branch.master.remote) &&
+	test "refs/heads/master" = $(git config branch.master.merge) &&
+	git push --track testrepo master:test &&
+	test "refs/heads/test" = $(git config branch.master.merge)
+
+'
+
 test_done
-- 
1.6.1.1.506.gdbe181

Re: [PATCH] push: Learn to set up branch tracking with '--track'

[Sverre Rabbelier]

On Thu, Jan 29, 2009 at 12:45, Sverre Rabbelier <srabbelier@gmail.com> wrote:
> Mhhh, so maybe we want a way to set up tracking branches when pushing,
> yes? From what I've seen a patch to do that shouldn't be too hard, so
> if there's interest in that I could look into that.

On Thu, Jan 29, 2009 at 14:38, Johannes Schindelin
<johannes.schindelin@gmx.de> wrote:
>        $ git checkout xyz
>        $ git push --track origin xyz:abc
>        $ git pull

Am I reading this correctly in that you beat me to the patch I
mentioned earlier in reply to Junio and Peff?

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH] push: Learn to set up branch tracking with '--track'

[Johannes Schindelin]

Hi,

On Thu, 29 Jan 2009, Johannes Schindelin wrote:

> 	This is a companion patch to the one I sent earlier:
> 
> 	http://article.gmane.org/gmane.comp.version-control.git/13735

Hmm, maybe I should explain a little better.

That patch tried to make 'git pull' more convenient, that you could set up 
tracking information with a single flag to an explicit pull.

This patch does the same for 'git push', although I agree that the 'pull' 
patch should be done rather differently.

Ciao,
Dscho

Re: [PATCH] push: Learn to set up branch tracking with '--track'

[Johannes Schindelin]

Hi,

On Thu, 29 Jan 2009, Sverre Rabbelier wrote:

> On Thu, Jan 29, 2009 at 12:45, Sverre Rabbelier <srabbelier@gmail.com> wrote:
> > Mhhh, so maybe we want a way to set up tracking branches when pushing,
> > yes? From what I've seen a patch to do that shouldn't be too hard, so
> > if there's interest in that I could look into that.
> 
> On Thu, Jan 29, 2009 at 14:38, Johannes Schindelin
> <johannes.schindelin@gmx.de> wrote:
> >        $ git checkout xyz
> >        $ git push --track origin xyz:abc
> >        $ git pull
> 
> Am I reading this correctly in that you beat me to the patch I
> mentioned earlier in reply to Junio and Peff?

I have to admit that due to time constraints, I did not follow your 
discussion closely.

I just just remembered that 'jast' mentioned something like this on IRC, 
and it seemed that it was easy to do.  Now you go and point out all those 
mistakes in my code, okay?

Ciao,
Dscho

Re: [PATCH] push: Learn to set up branch tracking with '--track'

[Jeff King]

On Thu, Jan 29, 2009 at 02:38:43PM +0100, Johannes Schindelin wrote:

> When pushing a branch to a remote repository that the remote side did
> not know beforehand, it is often handy to set up the branch tracking
> such that
> 
> 	$ git checkout xyz
> 	$ git push --track origin xyz:abc
> 	$ git pull
> 
> will pull the branch 'abc' from the remote 'origin' into the branch
> 'xyz'.

What if "xyz" is a tag? Right now, the patch generates a bogus config
section. Yes, you can say "don't use --track, then". But I can think two
reasons why that isn't acceptable:

  1. You might be pushing multiple things, one of which is a branch
     and one of which is not.

  2. This is likely to grow a related config variable, similar to
     branch.autosetupmerge.

How about "git push --track origin HEAD"? This also generates bogus
config. How about "git push --track origin refs/heads/*:refs/heads/*"?
Also bogus.

It looks like you catch "git push --track --all". So at least there is
no bogus config generated. But it would be nice to translate this to
"create a tracking branch for everything I pushed".

So I think this patch is going about it the wrong way. Instead of
parsing the refspec, I think you actually want to look at what we _do_
push (or at least try to push -- probably even uptodate refs should also
have tracking established), and use that. Then you will have wildcards
expanded, --all handled, etc. And I suspect all you have to do is
iterate over the result of match_refs (which we call later), which
should be even easier (because you don't have to parse the refspecs
yourself). But I haven't looked carefully.

> +		if (!err && track)
> +			setup_tracking(transport->url);

Wouldn't the remote name (if we have one) be preferable to the URL? As
it is, doing "git push --track origin master" results in

  [branch "master"]
    remote = $YOUR_URL_FOR_ORIGIN
    merge = refs/heads/master

which means that:

  1. If you ever change remote.origin.url, it will not propagate to your
     branch section.

  2. When you fetch, you are fetching into FETCH_HEAD, meaning your
     tracking refs are not updated.

-Peff

Re: [PATCH] push: Learn to set up branch tracking with '--track'

[Jeff King]

On Thu, Jan 29, 2009 at 05:33:08PM -0500, Jeff King wrote:

> So I think this patch is going about it the wrong way. Instead of
> parsing the refspec, I think you actually want to look at what we _do_
> push (or at least try to push -- probably even uptodate refs should also
> have tracking established), and use that. Then you will have wildcards
> expanded, --all handled, etc. And I suspect all you have to do is
> iterate over the result of match_refs (which we call later), which
> should be even easier (because you don't have to parse the refspecs
> yourself). But I haven't looked carefully.

Something like the patch below (which is obviously missing all of the
infrastructure for doing this optionally, but is meant to illustrate
what I'm talking about).

The downside of this is that it only works for the git protocol
transport, making dumb push even more of a second class citizen (it
looks like this is already the case for updating tracking refs). But I
think this is the right place to do it, since we have detailed
information on the matched refs. If other transports want to do the same
thing, we should abstract setup_tracking (and update_tracking_ref while
we're at it) and call them from those transports.

---
diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index d65d019..23b326a 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -247,6 +247,31 @@ static void update_tracking_ref(struct remote *remote, struct ref *ref)
 	}
 }
 
+static void setup_tracking(const char *remote, struct ref *ref)
+{
+	const char *name;
+	struct strbuf key = STRBUF_INIT;
+
+	if (ref->status != REF_STATUS_OK && ref->status != REF_STATUS_UPTODATE)
+		return;
+	if (!ref->peer_ref)
+		return;
+
+	name = ref->peer_ref->name;
+	if (prefixcmp(name, "refs/heads/"))
+		return;
+	name += 11;
+
+	strbuf_addf(&key, "branch.%s.remote", name);
+	git_config_set(key.buf, remote);
+
+	strbuf_reset(&key);
+	strbuf_addf(&key, "branch.%s.merge", name);
+	git_config_set(key.buf, ref->name);
+
+	strbuf_release(&key);
+}
+
 static const char *prettify_ref(const struct ref *ref)
 {
 	const char *name = ref->name;
@@ -523,6 +548,10 @@ static int do_send_pack(int in, int out, struct remote *remote, const char *dest
 		for (ref = remote_refs; ref; ref = ref->next)
 			update_tracking_ref(remote, ref);
 	}
+	if (/* args.track && */ !args.dry_run) {
+		for (ref = remote_refs; ref; ref = ref->next)
+			setup_tracking(remote ? remote->name : dest, ref);
+	}
 
 	if (!refs_pushed(remote_refs))
 		fprintf(stderr, "Everything up-to-date\n");

Re: [PATCH] push: Learn to set up branch tracking with '--track'

[Johannes Schindelin]

Hi,

On Thu, 29 Jan 2009, Jeff King wrote:

> On Thu, Jan 29, 2009 at 05:33:08PM -0500, Jeff King wrote:
> 
> > So I think this patch is going about it the wrong way. Instead of
> > parsing the refspec, I think you actually want to look at what we _do_
> > push (or at least try to push -- probably even uptodate refs should also
> > have tracking established), and use that. Then you will have wildcards
> > expanded, --all handled, etc. And I suspect all you have to do is
> > iterate over the result of match_refs (which we call later), which
> > should be even easier (because you don't have to parse the refspecs
> > yourself). But I haven't looked carefully.
> 
> Something like the patch below (which is obviously missing all of the 
> infrastructure for doing this optionally, but is meant to illustrate 
> what I'm talking about).

Except that you miss http:// and rsync:// protocols.  Those were the 
reasons I did not touch send-pack.

Ciao,
Dscho

Re: [PATCH] push: Learn to set up branch tracking with '--track'

[Jeff King]

On Fri, Jan 30, 2009 at 01:28:38AM +0100, Johannes Schindelin wrote:

> > Something like the patch below (which is obviously missing all of the 
> > infrastructure for doing this optionally, but is meant to illustrate 
> > what I'm talking about).
> 
> Except that you miss http:// and rsync:// protocols.  Those were the 
> reasons I did not touch send-pack.

You didn't comment on the part of my email where I said exactly that,
but that I think this is still the right path forward.

Pushing through those protocols is sorely in need of update (actually,
I thought rsync was all but dead at this point). But http push is
missing the update of tracking refs, the usual status output (it still
has the "Maybe you are not up-to-date and need to pull first?" message
that was removed from send-pack a year and a half ago), and who knows
what other tweaks made to do_send_pack (which it appears to have been
copy-and-pasted from in 2006) in the last few years.

So either we don't care about http-push being consistent with send-pack,
and it is OK to have this feature in one but not the other. Or we do,
and we really need to clean up the current divergence.

-Peff

Re: [PATCH] push: Learn to set up branch tracking with '--track'

[Johannes Schindelin]

Hi,

On Fri, 30 Jan 2009, Jeff King wrote:

> On Fri, Jan 30, 2009 at 01:28:38AM +0100, Johannes Schindelin wrote:
> 
> > > Something like the patch below (which is obviously missing all of the 
> > > infrastructure for doing this optionally, but is meant to illustrate 
> > > what I'm talking about).
> > 
> > Except that you miss http:// and rsync:// protocols.  Those were the 
> > reasons I did not touch send-pack.
> 
> You didn't comment on the part of my email where I said exactly that,
> but that I think this is still the right path forward.
> 
> Pushing through those protocols is sorely in need of update (actually,
> I thought rsync was all but dead at this point). But http push is
> missing the update of tracking refs, the usual status output (it still
> has the "Maybe you are not up-to-date and need to pull first?" message
> that was removed from send-pack a year and a half ago), and who knows
> what other tweaks made to do_send_pack (which it appears to have been
> copy-and-pasted from in 2006) in the last few years.
> 
> So either we don't care about http-push being consistent with send-pack,
> and it is OK to have this feature in one but not the other. Or we do,
> and we really need to clean up the current divergence.

I do not see how your patch to send-pack makes that divergence any 
better, or for that matter, keeps it as bad as it is.

In other words, if you want to give the other protocols at least a 
_chance_ to catch up, you definitely need the support for push --track in 
builtin-push.c or at least in transport.c.

Ciao,
Dscho

Re: [PATCH] push: Learn to set up branch tracking with '--track'

[Jeff King]

On Fri, Jan 30, 2009 at 04:58:25PM +0100, Johannes Schindelin wrote:

> > So either we don't care about http-push being consistent with send-pack,
> > and it is OK to have this feature in one but not the other. Or we do,
> > and we really need to clean up the current divergence.
> 
> I do not see how your patch to send-pack makes that divergence any 
> better, or for that matter, keeps it as bad as it is.

No, it makes it worse. My point was that I am not sure people actually
_care_ that much about the divergence.

> In other words, if you want to give the other protocols at least a 
> _chance_ to catch up, you definitely need the support for push --track in 
> builtin-push.c or at least in transport.c.

But neither of those places has the information to do it _right_. I
think the right thing to do is:

  1. factor out "generic" routines from send-pack, including status
     output formatting and tracking ref updating

  2. refactor http-push to use those routines, bringing it in line with
     send-pack

  3. add --track support in the same generic way, and hook it from both
     transports

I can try to work on this, but I'm not excited about major surgery to
http-push, which I don't have a working test setup for.  I can't bring
myself to care about refactoring rsync, given the recent deprecation
discussion.

If it is going to be added to push or transport, then the transport API
needs refactoring to actually pass out information on what happened
(specifically, how we expanded the refspecs into matching ref pairs).
And maybe that is a more sensible long-term solution, but it is going
involve a lot of changes, too.

-Peff

Re: [PATCH] push: Learn to set up branch tracking with '--track'

[Johannes Schindelin]

Hi,

On Fri, 30 Jan 2009, Jeff King wrote:

> I think the right thing to do is:
> 
>   1. factor out "generic" routines from send-pack, including status
>      output formatting and tracking ref updating
> 
>   2. refactor http-push to use those routines, bringing it in line with
>      send-pack
> 
>   3. add --track support in the same generic way, and hook it from both
>      transports

Now we're thinking along the same lines!

> I can try to work on this, but I'm not excited about major surgery to 
> http-push, which I don't have a working test setup for.

You don't have an apache installed?

> I can't bring myself to care about refactoring rsync, given the recent 
> deprecation discussion.

Don't do it, then.

> If it is going to be added to push or transport, then the transport API 
> needs refactoring to actually pass out information on what happened 
> (specifically, how we expanded the refspecs into matching ref pairs). 
> And maybe that is a more sensible long-term solution, but it is going 
> involve a lot of changes, too.

I think that the --track should just be passed to the transport API, which 
should call the necessary git_config thing itself upon successful push.

Ciao,
Dscho

Re: [PATCH] push: Learn to set up branch tracking with '--track'

[Jeff King]

On Fri, Jan 30, 2009 at 06:05:30PM +0100, Johannes Schindelin wrote:

> > I think the right thing to do is:
> > 
> >   1. factor out "generic" routines from send-pack, including status
> >      output formatting and tracking ref updating
> > 
> >   2. refactor http-push to use those routines, bringing it in line with
> >      send-pack
> > 
> >   3. add --track support in the same generic way, and hook it from both
> >      transports
> 
> Now we're thinking along the same lines!

OK, good.

> > I can try to work on this, but I'm not excited about major surgery to 
> > http-push, which I don't have a working test setup for.
> 
> You don't have an apache installed?

No, though "apt-get install apache" is easy enough. I was more concerned
about wading through the mess of apache configuration to turn on webdave
support. But that is just my empty grumbling and complaining; it's not a
real stumbling block to doing this patch.

And when I wrote that, I was fully intending to pick up this topic and
work on the steps outlined above.

_But_.

It looks like the consensus is to add a branch.master config section
even when cloning an empty repo. And that should address my concern in
the 99% of cases where people use the default "master" setup. Which kind
of takes away the main use case for this topic.

So it doesn't make much sense to me to put effort into it now.  The
http-push cleanups might be nice for http-push users, but I don't
remember even seeing a single user request or complaint about it. So I
am not too keen to go cleaning up code that _I_ don't care about, and
that I am not sure anyone _else_ even cares about.

-Peff

Re: [PATCH] push: Learn to set up branch tracking with '--track'

[Johannes Schindelin]

Hi,

On Mon, 2 Feb 2009, Jeff King wrote:

> It looks like the consensus is to add a branch.master config section 
> even when cloning an empty repo. And that should address my concern in 
> the 99% of cases where people use the default "master" setup. Which kind 
> of takes away the main use case for this topic.

>From where I sit, the main use would have been:

	# <hackhackhack>
	$ git init
	$ git add .
	$ git commit -m initial\ revision
	# <hackhackhack>
	# <create a repository on repo.or.cz>
	$ git remote add origin $URL
	$ git push -t origin master

BTW I always wondered if it would make sense to introduce "git commit 
--init" for the first 3 Git commands.  I use them way too often.

Ah, well, I will install an alias on all of my machines.

Ciao,
Dscho

Re: [PATCH] push: Learn to set up branch tracking with '--track'

[Jeff King]

On Mon, Feb 02, 2009 at 02:52:26PM +0100, Johannes Schindelin wrote:

> > It looks like the consensus is to add a branch.master config section 
> > even when cloning an empty repo. And that should address my concern in 
> > the 99% of cases where people use the default "master" setup. Which kind 
> > of takes away the main use case for this topic.
> 
> From where I sit, the main use would have been:
> 
> 	# <hackhackhack>
> 	$ git init
> 	$ git add .
> 	$ git commit -m initial\ revision
> 	# <hackhackhack>
> 	# <create a repository on repo.or.cz>
> 	$ git remote add origin $URL
> 	$ git push -t origin master

Hmm. True. The "clone empty" fix only affects people who use the "init;
clone empty; push" method. Push-tracking would work for the
"traditional" method you outline above.

So probably it is worth doing the steps I described earlier. I'll throw
it on my todo list, but the priority is not especially high.

> BTW I always wondered if it would make sense to introduce "git commit 
> --init" for the first 3 Git commands.  I use them way too often.

I have noticed that, too, but I think it is because I am constantly
creating test repos to debug git. Regular users don't have the same
problem. :)

-Peff

Re: [PATCH] push: Learn to set up branch tracking with '--track'

[Johannes Schindelin]

Hi,

On Fri, 6 Feb 2009, Jeff King wrote:

> > BTW I always wondered if it would make sense to introduce "git commit 
> > --init" for the first 3 Git commands.  I use them way too often.
> 
> I have noticed that, too, but I think it is because I am constantly
> creating test repos to debug git. Regular users don't have the same
> problem. :)

I do not do that for test repositories, but for untarred 3rd party 
projects, such as zlib.

	$ tar xvf $TAR
	$ cd $DIR
	$ git init
	$ git add .
	$ git commit -m initial
	<start hacking>
	<provide nice patches that everybody envies me for>

Ciao,
Dscho