# [PATCH] Add push --set-upstream 9 messages from 2010-01-15 to 2010-01-15. Participants: Ilari Liusvaara, Matthieu Moy, Johannes Schindelin, Jeff King, Junio C Hamano. Thread: https://gitlist.dev/t/22233 ## Ilari Liusvaara, 2010-01-15 16:36 Subject: [PATCH] Add push --set-upstream Message-ID: <1263573407-13642-1-git-send-email-ilari.liusvaara@elisanet.fi> URL: https://gitlist.dev/e/1263573407-13642-1-git-send-email-ilari.liusvaara%40elisanet.fi ``` 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 --- 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=] - [--repo=] [-f | --force] [-v | --verbose] + [--repo=] [-f | --force] [-v | --verbose] [-u | --set-upstream] [ ...] 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 ``` ## Matthieu Moy, 2010-01-15 17:03 Subject: Re: [PATCH] Add push --set-upstream Message-ID: URL: https://gitlist.dev/e/vpqy6jzuwpv.fsf%40bauges.imag.fr In-Reply-To: <1263573407-13642-1-git-send-email-ilari.liusvaara@elisanet.fi> ``` Ilari Liusvaara 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..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..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/ ``` ## Johannes Schindelin, 2010-01-15 17:12 Subject: Re: [PATCH] Add push --set-upstream Message-ID: URL: https://gitlist.dev/e/alpine.DEB.1.00.1001151811270.3106%40intel-tinevez-2-302 In-Reply-To: ``` Hi, On Fri, 15 Jan 2010, Matthieu Moy wrote: > Ilari Liusvaara 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. ``` ## Jeff King, 2010-01-15 17:17 Subject: Re: [PATCH] Add push --set-upstream Message-ID: <20100115171745.GB2115@coredump.intra.peff.net> URL: https://gitlist.dev/e/20100115171745.GB2115%40coredump.intra.peff.net In-Reply-To: <1263573407-13642-1-git-send-email-ilari.liusvaara@elisanet.fi> ``` 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 ``` ## Jeff King, 2010-01-15 17:42 Subject: Re: [PATCH] Add push --set-upstream Message-ID: <20100115174220.GA15821@coredump.intra.peff.net> URL: https://gitlist.dev/e/20100115174220.GA15821%40coredump.intra.peff.net In-Reply-To: <20100115171745.GB2115@coredump.intra.peff.net> ``` 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); } } ``` ## Junio C Hamano, 2010-01-15 18:19 Subject: Re: [PATCH] Add push --set-upstream Message-ID: <7vhbqnz0vl.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vhbqnz0vl.fsf%40alter.siamese.dyndns.org In-Reply-To: <1263573407-13642-1-git-send-email-ilari.liusvaara@elisanet.fi> ``` 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. ``` ## Junio C Hamano, 2010-01-15 21:01 Subject: Re: [PATCH] Add push --set-upstream Message-ID: <7vvdf3ulol.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vvdf3ulol.fsf%40alter.siamese.dyndns.org In-Reply-To: ``` Matthieu Moy writes: > Ilari Liusvaara 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..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..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. ``` ## Ilari Liusvaara, 2010-01-15 22:06 Subject: Re: [PATCH] Add push --set-upstream Message-ID: <20100115220658.GA14755@Knoppix> URL: https://gitlist.dev/e/20100115220658.GA14755%40Knoppix In-Reply-To: <20100115171745.GB2115@coredump.intra.peff.net> ``` 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? ``` ## Jeff King, 2010-01-15 22:08 Subject: Re: [PATCH] Add push --set-upstream Message-ID: <20100115220800.GA25262@coredump.intra.peff.net> URL: https://gitlist.dev/e/20100115220800.GA25262%40coredump.intra.peff.net In-Reply-To: <20100115220658.GA14755@Knoppix> ``` 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 ```