Re: [PATCH] git push --track

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

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.

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.

On Wed, Jan 13, 2010 at 11:55 PM, Rudolf Polzer [off-list ref] wrote:

quoted

On Wed, 13 Jan 2010 16:43:10 +0100, Ilari Liusvaara
[off-list ref] wrote:

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

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.

quoted

From 123598516c7d4e1f83591e8dae64e2c76dc87c90 Mon Sep 17 00:00:00 2001
From: Rudolf Polzer <redacted>
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

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

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

quoted

 static const char * const push_usage[] = {

@@ -115,6 +116,36 @@ static int push_with_options(struct transport

*transport, int flags)
               fprintf(stderr, "Pushing to %s\n", transport->url);
       err = transport_push(transport, refspec_nr, refspec, flags,
                            &nonfastforward);
+       if (err == 0 && flags & TRANSPORT_PUSH_TRACK) {
+               struct ref *remote_refs =
+                       transport->get_refs_list(transport, 1);
+               struct ref *local_refs = get_local_heads();
+               int match_flags = 0;
+               if (flags & TRANSPORT_PUSH_ALL)
+                       match_flags |= MATCH_REFS_ALL;
+               if (flags & TRANSPORT_PUSH_MIRROR)
+                       match_flags |= MATCH_REFS_MIRROR;
+               if(!(flags & TRANSPORT_PUSH_DRY_RUN))
+               if(!match_refs(local_refs, &remote_refs, refspec_nr,
refspec,
+                                       match_flags)) {

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.

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

quoted

+                       struct ref *next = remote_refs;
+                       while(next) {
[snip]
+                               next = next->next;

In most places, this is done like this:

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

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

Best regards,

Rudolf