patchgit push --track

Hi,

I'd like a feature to automatically "transform" a non-tracking local branch into a tracking branch on push. A patch to do that is attached.

Usage:

git branch mybranch git checkout mybranch ... git push --track origin mybranch:mybranch

will not just perform the push, but also write a block

[branch "mybranch"]
         remote = origin
         merge = refs/heads/mybranch

to the git configuration so the branch becomes tracking.

This should be a simpler alternative to the otherwise usual procedure

git push origin mybranch:mybranch git config branch.mybranch.remote origin git config branch.mybranch.merge refs/heads/mybranch

Are there any chances for this getting added to official git - or an alternate convenient way convert a local to a tracking branch?

Best regards,

Rudolf

On Wed, Jan 13, 2010 at 04:12:49PM +0100, Rudolf Polzer wrote:

The patches should be sent inline, together with commit messages (unless you are asked to resend as attachment because of whitespace mangling). Attached patches are very hard to comment on.

This is missing sign-off. It can't be included without it.

Also couple comments:

- Some lines look way too long (~160 chars, should be max 80 unles
it would linebreak error message).
- Should the tracking be set up even if only part of ref update suceeded
(for those that succeeded), not requiring all to succeed?
- Is --track the best name for this?

-Ilari

On Wed, 13 Jan 2010 16:43:10 +0100, Ilari Liusvaara <ilari.liusvaara@elisanet.fi> wrote:

Of course, but I assume the sign-off would not be by me, but by some of the git developers, and would depend on whether they actually want this feature.

Yes, also I got told that I used the wrong braces style... well, fixed that.

Good point, but I simply see no clean way to set it up for the succeeded refs. Would be a nice idea for improvement of this.

I am assuming this, because this is what git-checkout and git-branch use for the same thing.

As I am absolutely not sure if with Opera I can include the file as is, I also provided it on http://nopaste.linux-dev.org/?6248 this time.

Best regards,

Rudolf

 From 123598516c7d4e1f83591e8dae64e2c76dc87c90 Mon Sep 17 00:00:00 2001
From: Rudolf Polzer <divVerent@alientrap.org>
Date: Wed, 13 Jan 2010 16:42:04 +0100
Subject: [PATCH 1/2] Add a feature "git push --track" to automatically  
make the pushed branches tracking

---
  builtin-push.c |   33 +++++++++++++++++++++++++++++++++
  transport.h    |    1 +
  2 files changed, 34 insertions(+), 0 deletions(-)

On Wed, Jan 13, 2010 at 04:55:20PM +0100, Rudolf Polzer wrote:

It would need sign-off by you. Even if you took the code from somewhere
(and then it would need theirs as well) and passed it along.
 

Ah, that is only known in transport_push and what it calls (and transport_push
is last point to insert common functionality)...
 

Linewrap damage.

-Ilari

"Rudolf Polzer" <divVerent@alientrap.org> writes:

Read Documentation/SubmittingPatches in Git's source to make sure you understand what Signed-off-by: means for the Git project.

Hi,

generally, it would be better if you could add some tests for this.

If I'm not wrong, the place to put it would be t5516-fetch-push.sh.

On Wed, Jan 13, 2010 at 11:55 PM, Rudolf Polzer <divVerent@alientrap.org> wrote:

please don't drop people from the Cc list - especially when you're replying to somebody!

Each patch should be sent out in its own mail. (As Matthieu has recommended, you should check out Documentation/SubmittingPatches.)

It would be better if you can move this to transport.c::transport_push(). It repeats what's already there, so you don't have to configure match_flags, nor call match_refs, etc.

In most places, this is done like this:

  struct ref* ref;
  for (ref = remote_refs; ref; ref = ref->next) {
    ...
  }

On Thu, Jan 14, 2010 at 01:21:17PM +0800, Tay Ray Chuan wrote:

Can add that, but it seems like it won't go in anyway from the discussion here, so it's probably not worth working on it. Sad.

I did not drop anyone, but simply replied from my newsreader. I really don't want to subscribe to a mailing list and then get hundreds of emails a day.

So, using a newsreader is not accepted practice? Why is the mailing list on a newsgroup then?

Then I have to duplicate it in the rsync specific push code too. Otherwise, agreed.

Sure, could do that too, I got this loop from the loop that frees a ref list.

Best regards,

Rudolf

Rudolf Polzer <divVerent@alientrap.org> writes:

I do read this list in a newsreader. When I say "reply" or "follow-up", the response is sent out to the list via e-mail. Your newsreader should be configurable in a similar way, I think.

On Wed, Jan 13, 2010 at 04:55:20PM +0100, Rudolf Polzer wrote:

I don't think it's that hard. In fact, I did a preliminary patch for it about a year ago:

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

That patch was held up because there were a lot of cleanups needed in transport.c before it would make sense (read the whole thread for details). I think most of those cleanups have happened in the meantime, so it would be pretty straightforward to use the same setup_tracking() function and just call it from the right spot in transport.c. But I haven't actually looked at this topic since the above-referenced thread.

-Peff

"Rudolf Polzer" <divVerent@alientrap.org> writes:

You didn't, although you tried to "hide" one level, which is even worse.

If you see overlong lines in patch text, it often is that the added codepath is too deeply nested, and it often becomes much easier to understand if you split it into a separate smaller helper function.

For example, if you have

	if (A) {
        	if (B) {
                	do something #1
                        if (C) {
                        	do something #2
                                while (D) {
                                	if (E) {
                                        	do something #3
					}
				}
			}
		}
	}

it often is much easier to read if you did:

	if (A)
		helper(...)

and wrote a helper that is

	helper()
        {
        	if (!B)
                	return
		do something #1
                if (!C)
                	return
		do something #2
		while (D) {
                	if (!E)
                        	continue
			do something #3
		}
	}

I think giving configuration to the ones that succeeded, while not doing so for the ones that failed, would be the best.

Most probably not. "git branch --track" was already a mistake, whose damage can be seen in the first message in this thread. I originally read "this converts a local branch to a tracking branch", and went "Huh??? --- Is this patch running 'mv refs/heads/frotz refs/remotes/origin/frotz'? What's fun about it???"

Style:

 - Have SP between syntactic keyword and open parenthesis.

 - Never place an opening brace, except the one that begins a function
   body, on its own line;

Also the overlong line is merely a symptom that you are putting too much stuff in this function. The whole addition should probably be a helper function.

You have already pushed by calling transport_push() before you got "err" back. Do you need to make a second, separate call to ls-remote here and if so why?

I have a feeling that it is more appropriate to have the additional code in transport_push(), which gets ls-remote information, runs match_refs() and finally calls transport->push_refs(). I think the extra branch configuration would fit better inside the if block immediately after all that happens, i.e.

	if (!(flags & TRANSPORT_PUSH_DRY_RUN)) {
		struct ref *ref;
		for (ref = remote_refs; ref; ref = ref->next)
			update_tracking_ref(transport->remote, ref, verbose);
+		if (flags & TRANSPORT_PUSH_RECONFIGURE_FORK)
+			configure_forked_branch(...);
	}

in transport.c

Yuck; hiding the fact that you have an over-nested logic is not a way to fix it.

On Thu, Jan 14, 2010 at 09:47:22PM -0800, Junio C Hamano wrote:

I thought about this place when making my patch, but didn't put it there because this function is not called in the rsync protocol (which defines transport->push). So I instead did the logical step and went into the caller of that function. Functionality-wise, it also isn't really a "transport" function but part of pushing.

This was accidental.

But well. Why bother with this, if this feature was rejected before already anyway.

Rudolf

Rudolf Polzer <divVerent@alientrap.org> writes:

It's a good feature and people obviously want it, so it's worth trying.

Sometimes it takes a few tries to get a feature added...

-Miles

Rudolf Polzer <divVerent@alientrap.org> writes:

That's not a very good reasoning. Instead of punishing well behaved transport that defines push_ref, punish _only_ the transports that does not define it (see the paragraph at the end of this message).

Read the thread Nana quoted for you again; I nor anybody ever _rejected_ the ultimate goal, even though I said that the justifications were not sufficiently convincing for the previous implementation attempts.

I think Ilari's patch is done right and can be extended by anybody who cares about rsync transport to call an extra ls-remote in the "does this one lack push_ref but know how to push" codepath.

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

I think "--track" is absolutely the right name for this option -- it's the option name used for "set up tracking" option in other commands, and it's just very natural (when I've thought about implementing a similar functionality myself, I also chose the name "--track").

-Miles

"Rudolf Polzer" <divVerent@alientrap.org> writes:

Yay!!

I've wanted this for a long time, but discussions about it always seem to end up petering out...

Does it default to the current branch so you can just say "git push --track origin"?

I hope this can be added to the distro...

Thanks,

-Miles

Hi,

On Thu, 14 Jan 2010, Miles Bader wrote:

That is not fair. I came up with a patch already years ago. You could always have applied it to your own source, maybe after tweaking. And then maybe lobbying for it.

Ciao, Dscho

On Thu, Jan 14, 2010 at 9:33 AM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Ah, sorry, I didn't mean anything against your patch, I just didn't know about it. I don't track this list as well as I could....

[Do you have a pointer to your patch?]

Thanks,

-Miles

BTW, I was _just_ going to post a message saying "whatever happened to push --track", but then decided to check the list once more, and saw your message...!

-Miles

On Thu, Jan 14, 2010 at 09:25:49AM +0900, Miles Bader wrote:

It does the very same decisions as push. Basically, it is "whatever got pushed, mark as tracking".

Best regards,

Rudolf

On Thu, Jan 14, 2010 at 1:25 AM, Miles Bader <miles@gnu.org> wrote:

Yay too!

<wishlist>Can we take this further and say

 git push --make-a-new-repo-and-track-it --shared
git+ssh://foo.bar/var/git/mynewthing.git master:master

...?

</wishlist>

Of course it'd only work if you have full ssh access, unless the git server learns a new command to mkdir (in sane and approved locations).

cheers,

m

Hi,

On Thu, 14 Jan 2010, Martin Langhoff wrote:

You mean just like http pushing?

You mean a new "init" command a la "git --git-dir=bla.git init", which _does_ mkdir the directory.

Ciao, Dscho

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

I think he meant

  git --git-dir=git+ssh://foo.bar/var/git/mynewthing.git init

which doesn't.

On Thu, Jan 14, 2010 at 3:25 PM, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:

exacto. And push --track, all in one cmd.

Zooper easy for users.

m

On Thu, 14 Jan 2010 15:25:57 +0000, Matthieu Moy wrote:

But 'ssh foo.bar git --git-dir=/var/git/mynewthing.git init' likely would.

Probably missing a --bare somewhere; and I think that repo creation potentially needs too many things (like custom hook or access rights setup) to be done via git push. Especially since you really only need to create an empty repo, and only if you didn't clone from the push target in the first place.

Andreas

Hi,

On Wed, Jan 13, 2010 at 11:12 PM, Rudolf Polzer <divVerent@alientrap.org> wrote:

before I put up my comments on the patch, I wonder if git-push is the best place to add this feature, as git-push usually deals with "pushing" data to another repo.

I think git-branch would be a better place to do this.

Tay Ray Chuan <rctay89@gmail.com> writes:

No.  I think push is absolutely the right place to do it.

I very often create local branches and only later decide that I'd also like to push them to a remote -- but just as often, or more often, I never do so. It would be extremely annoying if I had to make that decision at branch-time.

-Miles

Hi,

On Thu, Jan 14, 2010 at 9:35 AM, Miles Bader <miles@gnu.org> wrote:

I'm not saying that the feature should belong to git-branch and so you have to make this decision at branch time - I'm saying that since this has got to do with modifying branches (sort of), it should be in there.

On Thu, Jan 14, 2010 at 10:37 AM, Tay Ray Chuan <rctay89@gmail.com> wrote:

I, and it appears other people, want to make this association at push-time -- that's almost always when I make the decision "I'd like to track this" -- so the most convenient and intuitive thing would be to have a --track option to push.

Of course there could _also_ be some sort of branch sub-command (or another command) to setup or change tracking state without pushing. But "push --track" is more important for normal usage I think.

-miles

On Thu, Jan 14, 2010 at 9:49 AM, Miles Bader <miles@gnu.org> wrote:

Ok, I see your point.

On Thu, Jan 14, 2010 at 09:27:26AM +0800, Tay Ray Chuan wrote:

I think git-branch can already do this: after pushing, you can do git branch -f --track origin/mybranch.

But the goal of this is to postponing the decision to track to the push time, and adding as little as possible extra commands/options to do this.

Rudolf Polzer <divVerent@alientrap.org> writes:

Thinking about this again (when was the last time we discussed it?), I like the "git branch -f" suggestion (modulo one small nit).

Yes, "push --track" lets you postpone the decision; branching, working on it, pushing it out _and_ _then_ using your "branch -f" trick will let you postpone the decision even further. And it doesn't add --track to the UI.

The small nit is that "branch -f --track me origin/me" will happily overwrite "me", even when your "me" is not up to date with "origin/me", losing commits.

Perhaps we could teach "branch --track me origin/me" (i.e. no "-f") not to barf even when "me" exists, as long as "me" is a subset of "origin/me", and treat it as a request to re-configure the upstream information for the existing branch "me" and at the same time fast-forward it to "origin/me"?

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

Nonetheless, "push --track" is by far the most natural UI for the most common case, I think.

That's not a positive...

None of this is _necessary_, it's to make git more convenient.

Halfway-measures seem like "oh the user can just use this branch command" seem like, well, halfway measures. Sure, it would be nice to have a branch command _too_, just to make it easier for branch maintenance, but it's not what's really wanted.

-Miles

Miles Bader <miles@gnu.org> writes:

Oh, that is definitely a *HUGE* plus. I wouldn't go so far as to say that the word --track was a mistake. But the thing is, unfortunately it has already been contaminated by people using it in two completely different ways and ended up confusing new people. Some use it to mean "this branch forked off of and builds on top of", and others use it to mean "this ref holds a copy for reference purposes".

On Sat, Jan 16, 2010 at 3:18 AM, Junio C Hamano <gitster@pobox.com> wrote:

Then the right thing to do is to rename existing uses of --track so that they're not confusing. Much-wanted functionality should not be rejected simply because of an earlier mistake which is essentially orthogonal to it, and which can be corrected easily enough.

So let's just say we're discussing the semantics, and it will use another option name, which will be then retrofitted to other similar uses for consistency (as should surely be done regardless of what happens with this command):

   git push --GRUBBLENUT

-Miles

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

And another issue is:

$ git branch -f --track my-branch origin/my-branch fatal: Cannot force update the current branch. $ git branch --track my-branch origin/my-branch fatal: A branch named 'my-branch' already exists.

Actually, I just can't find a natural set of commands doing:

1. create a branch (git checkout -b)
2. work on it
3. send it upstream (git push)
4. set the upstream as tracking (???)

with the current version of Git. I just do 4. with $EDITOR .git/config ...

+1, and in addition, allow doing this on the checkout branch if it doesn't actually change the reference (i.e. touch .git/config, not .git/refs/...).

Quoting Rudolf Polzer <divVerent@alientrap.org>

How well does this take earlier discussions on the same topic into account? For example, did you study the design discussion in
  http://thread.gmane.org/gmane.comp.version-control.git/135325/focus=135390

On Thu, Jan 14, 2010 at 03:41:54PM +0900, Nanako Shiraishi wrote:

I don't really think this has much to do with the other. git branch --will-track still means one needs to know it at branch setup time, and git pull --remember still means one needs to type way more stuff than with a simple push --track.

But well, given the discussion here I see the feature is essentially rejected, and already was rejected a previous time. Will probably forget about this and make a shell script that does for ME what I want.

Best regards,

Rudolf

Hi,

On Thu, 14 Jan 2010, Nanako Shiraishi wrote:

Thank you for looking up that reference.

Do you remember what the outcome was? (Peff mentioned that there was some talk about putting this code into transport.c and some needed restructurings, but I do not remember the details, and I did not have time to follow the development of that file in the recent months.)

Ciao, Dscho

Quoting Johannes Schindelin <Johannes.Schindelin@gmx.de>

I summarized it when I reminded Junio on this topic last time and it is in the same discussion thread: http://thread.gmane.org/gmane.comp.version-control.git/135325/focus=136216

Here is an extended version.

'git branch --will-track origin/topic topic origin/master' was proposed as a way to fork a topic branch at origin's master. Later 'git pull' will merge topic from origin to topic.

This is bad in two ways. It will force users to decide when the branch is created. It will not allow users to configure branch.topic.rebase variable.

'git push --track' was suggested as a way to let users delay that decision.

'git branch --configure' to update the same information for an existing branch was suggested as an alternative UI. An added benefit is that this approach will allow the same option to be used when creating a branch.

'git pull --remember' that remembers the options used from the command line was suggested as a solution in addition to 'git branch --reconfigure'. Users can postpone the decision even more than 'git push --track', and it naturally supports setting branch.topic.rebase with 'git pull --rebase --remember'.  It also has two additional benefits. 'push --track' configures what happens when you 'pull' (counter-intuitive), but 'pull --remember' makes 'pull' to change the setting used by 'pull' (much more natural). Also it does not add the confusing word 'track' to the interface (for a more detailed discussion on 'track', see http://article.gmane.org/gmane.comp.version-control.git/136785).

Nanako Shiraishi <nanako3@lavabit.com> writes:

Please wrap your lines for readability.

Although I admit I was the one who suggested it, and I think that it is the most natural way to do this from the end user's point of view, from the implementation point of view, it is the most difficult one in its current form. "git pull" does not interpret refspecs and delegates all the hard work to "git fetch".

On Fri, Jan 15, 2010 at 07:27:41AM +0900, Nanako Shiraishi wrote:

Still requires you to specify the remote and the branch name twice.

So the workflow would be:

git push origin localbranch:remotebranch ... git pull --remember origin remotebranch:localbranch

instead of

git push --track origin localbranch:remotebranch ... git pull

The one thing I want to avoid, is specifying the "origin localbranch:remotebranch" stuff twice.

Doesn't make git pull --remember a bad idea, it's good in many other cases. But in my specific use case, git push --track is the most useful one.

Rudolf

Hi,

On Fri, 15 Jan 2010, Rudolf Polzer wrote:

Thanks for the very nice summary!

The thing is: done right, the three can share the major part of the code.

Ciao, Dscho