# [PATCH] match_push_refs(): nobody sets src->peer_ref anymore 2 messages from 2013-03-04 to 2013-03-05. Participants: Junio C Hamano, Jeff King. Thread: https://gitlist.dev/t/33068 ## Junio C Hamano, 2013-03-04 22:36 Subject: [PATCH] match_push_refs(): nobody sets src->peer_ref anymore Message-ID: <7vlia2x05q.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vlia2x05q.fsf%40alter.siamese.dyndns.org ``` In ancient times, we used to disallow the same source ref to be pushed to more than one places, e.g. "git push there master:master master:naster" was disallowed. We later lifted this restriction with db27ee63929f (send-pack: allow the same source to be pushed more than once., 2005-08-06) and there no longer is anybody that sets peer_ref for the source side of the ref list in the push codepath since then. Remove one leftover no-op in a loop that iterates over the source side of ref list (i.e. our local ref) to see if it can/should be sent to a matching destination ref while skipping ones that is marked with peer_ref (which will never exist, so we do not skip anything). Signed-off-by: Junio C Hamano --- remote.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/remote.c b/remote.c index e53a6eb..bac2b02 100644 --- a/remote.c +++ b/remote.c @@ -1227,9 +1227,6 @@ int match_push_refs(struct ref *src, struct ref **dst, const struct refspec *pat = NULL; char *dst_name; - if (ref->peer_ref) - continue; - dst_name = get_ref_match(rs, nr_refspec, ref, send_mirror, FROM_SRC, &pat); if (!dst_name) continue; -- 1.8.2-rc2-182-g857a7fa ``` ## Jeff King, 2013-03-05 05:45 Subject: Re: [PATCH] match_push_refs(): nobody sets src->peer_ref anymore Message-ID: <20130305054456.GA12446@sigill.intra.peff.net> URL: https://gitlist.dev/e/20130305054456.GA12446%40sigill.intra.peff.net In-Reply-To: <7vlia2x05q.fsf@alter.siamese.dyndns.org> ``` On Mon, Mar 04, 2013 at 02:36:33PM -0800, Junio C Hamano wrote: > In ancient times, we used to disallow the same source ref to be > pushed to more than one places, e.g. "git push there master:master > master:naster" was disallowed. We later lifted this restriction > with db27ee63929f (send-pack: allow the same source to be pushed > more than once., 2005-08-06) and there no longer is anybody that > sets peer_ref for the source side of the ref list in the push > codepath since then. > > Remove one leftover no-op in a loop that iterates over the source > side of ref list (i.e. our local ref) to see if it can/should be > sent to a matching destination ref while skipping ones that is > marked with peer_ref (which will never exist, so we do not skip > anything). > > Signed-off-by: Junio C Hamano This looks good to me. I was curious how difficult it would be to verify the "no longer is anybody that sets peer_ref" claim. It's actually quite easy. All of the call-sites just feed the result of get_local_heads(), which is easy to verify does not ever set peer_ref. Looking at the current code, I think you could even drop the "src" parameter and just have match_push_refs call get_local_heads() itself, though that does feel like a step backwards in terms of function generality. As an aside, I have always found the embedded "next" and "peer_ref" pointers in "struct ref" to be hacky. They are not properties of the ref at all, and the "refs to fetch" list would be more logically represented as a list of pairs of refs (and then I would not have to ever remember whether the peer is the local or remote ref in a given case). Probably not worth worrying about at this point, though, as the code fallout would be significant for little gain. -Peff ```