New output style for git remote show
cover New output style for git remote show
Junio, this is based on pu, as it builds on my and Jeff's git remote changes. Patches 1-7 are about new output style for "git remote show" for everything but the push refspecs. Patches 8-13 then culiminate in teaching it a new output style for push refspecs as well.
1-4 were previously a single patch which I broke up per your feedback (gmane 110806).
Jay Soffian (13):
remote: rename variable and eliminate redundant function call
remote: remove unused code in get_ref_states
remote: fix two inconsistencies in the output of "show <remote>"
remote: make get_remote_ref_states() always populate states.tracked
remote: name remote_refs consistently
string-list: new for_each_string_list() function
remote: new show output style
refactor duplicated get_local_heads() to remote.c
refactor duplicated ref_newer() to remote.c
remote.c: make match_refs() copy src ref before assigning to peer_ref remote.c: don't short-circuit match_refs() when error in
match_explicit_refs()
remote.c: refactor get_remote_ref_states()
remote: new show output style for push refspecs
builtin-remote.c | 469 ++++++++++++++++++++++++++++++++++++++++----------- builtin-send-pack.c | 78 +--------- http-push.c | 72 +-------- remote.c | 88 +++++++++- remote.h | 2 + string-list.c | 10 + string-list.h | 5 + t/t5505-remote.sh | 66 +++++--- 8 files changed, 516 insertions(+), 274 deletions(-)
PATCH 1/13 remote: rename variable and eliminate redundant function call
The variable name "remote" is used as both a "char *" and as a "struct remote *"; this is confusing, so rename the former to remote_name.
There is no need to call "sort_string_list(&branch_list)" as branch_list is populated via string_list_insert(), which maintains its order.
--- builtin-remote.c | 13 ++++++------- 1 files changed, 6 insertions(+), 7 deletions(-)
builtin-remote.c +6 -7
@@ -144,7 +144,7 @@ static int add(int argc, const char **argv) }struct branch_info { -char *remote; +char *remote_name;struct string_list merge; };
@@ -183,9 +183,9 @@ static int config_read_branches(const char *key, const char *value, void *cb) item->util =xcalloc (sizeof (struct branch_info ),1 ); info = item->util;if (type ==REMOTE ) { -if (info->remote) +if (info->remote_name)warning ("more than one branch.%s" , key); - info->remote =xstrdup (value); + info->remote_name =xstrdup (value); }else {char *space =strchr (value,' ' ); value =abbrev_branch (value);
@@ -207,7 +207,6 @@ static void read_branches(void)if (branch_list.nr)return ;git_config (config_read_branches,NULL ); -sort_string_list (&branch_list); }struct ref_states {
@@ -490,7 +489,7 @@ static int mv(int argc, const char **argv)for (i =0 ; i < branch_list.nr; i++) {struct string_list_item *item = branch_list.items + i;struct branch_info *info = item->util; -if (info->remote && !strcmp (info->remote, rename.old)) { +if (info->remote_name && !strcmp (info->remote_name, rename.old)) {strbuf_reset (&buf);strbuf_addf (&buf,"branch.%s.remote" , item->string);if (git_config_set (buf.buf, rename.new)) {
@@ -600,7 +599,7 @@ static int rm(int argc, const char **argv)for (i =0 ; i < branch_list.nr; i++) {struct string_list_item *item = branch_list.items + i;struct branch_info *info = item->util; -if (info->remote && !strcmp (info->remote, remote->name)) { +if (info->remote_name && !strcmp (info->remote_name, remote->name)) {const char *keys[] = {"remote" ,"merge" ,NULL }, **k;for (k = keys; *k; k++) {strbuf_reset (&buf);
@@ -743,7 +742,7 @@ static int show(int argc, const char **argv)struct branch_info *info = branch->util;int j; -if (!info->merge.nr ||strcmp (*argv, info->remote)) +if (!info->merge.nr ||strcmp (*argv, info->remote_name))continue ; printf(" Remote branch%s merged with 'git pull' " "while on branch %s\n ",
Signature · 1 line
1.6.2.rc1.223.gfed32
Quoted 7 lines · Jay Soffian
Jay Soffian (13):
remote: rename variable and eliminate redundant function call
remote: remove unused code inget_ref_states
remote: fix two inconsistencies in the output of "show <remote>"
remote: makeget_remote_ref_states()always populate states.tracked
remote: nameremote_refsconsistently
string-list: newfor_each_string_list()function
Quoted 8 lines
remote: new show output style
refactor duplicatedget_local_heads()toremote.c
refactor duplicatedref_newer()toremote.c
remote.c: makematch_refs()copy src ref before assigning topeer_refremote.c: don't short-circuitmatch_refs()when error in
match_explicit_refs()
remote.c: refactorget_remote_ref_states()
remote: new show output style for push refspecs
I gave these a quick read (but not any extensive apply+build+experiment cycle), and they all look good to me. I don't know when I'll have time to try breaking them, though. ;)
Thank you for splitting them. It made reading a lot more pleasant.
-Peff
Quoted 5 lines · Jeff King
I gave these a quick read (but not any extensive apply+build+experiment cycle), and they all look good to me. I don't know when I'll have time to try breaking them, though. ;)
Thank you for splitting them. It made reading a lot more pleasant.
Thank you for the review. I humbly await your breakage attempts.
j.
PATCH 2/13 remote: remove unused code in get_ref_states
In reply to Jay Soffianget_ref_states() populates the util pointer of the string_list_item's that it adds to states->new and states->tracked, but nothing ever uses the pointer, so we can get rid of the extra code.
--- builtin-remote.c | 13 +++---------- 1 files changed, 3 insertions(+), 10 deletions(-)
builtin-remote.c +3 -10
@@ -250,18 +250,11 @@ static int get_ref_states(const struct ref *ref, struct ref_states *states) states->new.strdup_strings = states->tracked.strdup_strings =1 ;for (ref = fetch_map; ref; ref = ref->next) { -struct string_list *target = &states->tracked;unsigned char sha1[20 ]; -void *util =NULL ; -if (!ref->peer_ref ||read_ref (ref->peer_ref->name, sha1)) - target = &states->new; -else { - target = &states->tracked; -if (hashcmp (sha1, ref->new_sha1)) - util = &states; - } -string_list_append (abbrev_branch (ref->name), target)->util = util; +string_list_append (abbrev_branch (ref->name), &states->new); +else +string_list_append (abbrev_branch (ref->name), &states->tracked); }free_refs (fetch_map);
Signature · 1 line
1.6.2.rc1.223.gfed32
PATCH 3/13 remote: fix two inconsistencies in the output of "show <remote>"
In reply to Jay SoffianRemote and stale branches are emitted in alphabetical order, but new and tracked branches are not. So sort the latter to be consistent with the former. This also lets us use more efficient string_list_has_string() instead of unsorted_string_list_has_string().
"show <remote>" prunes symrefs, but "show <remote> -n" does not. Fix the latter to match the former.
--- builtin-remote.c | 15 ++++++++++----- t/t5505-remote.sh | 2 +- 2 files changed, 11 insertions(+), 6 deletions(-)
builtin-remote.c +10 -5
@@ -227,10 +227,8 @@ static int handle_one_branch(const char *refname,const char *name =abbrev_branch (refspec.src);/* symbolic refs pointing nowhere were handled already */ if ((flags &REF_ISSYMREF ) || -unsorted_string_list_has_string (&states->tracked, - name) || -unsorted_string_list_has_string (&states->new, - name)) +string_list_has_string (&states->tracked, name) || +string_list_has_string (&states->new, name))return 0 ; item =string_list_append (name, &states->stale); item->util =xstrdup (refname);
@@ -258,6 +256,8 @@ static int get_ref_states(const struct ref *ref, struct ref_states *states) }free_refs (fetch_map); +sort_string_list (&states->new); +sort_string_list (&states->tracked);for_each_ref (handle_one_branch, states);sort_string_list (&states->stale);
@@ -688,6 +688,9 @@ static int append_ref_to_tracked_list(const char *refname,struct ref_states *states = cb_data;struct refspec refspec; +if (flags &REF_ISSYMREF ) +return 0 ; +memset (&refspec,0 ,sizeof (refspec)); refspec.dst = (char *)refname;if (!remote_find_tracking (states->remote, &refspec))
@@ -754,8 +757,10 @@ static int show(int argc, const char **argv) "prune')", &states.stale, ""); } -if (no_query) +if (no_query) {for_each_ref (append_ref_to_tracked_list, &states); +sort_string_list (&states.tracked); + }show_list (" Tracked remote branch%s" , &states.tracked,"" );if (states.remote->push_refspec_nr) {
t/t5505-remote.sh +1 -1
@@ -142,8 +142,8 @@ cat > test/expect << EOF New remote branch (next fetch will store in remotes/origin) master Tracked remote branches - side master + side Local branches pushed with 'git push' master:upstream +refs/tags/lastbackup
Signature · 1 line
1.6.2.rc1.223.gfed32
PATCH 4/13 remote: make get_remote_ref_states() always populate states.tracked
In reply to Jay SoffianWhen not querying the remote, show() was having to populate states.tracked itself. It makes more sense for get_remote_ref_states() to do this consistently. Since show() is the only caller of get_remote_ref_states() with query=0, this change does not affect the other call sites.
--- builtin-remote.c | 41 ++++++++++++++++++++--------------------- 1 files changed, 20 insertions(+), 21 deletions(-)
builtin-remote.c +20 -21
@@ -656,6 +656,23 @@ static void free_remote_ref_states(struct ref_states *states)string_list_clear (&states->heads,0 ); } +static int append_ref_to_tracked_list (const char *refname, +const unsigned char *sha1,int flags,void *cb_data) +{ +struct ref_states *states = cb_data; +struct refspec refspec; + +if (flags &REF_ISSYMREF ) +return 0 ; + +memset (&refspec,0 ,sizeof (refspec)); + refspec.dst = (char *)refname; +if (!remote_find_tracking (states->remote, &refspec)) +string_list_append (abbrev_branch (refspec.src), &states->tracked); + +return 0 ; +} +static int get_remote_ref_states (const char *name,struct ref_states *states,int query)
@@ -677,28 +694,14 @@ static int get_remote_ref_states(const char *name,get_head_names (ref, name, states);get_ref_states (ref, states); + }else { +for_each_ref (append_ref_to_tracked_list, states); +sort_string_list (&states->tracked); }return 0 ; } -static int append_ref_to_tracked_list (const char *refname, -const unsigned char *sha1,int flags,void *cb_data) -{ -struct ref_states *states = cb_data; -struct refspec refspec; - -if (flags &REF_ISSYMREF ) -return 0 ; - -memset (&refspec,0 ,sizeof (refspec)); - refspec.dst = (char *)refname; -if (!remote_find_tracking (states->remote, &refspec)) -string_list_append (abbrev_branch (refspec.src), &states->tracked); - -return 0 ; -} -static int show (int argc,const char **argv) {int no_query =0 , result =0 ;
@@ -757,10 +760,6 @@ static int show(int argc, const char **argv) "prune')", &states.stale, ""); } -if (no_query) { -for_each_ref (append_ref_to_tracked_list, &states); -sort_string_list (&states.tracked); - }show_list (" Tracked remote branch%s" , &states.tracked,"" );if (states.remote->push_refspec_nr) {
Signature · 1 line
1.6.2.rc1.223.gfed32
Re: [PATCH 04/13] remote: make get_remote_ref_states() always populate states.tracked
Does this series _semantically_ depend on topics other than your other "set HEAD" topic that is still not in 'master'?
I see you made it cleanly apply on top of 'pu', but frankly, I really hate to see a series based on 'pu' as a matter of principle and discipline.
Other topics in 'pu' may not be ready for a long time, and you do not want this series to be taken hostage to them. Having to fork your topic off of many other topics not even in 'next' means it would be more cumbersome for me to handle your series when any of the other topics that you base your work on need to be discarded or replaced.
In other words, you make your series unworkable with if you introduce too many dependencies.
I've rebased it on top of Peff's jk/head-lookup topic (which in turn depends on your js/remote-set-head topic), but I suspect that you would want this series fork from and only depend on js/remote-set-head topic. In any case, please check the result after I push the results out to make sure that I did not make any silly mistakes while rebasing the series.
I also suspect that you would want to reroll the entire series anyway; see separate comments on 10/13 and 11/13.
Thanks.
Re: [PATCH 04/13] remote: make get_remote_ref_states() always populate states.tracked
Quoted 2 lines · Junio C Hamano
Does this series _semantically_ depend on topics other than your other "set HEAD" topic that is still not in 'master'?
No, but I wanted Jeff's changes. I have checked that I can apply the "set HEAD" series, then Jeff's changes, then this latest series all to next w/o conflict.
I've not tried it w/o Jeff's changes in the middle.
Quoted 8 lines
I see you made it cleanly apply on top of 'pu', but frankly, I really hate to see a series based on 'pu' as a matter of principle and discipline.
Other topics in 'pu' may not be ready for a long time, and you do not want this series to be taken hostage to them. Having to fork your topic off of many other topics not even in 'next' means it would be more cumbersome for me to handle your series when any of the other topics that you base your work on need to be discarded or replaced.
Understood. I thought this situation was a little different though due to Jeff's changes in the middle which really build on my set HEAD topic.
Quoted 8 lines
In other words, you make your series unworkable with if you introduce too many dependencies.
I've rebased it on top of Peff's jk/head-lookup topic (which in turn depends on your js/remote-set-head topic), but I suspect that you would want this series fork from and only depend on js/remote-set-head topic. In any case, please check the result after I push the results out to make sure that I did not make any silly mistakes while rebasing the series.
Will do, thanks.
Quoted 2 lines
I also suspect that you would want to reroll the entire series anyway; see separate comments on 10/13 and 11/13.
I've replied to those. Would you mind applying 10/13 and 11/13 independently of this series? This topic depends on them since otherwise there will be double-frees, but they can go anywhere before the topic.
j.
Re: [PATCH 04/13] remote: make get_remote_ref_states() always populate states.tracked
Quoted 3 lines · Jay Soffian
No, but I wanted Jeff's changes. I have checked that I can apply the "set HEAD" series, then Jeff's changes, then this latest series all to next w/o conflict.
Put another way, IMO, the set HEAD series, Jeff's changes, and this series might very well be a single topic.
Quoted 2 lines
Quoted 2 lines
In other words, you make your series unworkable with if you introduce too many dependencies.
For future reference, should I build on top of master or next?
j.
PATCH 5/13 remote: name remote_refs consistently
In reply to Jay SoffianClarify the code a bit by consistently referring to the remote refs returned by transport_get_remote_refs() as remote_refs.
--- builtin-remote.c | 22 +++++++++++----------- 1 files changed, 11 insertions(+), 11 deletions(-)
builtin-remote.c +11 -11
@@ -236,13 +236,14 @@ static int handle_one_branch(const char *refname,return 0 ; } -static int get_ref_states (const struct ref *ref,struct ref_states *states) +static int get_ref_states (const struct ref *remote_refs,struct ref_states *states) {struct ref *fetch_map =NULL , **tail = &fetch_map; +struct ref *ref;int i;for (i =0 ; i < states->remote->fetch_refspec_nr; i++) -if (get_fetch_map (ref, states->remote->fetch + i, &tail,1 )) +if (get_fetch_map (remote_refs, states->remote->fetch + i, &tail,1 ))die ("Could not get fetch map for refspec %s" , states->remote->fetch_refspec[i]);
@@ -264,7 +265,7 @@ static int get_ref_states(const struct ref *ref, struct ref_states *states)return 0 ; } -static int get_head_names (const struct ref *refs, +static int get_head_names (const struct ref *remote_refs,const char *remote_name,struct ref_states *states) {struct ref *ref, *matches;
@@ -275,9 +276,9 @@ static int get_head_names(const struct ref *refs, refspec.pattern =1 ; refspec.src = refspec.dst ="refs/heads/" ; states->heads.strdup_strings =1 ; -get_fetch_map (refs, &refspec, &fetch_map_tail,0 ); - matches =guess_remote_head (find_ref_by_name (refs,"HEAD" ), - fetch_map,1 ); +get_fetch_map (remote_refs, &refspec, &fetch_map_tail,0 ); + matches =guess_remote_head (find_ref_by_name (remote_refs,"HEAD" ), + fetch_map,1 );for (ref = matches; ref; ref = ref->next)string_list_append (abbrev_branch (ref->name), &states->heads);
@@ -678,7 +679,7 @@ static int get_remote_ref_states(const char *name, int query) {struct transport *transport; -const struct ref *ref; +const struct ref *remote_refs; states->remote =remote_get (name);if (!states->remote)
@@ -689,11 +690,10 @@ static int get_remote_ref_states(const char *name,if (query) { transport =transport_get (NULL , states->remote->url_nr >0 ? states->remote->url[0 ] :NULL ); - ref =transport_get_remote_refs (transport); + remote_refs =transport_get_remote_refs (transport);transport_disconnect (transport); - -get_head_names (ref, name, states); -get_ref_states (ref, states); +get_head_names (remote_refs, name, states); +get_ref_states (remote_refs, states); }else {for_each_ref (append_ref_to_tracked_list, states);sort_string_list (&states->tracked);
Signature · 1 line
1.6.2.rc1.223.gfed32
PATCH 6/13 string-list: new for_each_string_list() function
In reply to Jay SoffianAdd a convenience function for iterating over a string_list's items via a callback.
--- string-list.c | 10 ++++++++++ string-list.h | 5 +++++ 2 files changed, 15 insertions(+), 0 deletions(-)
string-list.c +10 -0
@@ -92,6 +92,16 @@ struct string_list_item *string_list_lookup(const char *string, struct string_lireturn list->items + i; } +int for_each_string_list (string_list_each_func_t fn, +struct string_list *list,void *cb_data) +{ +int i, ret =0 ; +for (i =0 ; i < list->nr; i++) +if ((ret =fn (&list->items[i], cb_data))) +break ; +return ret; +} +void string_list_clear (struct string_list *list,int free_util) {if (list->items) {
string-list.h +5 -0
@@ -20,6 +20,11 @@ void string_list_clear(struct string_list *list, int free_util);typedef void (*string_list_clear_func_t )(void *p,const char *str);void string_list_clear_func (struct string_list *list,string_list_clear_func_t clearfunc); +/* Use this function to iterate over each item */ +typedef int (*string_list_each_func_t )(struct string_list_item *,void *); +int for_each_string_list (string_list_each_func_t , +struct string_list *list,void *cb_data); +/* Use these functions only on sorted lists: */ int string_list_has_string (const struct string_list *list,const char *string); int string_list_find_insert_index(const struct string_list *list, const char *string,
Signature · 1 line
1.6.2.rc1.223.gfed32
PATCH 7/13 remote: new show output style
In reply to Jay SoffianThe existing output of "git remote show <remote>" is too verbose for the information it provides. This patch teaches it to provide more information in less space.
The output for push refspecs is addressed in followup patch.
Before the patch:
$ git remote show origin
* remote origin
URL: git://git.kernel.org/pub/scm/git/git.git
HEAD branch: master
Remote branch merged with 'git pull' while on branch master
master
Remote branch merged with 'git pull' while on branch next
next
Remote branches merged with 'git pull' while on branch octopus
foo bar baz frotz
New remote branch (next fetch will store in remotes/origin)
html
Stale tracking branch (use 'git remote prune')
bogus
Tracked remote branches
maint
man
master
next
pu
todoAfter this patch:
$ git remote show origin
* remote origin
URL: git://git.kernel.org/pub/scm/git/git.git
HEAD branch: master
Remote branches:
bogus stale (use 'git remote prune' to remove)
html new (next fetch will store in remotes/origin)
maint tracked
man tracked
master tracked
next tracked
pu tracked
todo tracked
Local branches configured for 'git pull':
master rebases onto remote master
next rebases onto remote next
octopus merges with remote foo
and with remote bar
and with remote baz
and with remote frotz$ git remote show origin -n
* remote origin
URL: git://git.kernel.org/pub/scm/git/git.git
HEAD branch: (not queried)
Remote branches: (status not queried)
bogus
maint
man
master
next
pu
todo
Local branches configured for 'git pull':
master rebases onto remote master
next rebases onto remote next
octopus merges with remote foo
and with remote bar
and with remote baz
and with remote frotz---
I incorporated the feedback I received to gmane 110626. As the commit message mentions, the push refspec output is addressed in a later patch in this series.
builtin-remote.c | 187 ++++++++++++++++++++++++++++++++++++++++------------- t/t5505-remote.sh | 31 ++++++---- 2 files changed, 161 insertions(+), 57 deletions(-)
builtin-remote.c +142 -45
@@ -146,6 +146,7 @@ static int add(int argc, const char **argv)struct branch_info {char *remote_name;struct string_list merge; +int rebase; };static struct string_list branch_list;
@@ -162,10 +163,11 @@ static const char *abbrev_ref(const char *name, const char *prefix)static int config_read_branches (const char *key,const char *value,void *cb) {if (!prefixcmp (key,"branch." )) { +const char *orig_key = key;char *name;struct string_list_item *item;struct branch_info *info; -enum {REMOTE ,MERGE } type; +enum {REMOTE ,MERGE ,REBASE } type; key +=7 ;if (!postfixcmp (key,".remote" )) {
@@ -174,6 +176,9 @@ static int config_read_branches(const char *key, const char *value, void *cb) }else if (!postfixcmp (key,".merge" )) { name =xstrndup (key,strlen (key) -6 ); type =MERGE ; + }else if (!postfixcmp (key,".rebase" )) { + name =xstrndup (key,strlen (key) -7 ); + type =REBASE ; }else return 0 ;
@@ -184,9 +189,9 @@ static int config_read_branches(const char *key, const char *value, void *cb) info = item->util;if (type ==REMOTE ) {if (info->remote_name) -warning ("more than one branch.%s" , key); +warning ("more than one %s" , orig_key); info->remote_name =xstrdup (value); - }else { + }else if (type ==MERGE ) {char *space =strchr (value,' ' ); value =abbrev_branch (value);while (space) {
@@ -197,7 +202,8 @@ static int config_read_branches(const char *key, const char *value, void *cb) space =strchr (value,' ' ); }string_list_append (xstrdup (value), &info->merge); - } + }else + info->rebase =git_config_bool (orig_key, value); }return 0 ; }
@@ -212,6 +218,7 @@ static void read_branches(void)struct ref_states {struct remote *remote;struct string_list new, stale, tracked, heads; +int queried; }; static int handle_one_branch(const char *refname,
@@ -635,20 +642,6 @@ static int rm(int argc, const char **argv)return result; } -static void show_list (const char *title,struct string_list *list, -const char *extra_arg) -{ -int i; - -if (!list->nr) -return ; - -printf (title, list->nr >1 ?"es" :"" , extra_arg); -printf ("\n" ); -for (i =0 ; i < list->nr; i++) -printf (" %s\n" , list->items[i].string); -} -static void free_remote_ref_states (struct ref_states *states) {string_list_clear (&states->new,0 );
@@ -698,6 +691,105 @@ static int get_remote_ref_states(const char *name,for_each_ref (append_ref_to_tracked_list, states);sort_string_list (&states->tracked); } + states->queried = query; + +return 0 ; +} + +struct show_info { +struct string_list *list; +struct ref_states *states; +int width; +int any_rebase; +}; + +int add_remote_to_show_info (struct string_list_item *item,void *cb_data) +{ +struct show_info *info = cb_data; +int n =strlen (item->string); +if (n > info->width) + info->width = n; +string_list_insert (item->string, info->list); +return 0 ; +} + +int show_remote_info_item (struct string_list_item *item,void *cb_data) +{ +struct show_info *info = cb_data; +struct ref_states *states = info->states; +const char *name = item->string; + +if (states->queried) { +const char *fmt ="%s" ; +const char *arg ="" ; +if (string_list_has_string (&states->new, name)) { + fmt =" new (next fetch will store in remotes/%s)" ; + arg = states->remote->name; + }else if (string_list_has_string (&states->tracked, name)) + arg =" tracked" ; +else if (string_list_has_string (&states->stale, name)) + arg =" stale (use 'git remote prune' to remove)" ; +else + arg =" ???" ; +printf (" %-*s" , info->width, name); +printf (fmt, arg); +printf ("\n" ); + }else +printf (" %s\n" , name); + +return 0 ; +} + +int add_local_to_show_info (struct string_list_item *branch_item,void *cb_data) +{ +struct show_info *show_info = cb_data; +struct ref_states *states = show_info->states; +struct branch_info *branch_info = branch_item->util; +struct string_list_item *item; +int n; + +if (!branch_info->merge.nr || !branch_info->remote_name || +strcmp (states->remote->name, branch_info->remote_name)) +return 0 ; +if ((n =strlen (branch_item->string)) > show_info->width) + show_info->width = n; +if (branch_info->rebase) + show_info->any_rebase =1 ; + + item =string_list_insert (branch_item->string, show_info->list); + item->util = branch_info; + +return 0 ; +} + +int show_local_info_item (struct string_list_item *item,void *cb_data) +{ +struct show_info *show_info = cb_data; +struct branch_info *branch_info = item->util; +struct string_list *merge = &branch_info->merge; +const char *also; +int i; + +if (branch_info->rebase && branch_info->merge.nr >1 ) { +error ("invalid branch.%s.merge; cannot rebase onto > 1 branch" , + item->string); +return 0 ; + } + +printf (" %-*s " , show_info->width, item->string); +if (branch_info->rebase) { +printf ("rebases onto remote %s\n" , merge->items[0 ].string); +return 0 ; + }else if (show_info->any_rebase) { +printf (" merges with remote %s\n" , merge->items[0 ].string); + also =" and with remote" ; + }else { +printf ("merges with remote %s\n" , merge->items[0 ].string); + also =" and with remote" ; + } +for (i =1 ; i < merge->nr; i++) +printf (" %-*s %s %s\n" , show_info->width,"" , also, + merge->items[i].string);return 0 ; }
@@ -711,6 +803,8 @@ static int show(int argc, const char **argv) OPT_END() };struct ref_states states; +struct string_list info_list = {NULL ,0 ,0 ,0 }; +struct show_info info; argc =parse_options (argc, argv, options, builtin_remote_usage,0 );
@@ -718,6 +812,9 @@ static int show(int argc, const char **argv)return show_all ();memset (&states,0 ,sizeof (states)); +memset (&info,0 ,sizeof (info)); + info.states = &states; + info.list = &info_list;for (; argc; argc--, argv++) {int i;
@@ -731,37 +828,37 @@ static int show(int argc, const char **argv)else if (!states.heads.nr)printf (" HEAD branch: (unknown)\n" );else if (states.heads.nr ==1 ) -printf (" HEAD branch: %s\n" , - states.heads.items[0 ].string); -else -show_list (" HEAD branch%s:" , &states.heads,"" ); - -for (i =0 ; i < branch_list.nr; i++) { -struct string_list_item *branch = branch_list.items + i; -struct branch_info *info = branch->util; -int j; - -if (!info->merge.nr ||strcmp (*argv, info->remote_name)) -continue ; -printf (" Remote branch%s merged with 'git pull' " -"while on branch %s\n " , - info->merge.nr >1 ?"es" :"" , - branch->string); -for (j =0 ; j < info->merge.nr; j++) -printf (" %s" , info->merge.items[j].string); -printf ("\n" ); +printf (" HEAD branch: %s\n" , states.heads.items[0 ].string); +else { +printf (" HEAD branch (remote HEAD is ambiguous," +" may be one of the following):\n" ); +for (i =0 ; i < states.heads.nr; i++) +printf (" %s\n" , states.heads.items[i].string); } -if (!no_query) { -show_list (" New remote branch%s (next fetch " -"will store in remotes/%s)" , - &states.new, states.remote->name); -show_list (" Stale tracking branch%s (use 'git remote " -"prune')" , &states.stale,"" ); - } +/* remote branch info */ + info.width =0 ; +for_each_string_list (add_remote_to_show_info, &states.new, &info); +for_each_string_list (add_remote_to_show_info, &states.tracked, &info); +for_each_string_list (add_remote_to_show_info, &states.stale, &info); +if (info.list->nr) +printf (" Remote branch%s:%s\n" , + info.list->nr >1 ?"es" :"" , + no_query ?" (status not queried)" :"" ); +for_each_string_list (show_remote_info_item, info.list, &info); +string_list_clear (info.list,0 ); -show_list (" Tracked remote branch%s" , &states.tracked,"" ); +/* git pull info */ + info.width =0 ; + info.any_rebase =0 ; +for_each_string_list (add_local_to_show_info, &branch_list, &info); +if (info.list->nr) +printf (" Local branch%s configured for 'git pull':\n" , + info.list->nr >1 ?"es" :"" ); +for_each_string_list (show_local_info_item, info.list, &info); +string_list_clear (info.list,0 ); +/* git push info */ if (states.remote->push_refspec_nr) { printf(" Local branch%s pushed with 'git push'\n", states.remote->push_refspec_nr > 1 ?
t/t5505-remote.sh +19 -12
@@ -28,7 +28,7 @@ tokens_match () { } check_remote_track () { - actual=$(git remote show "$1" | sed -e '1,/Tracked/d') && + actual=$(git remote show "$1" | sed -ne 's|^ \(.*\) tracked$|\1|p') shift && tokens_match "$*" "$actual" }
@@ -137,19 +137,21 @@ cat > test/expect << EOF * remote origin URL: $(pwd)/one HEAD branch: master - Remote branch merged with 'git pull' while on branch master - master - New remote branch (next fetch will store in remotes/origin) - master - Tracked remote branches - master - side + Remote branches: + master new (next fetch will store in remotes/origin) + side tracked + Local branches configured for 'git pull': + foo rebases onto remote master + master merges with remote master + oct merges with remote topic-a + and with remote topic-b + and with remote topic-c Local branches pushed with 'git push' master:upstream +refs/tags/lastbackup * remote two URL: ../two - HEAD branches: + HEAD branch (remote HEAD is ambiguous, may be one of the following): another master EOF
@@ -159,8 +161,12 @@ test_expect_success 'show' ' git config --add remote.origin.fetch \ refs/heads/master:refs/heads/upstream && git fetch && + git branch --track oct origin/master && + git branch --track foo origin/master && git branch -d -r origin/master && git config --add remote.two.url ../two && + git config branch.foo.rebase true && + git config branch.oct.merge "topic-a topic-b topic-c" && (cd ../one && echo 1 > file && test_tick &&
@@ -170,6 +176,7 @@ test_expect_success 'show' ' git config --add remote.origin.push \ +refs/tags/lastbackup && git remote show origin two > output && + git branch -d foo oct && test_cmp expect output) '
@@ -177,11 +184,11 @@ cat > test/expect << EOF * remote origin URL: $(pwd)/one HEAD branch: (not queried) - Remote branch merged with 'git pull' while on branch master - master - Tracked remote branches + Remote branches: (status not queried) master side + Local branch configured for 'git pull': + master merges with remote master Local branches pushed with 'git push' master:upstream +refs/tags/lastbackup
Signature · 1 line
1.6.2.rc1.223.gfed32
Re: [PATCH 07/13] remote: new show output style
Quoted 8 lines · Jay Soffian
The existing output of "git remote show <remote>" is too verbose for the information it provides. This patch teaches it to provide more information in less space.
Before the patch:
[...]
After this patch:
[...]
Wow. IMHO, the new one is _way_ easier to read.
-Peff
Re: [PATCH 07/13] remote: new show output style
Quoted 2 lines · Jeff King
Wow. IMHO, the new one is _way_ easier to read.
Agreed. Great work, Jay! Thanks!
M.
PATCH 8/13 refactor duplicated get_local_heads() to remote.c
In reply to Jay Soffianget_local_heads() appears to have been copied from builtin-send-pack.c to http-push.c via cut and paste. This patch moves the function and its helper one_local_ref() to remote.c.
The two copies of one_local_ref() were not identical. I used the more recent version from builtin-send-pack.c after confirming with Jeff King that it was an oversight that commit 30affa1e did not update both copies.
--- builtin-send-pack.c | 29 ++--------------------------- http-push.c | 23 ++--------------------- remote.c | 26 ++++++++++++++++++++++++++ remote.h | 1 + 4 files changed, 31 insertions(+), 48 deletions(-)
builtin-send-pack.c +2 -27
@@ -133,33 +133,8 @@ static int ref_newer(const unsigned char *new_sha1,return found; } -static struct ref *local_refs, **local_tail;static struct ref *remote_refs, **remote_tail; -static int one_local_ref (const char *refname,const unsigned char *sha1,int flag,void *cb_data) -{ -struct ref *ref; -int len; - -/* we already know it starts with refs/ to get here */ -if (check_ref_format (refname +5 )) -return 0 ; - - len =strlen (refname) +1 ; - ref =xcalloc (1 ,sizeof (*ref) + len); -hashcpy (ref->new_sha1, sha1); -memcpy (ref->name, refname, len); - *local_tail = ref; - local_tail = &ref->next; -return 0 ; -} - -static void get_local_heads (void ) -{ - local_tail = &local_refs; -for_each_ref (one_local_ref,NULL ); -} -static int receive_status (int in,struct ref *refs) {struct ref *hint;
@@ -387,7 +362,7 @@ static int refs_pushed(struct ref *ref)static int do_send_pack (int in,int out,struct remote *remote,const char *dest,int nr_refspec,const char **refspec) { -struct ref *ref; +struct ref *ref, *local_refs;int new_refs;int ask_for_status_report =0 ;int allow_deleting_refs =0 ;
@@ -405,7 +380,7 @@ static int do_send_pack(int in, int out, struct remote *remote, const char *dest/* No funny business with the matcher */ remote_tail =get_remote_heads (in, &remote_refs,0 ,NULL ,REF_NORMAL , &extra_have); -get_local_heads (); + local_refs =get_local_heads ();/* Does the other end support the reporting? */ if (server_supports ("report-status" ))
http-push.c +2 -21
@@ -1792,21 +1792,8 @@ static int update_remote(unsigned char *sha1, struct remote_lock *lock)return 1 ; } -static struct ref *local_refs, **local_tail;static struct ref *remote_refs, **remote_tail; -static int one_local_ref (const char *refname,const unsigned char *sha1,int flag,void *cb_data) -{ -struct ref *ref; -int len =strlen (refname) +1 ; - ref =xcalloc (1 ,sizeof (*ref) + len); -hashcpy (ref->new_sha1, sha1); -memcpy (ref->name, refname, len); - *local_tail = ref; - local_tail = &ref->next; -return 0 ; -} -static void one_remote_ref (char *refname) {struct ref *ref;
@@ -1839,12 +1826,6 @@ static void one_remote_ref(char *refname) remote_tail = &ref->next; } -static void get_local_heads (void ) -{ - local_tail = &local_refs; -for_each_ref (one_local_ref,NULL ); -} -static void get_dav_remote_heads (void ) { remote_tail = &remote_refs;
@@ -2195,7 +2176,7 @@ int main(int argc, char **argv)int rc =0 ;int i;int new_refs; -struct ref *ref; +struct ref *ref, *local_refs;char *rewritten_url =NULL ;git_extract_argv0_path (argv[0 ]);
@@ -2302,7 +2283,7 @@ int main(int argc, char **argv)fetch_indices ();/* Get a list of all local and remote heads to validate refspecs */ -get_local_heads (); + local_refs =get_local_heads ();fprintf (stderr,"Fetching remote heads...\n" );get_dav_remote_heads ();
remote.c +26 -0
@@ -1427,3 +1427,29 @@ struct ref *guess_remote_head(const struct ref *head,return list; } + +static int one_local_ref (const char *refname,const unsigned char *sha1,int flag,void *cb_data) +{ +struct ref ***local_tail = cb_data; +struct ref *ref; +int len; + +/* we already know it starts with refs/ to get here */ +if (check_ref_format (refname +5 )) +return 0 ; + + len =strlen (refname) +1 ; + ref =xcalloc (1 ,sizeof (*ref) + len); +hashcpy (ref->new_sha1, sha1); +memcpy (ref->name, refname, len); + **local_tail = ref; + *local_tail = &ref->next; +return 0 ; +} + +struct ref *get_local_heads (void ) +{ +struct ref *local_refs, **local_tail = &local_refs; +for_each_ref (one_local_ref, &local_tail); +return local_refs; +}
remote.h +1 -0
@@ -149,4 +149,5 @@ struct ref *guess_remote_head(const struct ref *head,const struct ref *refs, int all); +struct ref *get_local_heads (void );#endif
Signature · 1 line
1.6.2.rc1.223.gfed32
PATCH 9/13 refactor duplicated ref_newer() to remote.c
In reply to Jay Soffianref_newer() appears to have been copied from builtin-send-pack.c to http-push.c via cut and paste. This patch moves the function and its helper unmark_and_free() to remote.c.
---
Hmm, I just noticed that the version in builtin-send-pack.c uses 1 whereas http-push.c uses TMP_MARK as the mark argument. I used the version from builtin-send-pack.c. I don't know if this is signfificant.
builtin-send-pack.c | 49 ------------------------------------------------- http-push.c | 49 ------------------------------------------------- remote.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ remote.h | 1 + 4 files changed, 50 insertions(+), 98 deletions(-)
builtin-send-pack.c +0 -49
@@ -84,55 +84,6 @@ static int pack_objects(int fd, struct ref *refs, struct extra_have_objects *extreturn 0 ; } -static void unmark_and_free (struct commit_list *list,unsigned int mark) -{ -while (list) { -struct commit_list *temp = list; - temp->item->object.flags &= ~mark; - list = temp->next; -free (temp); - } -} - -static int ref_newer (const unsigned char *new_sha1, -const unsigned char *old_sha1) -{ -struct object *o; -struct commit *old, *new; -struct commit_list *list, *used; -int found =0 ; - -/* Both new and old must be commit-ish and new is descendant of -* old. Otherwise we require --force. -*/ - o =deref_tag (parse_object (old_sha1),NULL ,0 ); -if (!o || o->type !=OBJ_COMMIT ) -return 0 ; - old = (struct commit *) o; - - o =deref_tag (parse_object (new_sha1),NULL ,0 ); -if (!o || o->type !=OBJ_COMMIT ) -return 0 ; - new = (struct commit *) o; - -if (parse_commit (new) <0 ) -return 0 ; - - used = list =NULL ; -commit_list_insert (new, &list); -while (list) { - new =pop_most_recent_commit (&list,1 ); -commit_list_insert (new, &used); -if (new == old) { - found =1 ; -break ; - } - } -unmark_and_free (list,1 ); -unmark_and_free (used,1 ); -return found; -} -static struct ref *remote_refs, **remote_tail;static int receive_status (int in,struct ref *refs)
http-push.c +0 -49
55 lines not on this page · raw message
remote.c +49 -0
61 lines not on this page · raw message
remote.h +1 -0
@@ -76,6 +76,7 @@ int check_ref_type(const struct ref *ref, int flags);void free_refs (struct ref *ref);int resolve_remote_symref (struct ref *ref,struct ref *list); +int ref_newer (const unsigned char *new_sha1,const unsigned char *old_sha1);/* * Removes and frees any duplicate refs in the map.
Signature · 1 line
1.6.2.rc1.223.gfed32
Re: [PATCH 09/13] refactor duplicated ref_newer() to remote.c
Quoted 9 lines · Jay Soffian
ref_newer()appears to have been copied frombuiltin-send-pack.ctohttp-push.cvia cut and paste. This patch moves the function and its helperunmark_and_free()toremote.c.Signed-off-by Jay Soffian <[email protected]>---
Hmm, I just noticed that the version inbuiltin-send-pack.cuses 1 whereashttp-push.cusesTMP_MARKas the mark argument. I used the version frombuiltin-send-pack.c. I don't know if this is signfificant.
I think the TMP_MARK one is probably better. At the very least, the send-pack one should be using SEEN instead of a bare '1'. But given that it is not part of the usual revision machinery, it seems like that was what TMP_MARK was designed for.
-Peff
PATCH v2 9/13 refactor duplicated ref_newer() to remote.c
ref_newer() appears to have been copied from builtin-send-pack.c to http-push.c via cut and paste. This patch moves the function and its helper unmark_and_free() to remote.c. There was a slight difference between the two implementations, one used TMP_MARK for the mark, the other used 1. Per Jeff King, I went with TMP_MARK as more correct.
---
Quoted 4 lines · Jeff King
I think the
TMP_MARKone is probably better. At the very least, the send-pack one should be using SEEN instead of a bare '1'. But given that it is not part of the usual revision machinery, it seems like that was whatTMP_MARKwas designed for.
Okay then. This replaces the previous in the series using TMP_MARK instead, otherwise identical.
j.
builtin-send-pack.c | 49 ------------------------------------------------- http-push.c | 49 ------------------------------------------------- remote.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ remote.h | 1 + 4 files changed, 50 insertions(+), 98 deletions(-)
builtin-send-pack.c +0 -49
@@ -84,55 +84,6 @@ static int pack_objects(int fd, struct ref *refs, struct extra_have_objects *extreturn 0 ; } -static void unmark_and_free (struct commit_list *list,unsigned int mark) -{ -while (list) { -struct commit_list *temp = list; - temp->item->object.flags &= ~mark; - list = temp->next; -free (temp); - } -} - -static int ref_newer (const unsigned char *new_sha1, -const unsigned char *old_sha1) -{ -struct object *o; -struct commit *old, *new; -struct commit_list *list, *used; -int found =0 ; - -/* Both new and old must be commit-ish and new is descendant of -* old. Otherwise we require --force. -*/ - o =deref_tag (parse_object (old_sha1),NULL ,0 ); -if (!o || o->type !=OBJ_COMMIT ) -return 0 ; - old = (struct commit *) o; - - o =deref_tag (parse_object (new_sha1),NULL ,0 ); -if (!o || o->type !=OBJ_COMMIT ) -return 0 ; - new = (struct commit *) o; - -if (parse_commit (new) <0 ) -return 0 ; - - used = list =NULL ; -commit_list_insert (new, &list); -while (list) { - new =pop_most_recent_commit (&list,1 ); -commit_list_insert (new, &used); -if (new == old) { - found =1 ; -break ; - } - } -unmark_and_free (list,1 ); -unmark_and_free (used,1 ); -return found; -} -static struct ref *remote_refs, **remote_tail;static int receive_status (int in,struct ref *refs)
http-push.c +0 -49
55 lines not on this page · raw message
remote.c +49 -0
61 lines not on this page · raw message
remote.h +1 -0
@@ -76,6 +76,7 @@ int check_ref_type(const struct ref *ref, int flags);void free_refs (struct ref *ref);int resolve_remote_symref (struct ref *ref,struct ref *list); +int ref_newer (const unsigned char *new_sha1,const unsigned char *old_sha1);/* * Removes and frees any duplicate refs in the map.
Signature · 1 line
1.6.2.rc1.223.gfed32
Re: [PATCH v2 09/13] refactor duplicated ref_newer() to remote.c
remote.c line 8
Quoted 9 lines · Jay Soffian
remote.c +1
-0@@ -5,6 +5,7 @@#include "diff.h" #include "revision.h" #include "dir.h" +#include "tag.h"
You are only moving code from http-push.c and builtin-send-pack.c to remote.c, but you have to add this '#include "tag.h"' here. How high are the chances that you can remove the #include lines from http-push.c and builtin-send-pack.c?
-- Hannes
Re: [PATCH v2 09/13] refactor duplicated ref_newer() to remote.c
Quoted 15 lines · Johannes Sixt
remote.c line 8
Quoted 9 lines · Jay Soffian
remote.c +1
-0@@ -5,6 +5,7 @@#include "diff.h" #include "revision.h" #include "dir.h" +#include "tag.h" You are only moving code from
http-push.candbuiltin-send-pack.ctoremote.c, but you have to add this '#include "tag.h"' here. How high are the chances that you can remove the #include lines fromhttp-push.candbuiltin-send-pack.c?
50/50 as it turns out. Junio, please squash this in if you don't mind:
builtin-send-pack.c +0 -1
@@ -1,6 +1,5 @@#include "cache.h" #include "commit.h" -#include "tag.h" #include "refs.h" #include "pkt-line.h" #include "run-command.h"
j.
PATCH 10/13 remote.c: make match_refs() copy src ref before assigning to peer_ref
In reply to Jay SoffianIn some instances, match_refs() was setting the peer_ref field in the dst ref list such that it pointed to a ref in the src list, instead of to a copy. This would cause a double-free when both the src and dst ref lists were freed, as free_refs() frees the peer_ref. A double-free would also occur if the same branch was pushed to two different destinations such as:
push = refs/heads/master:refs/heads/backup
push = refs/heads/master:refs/heads/master
This patch corrects the problem by copying the ref when it has been plucked from the src list.
--- remote.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)
remote.c +5 -2
@@ -930,6 +930,7 @@ static int match_explicit(struct ref *src, struct ref *dst, struct refspec *rs) {struct ref *matched_src, *matched_dst; +int copy_src;const char *dst_value = rs->dst;char *dst_guess;
@@ -940,6 +941,7 @@ static int match_explicit(struct ref *src, struct ref *dst, matched_src = matched_dst =NULL ;switch (count_refspec_match (rs->src, src, &matched_src)) {case 1 : + copy_src =1 ;break ;case 0 :/* The source could be in the get_sha1() format
@@ -949,6 +951,7 @@ static int match_explicit(struct ref *src, struct ref *dst, matched_src =try_explicit_object_name (rs->src);if (!matched_src)return error ("src refspec %s does not match any." , rs->src); + copy_src =0 ;break ;default :return error ("src refspec %s matches more than one." , rs->src);
@@ -994,7 +997,7 @@ static int match_explicit(struct ref *src, struct ref *dst,return error ("dst ref %s receives from more than one src." , matched_dst->name); else { - matched_dst->peer_ref = matched_src; + matched_dst->peer_ref = copy_src ?copy_ref (matched_src) : matched_src; matched_dst->force = rs->force; }return 0 ;
@@ -1102,7 +1105,7 @@ int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail, dst_peer =make_linked_ref (dst_name, dst_tail);hashcpy (dst_peer->new_sha1, src->new_sha1); } - dst_peer->peer_ref = src; + dst_peer->peer_ref =copy_ref (src); dst_peer->force = pat->force; free_name:free (dst_name);
Signature · 1 line
1.6.2.rc1.223.gfed32
Re: [PATCH 10/13] remote.c: make match_refs() copy src ref before assigning to peer_ref
If this issue was present before you started touching the affected code, it should come much earlier in the series than [10/13]. If on the other hand this was introduced by earlier one in the series, the patch that introduced the bug should be corrected in place in the sequence without this patch.
Re: [PATCH 10/13] remote.c: make match_refs() copy src ref before assigning to peer_ref
Quoted 5 lines · Junio C Hamano
If this issue was present before you started touching the affected code, it should come much earlier in the series than [10/13]. If on the other hand this was introduced by earlier one in the series, the patch that introduced the bug should be corrected in place in the sequence without this patch.
This issue existed before, but the other callers of match_heads() only call it once before the process terminates, and don't bother to free what it returns. So those callers didn't notice. This topic introduces the first caller that calls match_heads() repeatedly, so it frees the returned ref list after it has copied out what it needs.
The change is independent and can be moved anywhere in the series, or even introduced as a patch independent of the series itself. Which would you prefer? (Actually, would you mind just applying it how you see fit?)
j.
Re: [PATCH 10/13] remote.c: make match_refs() copy src ref before assigning to peer_ref
Quoted 8 lines · Jay Soffian
This issue existed before, but the other callers of
match_heads()only call it once before the process terminates, and don't bother to free what it returns. So those callers didn't notice. This topic introduces the first caller that callsmatch_heads()repeatedly, so it frees the returned ref list after it has copied out what it needs.The change is independent and can be moved anywhere in the series, or even introduced as a patch independent of the series itself.
Thanks for explanation.
If I am reading the above correctly, a sequence that does not have 10/13 before the patch that makes match_heads() called repeatedly would break bisection, so it cannot be "can be moved anywhere".
And it sounds like keeping this as a separate patch would make more sense. Your proposed commit log message for the patch would contain the first paragraph I quoted above, I'd think.
PATCH 1/1 remote.c: make match_refs() copy src ref before assigning to peer_ref
In some instances, match_refs() sets the peer_ref field of refs in the dst list such that it points to a ref in the src list. Were a caller to free both the src and dst lists, this would cause a double-free as free_refs() frees the peer_ref.
A double-free would also occur freeing just the dst list under the following configuration:
push = refs/heads/master:refs/heads/backup
push = refs/heads/master:refs/heads/master
Since such a configuration would cause two refs in the dst list to have the same peer_ref.
This patch corrects the problem by copying the ref when it has been plucked from the src list.
This issue has gone undetected as the existing callers of match_heads() call it only once and then terminate, w/o ever bothering to free the src or dst lists.
--- remote.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)
remote.c +5 -2
@@ -927,6 +927,7 @@ static int match_explicit(struct ref *src, struct ref *dst, struct refspec *rs) {struct ref *matched_src, *matched_dst; +int copy_src;const char *dst_value = rs->dst;char *dst_guess;
@@ -937,6 +938,7 @@ static int match_explicit(struct ref *src, struct ref *dst, matched_src = matched_dst =NULL ;switch (count_refspec_match (rs->src, src, &matched_src)) {case 1 : + copy_src =1 ;break ;case 0 :/* The source could be in the get_sha1() format
@@ -946,6 +948,7 @@ static int match_explicit(struct ref *src, struct ref *dst, matched_src =try_explicit_object_name (rs->src);if (!matched_src)return error ("src refspec %s does not match any." , rs->src); + copy_src =0 ;break ;default :return error ("src refspec %s matches more than one." , rs->src);
@@ -991,7 +994,7 @@ static int match_explicit(struct ref *src, struct ref *dst,return error ("dst ref %s receives from more than one src." , matched_dst->name); else { - matched_dst->peer_ref = matched_src; + matched_dst->peer_ref = copy_src ?copy_ref (matched_src) : matched_src; matched_dst->force = rs->force; }return 0 ;
@@ -1099,7 +1102,7 @@ int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail, dst_peer =make_linked_ref (dst_name, dst_tail);hashcpy (dst_peer->new_sha1, src->new_sha1); } - dst_peer->peer_ref = src; + dst_peer->peer_ref =copy_ref (src); dst_peer->force = pat->force; free_name:free (dst_name);
Signature · 1 line
1.6.2.rc1.291.g83eb
Re: [PATCH] remote.c: make match_refs() copy src ref before assigning to peer_ref
Quoted 3 lines · Jay Soffian
This issue has gone undetected as the existing callers of
match_heads()call it only once and then terminate, w/o ever bothering to free the src or dst lists.
Thanks.
It sounds more like existing function and usage was alright but your new caller made it into an issue.
So I'd queue this as the first patch in the 13-patch series, dropping 10/13? What should happen to 11/13?
Re: [PATCH] remote.c: make match_refs() copy src ref before assigning to peer_ref
Quoted 10 lines · Junio C Hamano
Quoted 3 lines · Jay Soffian
This issue has gone undetected as the existing callers of
match_heads()call it only once and then terminate, w/o ever bothering to free the src or dst lists.Thanks.
It sounds more like existing function and usage was alright but your new caller made it into an issue.
Well, maybe. The existing function alters the passed in dst argument (a ref list) in two ways:
1) It potentially adds new refs to the tail of the list.
2) For the existing and new refs, it populates their peer_ref field. When it does this, it sometimes just points it at an existing ref in the src list, but sometimes it allocates a new ref and points to that.
So there is no sane way to free the result. As there is no indication that the caller shouldn't be able to free the src and dst lists after it is done, it was IMO a latent bug.
Initially I tried to fix this by modifying free_refs() to make two passes over the list passed to it, once to mark each peer_ref, then a second time to clear only those peer_refs it had not already seen. I did this because when I first saw the double free I was only freeing the dst list. It took me a while to realize the cause was two refs in the dst list having the same peer_ref.
But then I realized my caller needed to clear the src list as well, and saw that the problem was really in match_heads(), not in free'ing the result.
Quoted 2 lines
So I'd queue this as the first patch in the 13-patch series, dropping 10/13? What should happen to 11/13?
I sent you a message off-list you may not have read yet. I'm re-rolling the entire js/remote-set-head for you off of the current master to get rid of any dependencies on pu, with jk/head-lookup on the end of the series instead of in the middle. I needed to rebase off master since I have a dependency on cfa1ee6 which is now in master, but wasn't when you spawned js/remote-set-head initially.
j.
Re: [PATCH] remote.c: make match_refs() copy src ref before assigning to peer_ref
Quoted 1 line · Jay Soffian
So there is no sane way to free the result.
Yeah, I am not disagreeing that this is a good change to make the function easier to use. There is no question about it.
I just found your description a bit unfair to the original author of the codepath who was probably fully aware of this.
Quoted 1 line
I sent you a message off-list you may not have read yet.
Yes I have, but I was deep into tonight's integration cycle (I earlier asked you to check the rebased result when I push it out --- that is what I've been preparing). I am almost done, and it will be pushed out shortly, but it won't have your re-roll. That will be after 1.6.2-rc2.
PATCH 11/13 remote.c: don't short-circuit match_refs() when error in match_explicit_refs()
In reply to Jay Soffianmatch_refs() returns non-zero if there is an error in match_explicit_refs(), without handling any remaining pattern ref specs.
Its existing callers exit upon receiving non-zero, so a partial result is of no consequence to them; however it is about to have a new caller that is interested in the complete result even if there are errors in match_explicit_refs().
--- remote.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)
remote.c +4 -2
@@ -1046,6 +1046,7 @@ int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail,struct refspec *rs;int send_all = flags &MATCH_REFS_ALL ;int send_mirror = flags &MATCH_REFS_MIRROR ; +int errs;static const char *default_refspec[] = {":" ,0 };if (!nr_refspec) {
@@ -1053,8 +1054,7 @@ int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail, refspec = default_refspec; } rs =parse_push_refspec (nr_refspec, (const char **) refspec); -if (match_explicit_refs (src, dst, dst_tail, rs, nr_refspec)) -return -1 ; + errs =match_explicit_refs (src, dst, dst_tail, rs, nr_refspec);/* pick the remainder */ for ( ; src; src = src->next) {
@@ -1110,6 +1110,8 @@ int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail, free_name:free (dst_name); } +if (errs) +return -1 ;return 0 ; }
Signature · 1 line
1.6.2.rc1.223.gfed32
Re: [PATCH 11/13] remote.c: don't short-circuit match_refs() when error in match_explicit_refs()
The same comment as 10/13 applies here.
Re: [PATCH 11/13] remote.c: don't short-circuit match_refs() when error in match_explicit_refs()
Quoted 1 line · Junio C Hamano
The same comment as 10/13 applies here.
As does the reply. :-)
j.
PATCH 12/13 remote.c: refactor get_remote_ref_states()
In reply to Jay Soffianget_remote_ref_states() has three callers, but each is interested in slightly different information. Give it a bit-field flag so that callers can specify which pieces of information they need.
--- builtin-remote.c | 23 +++++++++++++++-------- 1 files changed, 15 insertions(+), 8 deletions(-)
builtin-remote.c +15 -8
@@ -19,6 +19,9 @@ static const char * const builtin_remote_usage[] = { NULL }; +#define GET_REF_STATES (1<<0) +#define GET_HEAD_NAMES (1<<1) +static int verbose;static int show_all (void );
@@ -685,14 +688,15 @@ static int get_remote_ref_states(const char *name, states->remote->url[0] : NULL); remote_refs =transport_get_remote_refs (transport);transport_disconnect (transport); -get_head_names (remote_refs, name, states); -get_ref_states (remote_refs, states); + states->queried =1 ; +if (query &GET_REF_STATES ) +get_ref_states (remote_refs, states); +if (query &GET_HEAD_NAMES ) +get_head_names (remote_refs, name, states); }else {for_each_ref (append_ref_to_tracked_list, states);sort_string_list (&states->tracked); } - states->queried = query; -return 0 ; }
@@ -796,7 +800,7 @@ int show_local_info_item(struct string_list_item *item, void *cb_data)static int show (int argc,const char **argv) { -int no_query =0 , result =0 ; +int no_query =0 , result =0 , query_flag =0 ;struct option options[] = {OPT_GROUP ("show specific options" ),OPT_BOOLEAN ('n' ,NULL , &no_query,"do not query remotes" ),
@@ -811,6 +815,9 @@ static int show(int argc, const char **argv)if (argc <1 )return show_all (); +if (!no_query) + query_flag = (GET_REF_STATES |GET_HEAD_NAMES ); +memset (&states,0 ,sizeof (states));memset (&info,0 ,sizeof (info)); info.states = &states;
@@ -818,7 +825,7 @@ static int show(int argc, const char **argv)for (; argc; argc--, argv++) {int i; -get_remote_ref_states (*argv, &states, !no_query); +get_remote_ref_states (*argv, &states, query_flag); printf("* remote %s\n URL: %s\n", *argv, states.remote->url_nr > 0 ?
@@ -902,7 +909,7 @@ static int set_head(int argc, const char **argv) }else if (opt_a && !opt_d && argc ==1 ) {struct ref_states states;memset (&states,0 ,sizeof (states)); -get_remote_ref_states (argv[0 ], &states,1 ); +get_remote_ref_states (argv[0 ], &states,GET_HEAD_NAMES );if (!states.heads.nr) result |=error ("Cannot determine remote HEAD" );else if (states.heads.nr >1 ) {
@@ -962,7 +969,7 @@ static int prune(int argc, const char **argv)for (; argc; argc--, argv++) {int i; -get_remote_ref_states (*argv, &states,1 ); +get_remote_ref_states (*argv, &states,GET_REF_STATES );if (states.stale.nr) {printf ("Pruning %s\n" , *argv);
Signature · 1 line
1.6.2.rc1.223.gfed32
Re: [PATCH 12/13] remote.c: refactor get_remote_ref_states()
Quoted 3 lines · Jay Soffian
get_remote_ref_states()has three callers, but each is interested in slightly different information. Give it a bit-field flag so that callers can specify which pieces of information they need.
Hmph. I think this is probably an indication that get_remote_ref_states() should really be 3 functions:
common_stuff(); query_one(); query_two();
and then callers can choose the subset they are interested in.
Which is really more or less equivalent; it just seems like extra obfuscation to have a single function with a bit-field.
-Peff
Re: [PATCH 12/13] remote.c: refactor get_remote_ref_states()
Quoted 9 lines · Jeff King
Quoted 3 lines · Jay Soffian
get_remote_ref_states()has three callers, but each is interested in slightly different information. Give it a bit-field flag so that callers can specify which pieces of information they need.Hmph. I think this is probably an indication that
get_remote_ref_states()should really be 3 functions:
Quoted 8 lines
common_stuff();
query_one();
query_two();and then callers can choose the subset they are interested in.
Which is really more or less equivalent; it just seems like extra obfuscation to have a single function with a bit-field.
I see your hmph and raise you a hmph. :-)
Well, I _had_ tried as you suggested first, and thought it yuckier. It would actually be more like:
caller1() {
setup_for_get();
get_thing_one();
}
caller2() {
setup_for_get();
get_thing_two();
}
caller3() {
if (query) {
setup_for_get();
get_thing_one();
get_thing_two();
get_thing_three();
} else {
get_thing_one_noquery()
get_thing_two_noquery();
}As opposed to:
caller1() {
get_things(ONE)
}
caller2() {
get_things(TWO)
}
caller3() {
things = 0;
if (query)
things = (ONE|TWO|THREE)
get_things(things)
}
I'm not sure why passing a flag saying what you want is obfuscating. Also, I did find other places in git that do this sort of thing (e.g., get_remote_heads).
(Awaiting Junio's lucid reply explaining why my justification is bogus.) :-)
j.
Re: [PATCH 12/13] remote.c: refactor get_remote_ref_states()
---
So here's the inter-diff doing it the way you suggest. I'm surprised it didn't add any LOC. If you really think this is better I'll re-do the end of the series to do it this way.
builtin-remote.c | 81 ++++++++++++++++++++++++++--------------------------- 1 files changed, 40 insertions(+), 41 deletions(-)
builtin-remote.c +40 -41
@@ -19,10 +19,6 @@ static const char * const builtin_remote_usage[] = { NULL }; -#define GET_REF_STATES (1<<0) -#define GET_HEAD_NAMES (1<<1) -#define GET_PUSH_REF_STATES (1<<2) -static int verbose;static int show_all (void );
@@ -383,7 +379,7 @@ static int get_push_ref_states_noquery(struct ref_states *states) }static int get_head_names (const struct ref *remote_refs, -const char *remote_name,struct ref_states *states) +struct ref_states *states) {struct ref *ref, *matches;struct ref *fetch_map =NULL , **fetch_map_tail = &fetch_map;
@@ -785,37 +781,15 @@ static int append_ref_to_tracked_list(const char *refname,return 0 ; } -static int get_remote_ref_states (const char *name, -struct ref_states *states, -int query) +static const struct ref *get_remote_refs (struct ref_states *states) {struct transport *transport;const struct ref *remote_refs; - - states->remote =remote_get (name); -if (!states->remote) -return error ("No such remote: %s" , name); - -read_branches (); - -if (query) { - transport =transport_get (NULL , states->remote->url_nr >0 ? - states->remote->url[0 ] :NULL ); - remote_refs =transport_get_remote_refs (transport); -transport_disconnect (transport); - states->queried =1 ; -if (query &GET_REF_STATES ) -get_ref_states (remote_refs, states); -if (query &GET_HEAD_NAMES ) -get_head_names (remote_refs, name, states); -if (query &GET_PUSH_REF_STATES ) -get_push_ref_states (remote_refs, states); - }else { -for_each_ref (append_ref_to_tracked_list, states); -sort_string_list (&states->tracked); -get_push_ref_states_noquery (states); - } -return 0 ; + transport =transport_get (NULL , states->remote->url_nr >0 ? + states->remote->url[0 ] :NULL ); + remote_refs =transport_get_remote_refs (transport); +transport_disconnect (transport); +return remote_refs; }struct show_info {
@@ -970,7 +944,7 @@ int show_push_info_item(struct string_list_item *item, void *cb_data)static int show (int argc,const char **argv) { -int no_query =0 , result =0 , query_flag =0 ; +int no_query =0 , result =0 ;struct option options[] = {OPT_GROUP ("show specific options" ),OPT_BOOLEAN ('n' ,NULL , &no_query,"do not query remotes" ),
@@ -985,17 +959,31 @@ static int show(int argc, const char **argv)if (argc <1 )return show_all (); -if (!no_query) - query_flag = (GET_REF_STATES |GET_HEAD_NAMES |GET_PUSH_REF_STATES ); - +read_branches ();memset (&states,0 ,sizeof (states));memset (&info,0 ,sizeof (info)); + states.queried = !no_query; info.states = &states; info.list = &info_list; +for (; argc; argc--, argv++) {int i; -get_remote_ref_states (*argv, &states, query_flag); +if (!(states.remote =remote_get (*argv))) { +error ("No such remote: %s" , *argv); +continue ; + } +if (no_query) { +for_each_ref (append_ref_to_tracked_list, &states); +sort_string_list (&states.tracked); +get_push_ref_states_noquery (&states); + }else { +const struct ref *remote_refs; + remote_refs =get_remote_refs (&states); +get_ref_states (remote_refs, &states); +get_head_names (remote_refs, &states); +get_push_ref_states (remote_refs, &states); + } printf("* remote %s\n URL: %s\n", *argv, states.remote->url_nr > 0 ?
@@ -1077,8 +1065,13 @@ static int set_head(int argc, const char **argv) head_name =xstrdup (argv[1 ]); }else if (opt_a && !opt_d && argc ==1 ) {struct ref_states states; +const struct ref *remote_refs;memset (&states,0 ,sizeof (states)); -get_remote_ref_states (argv[0 ], &states,GET_HEAD_NAMES ); +read_branches (); +if (!(states.remote =remote_get (argv[0 ]))) +return error ("No such remote: %s" , argv[0 ]); + remote_refs =get_remote_refs (&states); +get_head_names (remote_refs, &states);if (!states.heads.nr) result |=error ("Cannot determine remote HEAD" );else if (states.heads.nr >1 ) {
@@ -1134,11 +1127,17 @@ static int prune(int argc, const char **argv) ?" %s will become dangling!\n" : " %s has become dangling!\n"); +read_branches ();memset (&states,0 ,sizeof (states));for (; argc; argc--, argv++) {int i; - -get_remote_ref_states (*argv, &states,GET_REF_STATES ); +const struct ref *remote_refs; +if (!(states.remote =remote_get (*argv))) { +error ("No such remote: %s" , *argv); +continue ; + } + remote_refs =get_remote_refs (&states); +get_ref_states (remote_refs, &states);if (states.stale.nr) {printf ("Pruning %s\n" , *argv);
Signature · 1 line
1.6.2.rc1.223.gfed32
Re: [PATCH 12/13] remote.c: refactor get_remote_ref_states()
Quoted 4 lines · Jay Soffian
I see your hmph and raise you a hmph. :-)
Well, I _had_ tried as you suggested first, and thought it yuckier. It would actually be more like:
OK. My comment was along the lines of "why didn't you think of doing it this way first?" but apparently you did. ;)
Quoted 1 line
I'm not sure why passing a flag saying what you want is obfuscating.
It doesn't have to be. But from your description and my cursory look over the code, it seemed like get_remote_ref_states was really conflating several unrelated things. So breaking it apart would make sense for the same reason we have "strlen" and "strchr" as separate functions and not "string_ops(s, WANT_STRLEN)": the function is our basic unit of reusable work.
But from your description:
Quoted 4 lines
caller1(){
setup_for_get();
get_thing_one();
}
I didn't realize the commonality was actual setup; I thought it was "do a list of 5 things, but the first 2 are common, and then callers may want to some mix of the other 3".
So I think your original patch is fine.
-Peff
PATCH 13/13 remote: new show output style for push refspecs
In reply to Jay SoffianThe existing output of "git remote show <remote>" with respect to push ref specs is basically just to show the raw refspec. This patch teaches the command to interpret the refspecs and show how each branch will be pushed to the destination. The output gives the user an idea of what "git push" should do if it is run w/o any arguments.
Example new output:
1a. Typical output with no push refspec (i.e. matching branches only)
$ git remote show origin
* remote origin
[...]
Local refs configured for 'git push':
master pushes to master (up to date)
next pushes to next (local out of date)1b. Same as above, w/o querying the remote:
$ git remote show origin -n
* remote origin
[...]
Local ref configured for 'git push' (status not queried):
(matching) pushes to (matching)2a. With a forcing refspec (+), and a new topic
(something like push = refs/heads/*:refs/heads/*):
$ git remote show origin
* remote origin
[...]
Local refs configured for 'git push':
master pushes to master (fast forwardable)
new-topic pushes to new-topic (create)
next pushes to next (local out of date)
pu forces to pu (up to date)2b. Same as above, w/o querying the remote
$ git remote show origin -n
* remote origin
[...]
Local refs configured for 'git push' (status not queried):
master pushes to master
new-topic pushes to new-topic
next pushes to next
pu forces to pu3. With a remote configured as a mirror:
* remote backup
[...]
Local refs will be mirrored by 'git push'
---
After spending quite a long time standing in the paint aisle looking at all the different colors, this is my favorite.
You might like a different color paint. For example, an obvious choice might seem to be single character status fields a-la ls-files. But that would sacrifice clarity (IMO) as the user then has to go lookup what the values mean in a man page, and in any case wouldn't be' consistent with the rest of the new "show" output.
Also, "forces to" may sound a little funny, but I think it is obvious what it means and lines up more nicely than "force pushes" or any other choice I considered.
Anyway, I built this new shed, so I get to pick the color. (And hopefully no one blows it up w/a stick of dynamite) :-)
builtin-remote.c | 201 ++++++++++++++++++++++++++++++++++++++++++++++++---- t/t5505-remote.sh | 57 +++++++++------ 2 files changed, 219 insertions(+), 39 deletions(-)
builtin-remote.c +185 -16
@@ -21,6 +21,7 @@ static const char * const builtin_remote_usage[] = {#define GET_REF_STATES (1<<0)#define GET_HEAD_NAMES (1<<1) +#define GET_PUSH_REF_STATES (1<<2)static int verbose;
@@ -220,7 +221,7 @@ static void read_branches(void)struct ref_states {struct remote *remote; -struct string_list new, stale, tracked, heads; +struct string_list new, stale, tracked, push, heads;int queried; };
@@ -275,6 +276,112 @@ static int get_ref_states(const struct ref *remote_refs, struct ref_states *statreturn 0 ; } +struct push_info { +char *dest; +int forced; +enum { +PUSH_STATUS_CREATE =0 , +PUSH_STATUS_DELETE , +PUSH_STATUS_UPTODATE , +PUSH_STATUS_FASTFORWARD , +PUSH_STATUS_OUTOFDATE , +PUSH_STATUS_NOTQUERIED , + } status; +}; + +static int get_push_ref_states (const struct ref *remote_refs, +struct ref_states *states) +{ +struct remote *remote = states->remote; +struct ref *ref, *local_refs, *push_map, **push_tail; +if (remote->mirror) +return 0 ; + + local_refs =get_local_heads (); + ref = push_map =copy_ref_list (remote_refs); +while (ref->next) + ref = ref->next; + push_tail = &ref->next; + +match_refs (local_refs, push_map, &push_tail, remote->push_refspec_nr, + remote->push_refspec,MATCH_REFS_NONE ); + + states->push.strdup_strings =1 ; +for (ref = push_map; ref; ref = ref->next) { +struct string_list_item *item; +struct push_info *info; + +if (!ref->peer_ref) +continue ; +hashcpy (ref->new_sha1, ref->peer_ref->new_sha1); + + item =string_list_append (abbrev_branch (ref->peer_ref->name), + &states->push); + item->util =xcalloc (sizeof (struct push_info ),1 ); + info = item->util; + info->forced = ref->force; + info->dest =xstrdup (abbrev_branch (ref->name)); + +if (is_null_sha1 (ref->new_sha1)) { + info->status =PUSH_STATUS_DELETE ; + }else if (!hashcmp (ref->old_sha1, ref->new_sha1)) + info->status =PUSH_STATUS_UPTODATE ; +else if (is_null_sha1 (ref->old_sha1)) + info->status =PUSH_STATUS_CREATE ; +else if (has_sha1_file (ref->old_sha1) && +ref_newer (ref->new_sha1, ref->old_sha1)) + info->status =PUSH_STATUS_FASTFORWARD ; +else + info->status =PUSH_STATUS_OUTOFDATE ; +// ref->peer_ref = NULL; /* local ref which is freed below */ + } +free_refs (local_refs); +free_refs (push_map); +return 0 ; +} + +static int get_push_ref_states_noquery (struct ref_states *states) +{ +int i; +struct remote *remote = states->remote; +struct string_list_item *item; +struct push_info *info; + +if (remote->mirror) +return 0 ; + + states->push.strdup_strings =1 ; +if (!remote->push_refspec_nr) { + item =string_list_append ("(matching)" , &states->push); + info = item->util =xcalloc (sizeof (struct push_info ),1 ); + info->status =PUSH_STATUS_NOTQUERIED ; + info->dest =xstrdup (item->string); + } +for (i =0 ; i < remote->push_refspec_nr; i++) { +struct refspec *spec = remote->push + i; +char buf[PATH_MAX ]; +if (spec->matching) + item =string_list_append ("(matching)" , &states->push); +else if (spec->pattern) { +snprintf (buf, (sizeof (buf)),"%s*" , spec->src); + item =string_list_append (buf, &states->push); +snprintf (buf, (sizeof (buf)),"%s*" , spec->dst); + }else if (strlen (spec->src)) + item =string_list_append (spec->src, &states->push); +else + item =string_list_append ("(delete)" , &states->push); + + info = item->util =xcalloc (sizeof (struct push_info ),1 ); + info->forced = spec->force; + info->status =PUSH_STATUS_NOTQUERIED ; +if (spec->pattern) + info->dest =xstrdup (buf); +else + info->dest =xstrdup (spec->dst ? spec->dst : item->string); + } +return 0 ; +} +static int get_head_names (const struct ref *remote_refs,const char *remote_name,struct ref_states *states) {
@@ -645,12 +752,20 @@ static int rm(int argc, const char **argv)return result; } +void clear_push_info (void *util,const char *string) +{ +struct push_info *info = util; +free (info->dest); +free (info); +} +static void free_remote_ref_states (struct ref_states *states) {string_list_clear (&states->new,0 );string_list_clear (&states->stale,0 );string_list_clear (&states->tracked,0 );string_list_clear (&states->heads,0 ); +string_list_clear_func (&states->push, clear_push_info); } static int append_ref_to_tracked_list(const char *refname,
@@ -693,9 +808,12 @@ static int get_remote_ref_states(const char *name,get_ref_states (remote_refs, states);if (query &GET_HEAD_NAMES )get_head_names (remote_refs, name, states); +if (query &GET_PUSH_REF_STATES ) +get_push_ref_states (remote_refs, states); }else {for_each_ref (append_ref_to_tracked_list, states);sort_string_list (&states->tracked); +get_push_ref_states_noquery (states); }return 0 ; }
@@ -703,7 +821,7 @@ static int get_remote_ref_states(const char *name,struct show_info {struct string_list *list;struct ref_states *states; -int width; +int width, width2;int any_rebase; };
@@ -798,6 +916,58 @@ int show_local_info_item(struct string_list_item *item, void *cb_data)return 0 ; } +int add_push_to_show_info (struct string_list_item *push_item,void *cb_data) +{ +struct show_info *show_info = cb_data; +struct push_info *push_info = push_item->util; +struct string_list_item *item; +int n; +if ((n =strlen (push_item->string)) > show_info->width) + show_info->width = n; +if ((n =strlen (push_info->dest)) > show_info->width2) + show_info->width2 = n; + item =string_list_append (push_item->string, show_info->list); + item->util = push_item->util; +return 0 ; +} + +int show_push_info_item (struct string_list_item *item,void *cb_data) +{ +struct show_info *show_info = cb_data; +struct push_info *push_info = item->util; +char *src = item->string, *status =NULL ; + +switch (push_info->status) { +case PUSH_STATUS_CREATE : + status ="create" ; +break ; +case PUSH_STATUS_DELETE : + status ="delete" ; + src ="(none)" ; +break ; +case PUSH_STATUS_UPTODATE : + status ="up to date" ; +break ; +case PUSH_STATUS_FASTFORWARD : + status ="fast forwardable" ; +break ; +case PUSH_STATUS_OUTOFDATE : + status ="local out of date" ; +break ; +case PUSH_STATUS_NOTQUERIED : +break ; + } +if (status) +printf (" %-*s %s to %-*s (%s)\n" , show_info->width, src, + push_info->forced ?"forces" :"pushes" , + show_info->width2, push_info->dest, status); +else +printf (" %-*s %s to %s\n" , show_info->width, src, + push_info->forced ?"forces" :"pushes" , + push_info->dest); +return 0 ; +} +static int show (int argc,const char **argv) {int no_query =0 , result =0 , query_flag =0 ;
@@ -816,7 +986,7 @@ static int show(int argc, const char **argv)return show_all ();if (!no_query) - query_flag = (GET_REF_STATES |GET_HEAD_NAMES ); + query_flag = (GET_REF_STATES |GET_HEAD_NAMES |GET_PUSH_REF_STATES );memset (&states,0 ,sizeof (states));memset (&info,0 ,sizeof (info));
@@ -866,19 +1036,18 @@ static int show(int argc, const char **argv)string_list_clear (info.list,0 );/* git push info */ -if (states.remote->push_refspec_nr) { -printf (" Local branch%s pushed with 'git push'\n" , - states.remote->push_refspec_nr >1 ? -"es" :"" ); -for (i =0 ; i < states.remote->push_refspec_nr; i++) { -struct refspec *spec = states.remote->push + i; -printf (" %s%s%s%s\n" , - spec->force ?"+" :"" , -abbrev_branch (spec->src), - spec->dst ?":" :"" , - spec->dst ?abbrev_branch (spec->dst) :"" ); - } - } +if (states.remote->mirror) +printf (" Local refs will be mirrored by 'git push'\n" ); + + info.width = info.width2 =0 ; +for_each_string_list (add_push_to_show_info, &states.push, &info); +sort_string_list (info.list); +if (info.list->nr) +printf (" Local ref%s configured for 'git push'%s:\n" , + info.list->nr >1 ?"s" :"" , + no_query ?" (status not queried)" :"" ); +for_each_string_list (show_push_info_item, info.list, &info); +string_list_clear (info.list,0 );free_remote_ref_states (&states); }
t/t5505-remote.sh +34 -23
87 lines not on this page · raw message
Signature · 1 line
1.6.2.rc1.223.gfed32