{"thread":{"id":"33068","subject":"[PATCH] match_push_refs(): nobody sets src->peer_ref anymore","startedAt":"2013-03-04T22:36:33Z","lastAt":"2013-03-05T05:45:06Z","messageCount":2,"participants":["Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"210594","messageId":"7vlia2x05q.fsf@alter.siamese.dyndns.org","threadId":"33068","inReplyTo":null,"subject":"[PATCH] match_push_refs(): nobody sets src->peer_ref anymore","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-04T22:36:33Z","receivedAt":"2013-03-04T22:36:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"In ancient times, we used to disallow the same source ref to be\npushed to more than one places, e.g. \"git push there master:master\nmaster:naster\" was disallowed.  We later lifted this restriction\nwith db27ee63929f (send-pack: allow the same source to be pushed\nmore than once., 2005-08-06) and there no longer is anybody that\nsets peer_ref for the source side of the ref list in the push\ncodepath since then.\n\nRemove one leftover no-op in a loop that iterates over the source\nside of ref list (i.e. our local ref) to see if it can/should be\nsent to a matching destination ref while skipping ones that is\nmarked with peer_ref (which will never exist, so we do not skip\nanything).\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n remote.c | 3 ---\n 1 file changed, 3 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex e53a6eb..bac2b02 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1227,9 +1227,6 @@ int match_push_refs(struct ref *src, struct ref **dst,\n \t\tconst struct refspec *pat = NULL;\n \t\tchar *dst_name;\n \n-\t\tif (ref->peer_ref)\n-\t\t\tcontinue;\n-\n \t\tdst_name = get_ref_match(rs, nr_refspec, ref, send_mirror, FROM_SRC, &pat);\n \t\tif (!dst_name)\n \t\t\tcontinue;\n-- \n1.8.2-rc2-182-g857a7fa\n"},{"id":"210608","messageId":"20130305054456.GA12446@sigill.intra.peff.net","threadId":"33068","inReplyTo":"7vlia2x05q.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] match_push_refs(): nobody sets src->peer_ref anymore","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-03-05T05:45:06Z","receivedAt":"2013-03-05T05:45:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 04, 2013 at 02:36:33PM -0800, Junio C Hamano wrote:\n\n> In ancient times, we used to disallow the same source ref to be\n> pushed to more than one places, e.g. \"git push there master:master\n> master:naster\" was disallowed.  We later lifted this restriction\n> with db27ee63929f (send-pack: allow the same source to be pushed\n> more than once., 2005-08-06) and there no longer is anybody that\n> sets peer_ref for the source side of the ref list in the push\n> codepath since then.\n> \n> Remove one leftover no-op in a loop that iterates over the source\n> side of ref list (i.e. our local ref) to see if it can/should be\n> sent to a matching destination ref while skipping ones that is\n> marked with peer_ref (which will never exist, so we do not skip\n> anything).\n> \n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n\nThis looks good to me. I was curious how difficult it would be to verify\nthe \"no longer is anybody that sets peer_ref\" claim. It's actually quite\neasy. All of the call-sites just feed the result of get_local_heads(),\nwhich is easy to verify does not ever set peer_ref. Looking at the\ncurrent code, I think you could even drop the \"src\" parameter and just\nhave match_push_refs call get_local_heads() itself, though that does\nfeel like a step backwards in terms of function generality.\n\nAs an aside, I have always found the embedded \"next\" and \"peer_ref\"\npointers in \"struct ref\" to be hacky. They are not properties of the ref\nat all, and the \"refs to fetch\" list would be more logically represented\nas a list of pairs of refs (and then I would not have to ever remember\nwhether the peer is the local or remote ref in a given case). Probably\nnot worth worrying about at this point, though, as the code fallout\nwould be significant for little gain.\n\n-Peff\n"}]}