{"thread":{"id":"8492","subject":"git-send-pack: broken handling of ref specs with wildcards","startedAt":"2007-06-07T22:53:02Z","lastAt":"2007-06-08T18:47:46Z","messageCount":8,"participants":["Alex Riesen","Johannes Schindelin","Junio C Hamano","Daniel Barkalow"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"44277","messageId":"20070607225302.GA10633@steel.home","threadId":"8492","inReplyTo":null,"subject":"git-send-pack: broken handling of ref specs with wildcards","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-06-07T22:53:02Z","receivedAt":"2007-06-07T22:53:02Z","isPatch":false,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Try something like this:\n\n    git-send-pack --remote=origin --thin /some/other/repo \\\n    'refs/heads/*:refs/remotes/child/*'\n\nThe result looks broken: the sent reference are created not in\nrefs/remotes/child/ but just in refs/heads/ of /some/other/repo.\n"},{"id":"44279","messageId":"Pine.LNX.4.64.0706072356220.4046@racer.site","threadId":"8492","inReplyTo":"20070607225302.GA10633@steel.home","subject":"Re: git-send-pack: broken handling of ref specs with wildcards","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-06-07T22:56:56Z","receivedAt":"2007-06-07T22:56:56Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 8 Jun 2007, Alex Riesen wrote:\n\n> Try something like this:\n> \n>     git-send-pack --remote=origin --thin /some/other/repo \\\n>     'refs/heads/*:refs/remotes/child/*'\n> \n> The result looks broken: the sent reference are created not in\n> refs/remotes/child/ but just in refs/heads/ of /some/other/repo.\n\nI had the impression that it was git-push, a porcelain, which handles \nrefspec wildcards, not send-pack, which is plumbing.\n\nCiao,\nDscho\n"},{"id":"44288","messageId":"20070607234305.GB10633@steel.home","threadId":"8492","inReplyTo":"20070607225302.GA10633@steel.home","subject":"[PATCH] Fix push with refspecs containing wildcards","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-06-07T23:43:05Z","receivedAt":"2007-06-07T23:43:05Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Otherwise\n\n    git push 'remote-name' 'refs/heads/*:refs/remotes/other/*'\n\nwill consider references in \"refs/heads\" of the remote repository\n\"remote-name\", instead of the ones in \"refs/remotes/other\", which\nthe given refspec clearly means.\n\nSigned-off-by: Alex Riesen <raa.lkml@gmail.com>\n---\nAlex Riesen, Fri, Jun 08, 2007 00:53:02 +0200:\n> Try something like this:\n> \n>     git-send-pack --remote=origin --thin /some/other/repo \\\n>     'refs/heads/*:refs/remotes/child/*'\n> \n> The result looks broken: the sent reference are created not in\n> refs/remotes/child/ but just in refs/heads/ of /some/other/repo.\n> \n\nAnd as usual, I'm not sure if it is the right fix. Johannes has a\npoint: why the hell does send-pack parse the wildcards at all?!\n\n remote.c |   39 +++++++++++++++++++++++++++------------\n 1 files changed, 27 insertions(+), 12 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex d904616..33c8e50 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -501,16 +501,16 @@ static struct ref *find_ref_by_name(struct ref *list, const char *name)\n \treturn NULL;\n }\n \n-static int check_pattern_match(struct refspec *rs, int rs_nr, struct ref *src)\n+static const struct refspec *check_pattern_match(const struct refspec *rs,\n+\t\t\t\t\t\t int rs_nr,\n+\t\t\t\t\t\t const struct ref *src)\n {\n \tint i;\n-\tif (!rs_nr)\n-\t\treturn 1;\n \tfor (i = 0; i < rs_nr; i++) {\n \t\tif (rs[i].pattern && !prefixcmp(src->name, rs[i].src))\n-\t\t\treturn 1;\n+\t\t\treturn rs + i;\n \t}\n-\treturn 0;\n+\treturn NULL;\n }\n \n int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail,\n@@ -525,29 +525,44 @@ int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail,\n \t/* pick the remainder */\n \tfor ( ; src; src = src->next) {\n \t\tstruct ref *dst_peer;\n+\t\tconst struct refspec *pat = NULL;\n+\t\tchar *dst_name;\n \t\tif (src->peer_ref)\n \t\t\tcontinue;\n-\t\tif (!check_pattern_match(rs, nr_refspec, src))\n-\t\t\tcontinue;\n+\t\tif (nr_refspec) {\n+\t\t\tpat = check_pattern_match(rs, nr_refspec, src);\n+\t\t\tif (!pat)\n+\t\t\t\tcontinue;\n+\t\t}\n \n-\t\tdst_peer = find_ref_by_name(dst, src->name);\n+\t\tif (pat) {\n+\t\t\tdst_name = xmalloc(strlen(pat->dst) +\n+\t\t\t\t\t   strlen(src->name) -\n+\t\t\t\t\t   strlen(pat->src) + 2);\n+\t\t\tstrcpy(dst_name, pat->dst);\n+\t\t\tstrcat(dst_name, src->name + strlen(pat->src));\n+\t\t} else\n+\t\t\tdst_name = strdup(src->name);\n+\t\tdst_peer = find_ref_by_name(dst, dst_name);\n \t\tif (dst_peer && dst_peer->peer_ref)\n \t\t\t/* We're already sending something to this ref. */\n-\t\t\tcontinue;\n+\t\t\tgoto free_name;\n \t\tif (!dst_peer && !nr_refspec && !all)\n \t\t\t/* Remote doesn't have it, and we have no\n \t\t\t * explicit pattern, and we don't have\n \t\t\t * --all. */\n-\t\t\tcontinue;\n+\t\t\tgoto free_name;\n \t\tif (!dst_peer) {\n \t\t\t/* Create a new one and link it */\n-\t\t\tint len = strlen(src->name) + 1;\n+\t\t\tint len = strlen(dst_name) + 1;\n \t\t\tdst_peer = xcalloc(1, sizeof(*dst_peer) + len);\n-\t\t\tmemcpy(dst_peer->name, src->name, len);\n+\t\t\tmemcpy(dst_peer->name, dst_name, len);\n \t\t\thashcpy(dst_peer->new_sha1, src->new_sha1);\n \t\t\tlink_dst_tail(dst_peer, dst_tail);\n \t\t}\n \t\tdst_peer->peer_ref = src;\n+\tfree_name:\n+\t\tfree(dst_name);\n \t}\n \treturn 0;\n }\n-- \n1.5.2.1.147.g7c82d\n"},{"id":"44313","messageId":"7v8xavlzu0.fsf@assigned-by-dhcp.cox.net","threadId":"8492","inReplyTo":"Pine.LNX.4.64.0706072356220.4046@racer.site","subject":"Re: git-send-pack: broken handling of ref specs with wildcards","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-06-08T06:33:59Z","receivedAt":"2007-06-08T06:33:59Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> On Fri, 8 Jun 2007, Alex Riesen wrote:\n>\n>> Try something like this:\n>> \n>>     git-send-pack --remote=origin --thin /some/other/repo \\\n>>     'refs/heads/*:refs/remotes/child/*'\n>> \n>> The result looks broken: the sent reference are created not in\n>> refs/remotes/child/ but just in refs/heads/ of /some/other/repo.\n>\n> I had the impression that it was git-push, a porcelain, which handles \n> refspec wildcards, not send-pack, which is plumbing.\n\nWell, I do not think it is wrong per-se if plumbing send-pack\nstarts understanding 'a/*:b/*'.  Earlier it only understood\nconcrete refspecs, and such a refspec that has asterisks cannot\nbe concrete, as '*' is an invalid char in a refname.  So there\nis no risk of confusion.\n\nThat is, as long as it is done right, which was not true in this\ncase.\n"},{"id":"44314","messageId":"7v4pljlzm3.fsf@assigned-by-dhcp.cox.net","threadId":"8492","inReplyTo":"20070607234305.GB10633@steel.home","subject":"Re: [PATCH] Fix push with refspecs containing wildcards","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-06-08T06:38:44Z","receivedAt":"2007-06-08T06:38:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sending a fix is a good thing, but whenever doing one, could\npeople please also do a testcase that demonstrates the original\nbug, and also a demonstration that the fix does not introduce\nregression?\n\nFor this one, obviously a test for push that uses such wildcard\nref is needed but at the same time we would want a test for push\nthat does _not_ use a wildcard, fetch that uses a wildcard, and\na fetch that does not use a wildcard.\n"},{"id":"44321","messageId":"7vodjqlwnc.fsf@assigned-by-dhcp.cox.net","threadId":"8492","inReplyTo":"7v4pljlzm3.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Fix push with refspecs containing wildcards","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-06-08T07:42:47Z","receivedAt":"2007-06-08T07:42:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Sending a fix is a good thing, but whenever doing one, could\n> people please also do a testcase that demonstrates the original\n> bug, and also a demonstration that the fix does not introduce\n> regression?\n>\n> For this one, obviously a test for push that uses such wildcard\n> ref is needed but at the same time we would want a test for push\n> that does _not_ use a wildcard, fetch that uses a wildcard, and\n> a fetch that does not use a wildcard.\n\nHow about this?\n---\n\n t/t5516-fetch-push.sh |   82 +++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 82 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nnew file mode 100755\nindex 0000000..dba018f\n--- /dev/null\n+++ b/t/t5516-fetch-push.sh\n@@ -0,0 +1,80 @@\n+#!/bin/sh\n+\n+test_description='fetching and pushing, with or without wildcard'\n+\n+. ./test-lib.sh\n+\n+mk_empty () {\n+\trm -fr testrepo &&\n+\tmkdir testrepo &&\n+\t(\n+\t\tcd testrepo &&\n+\t\tgit init\n+\t)\n+}\n+\n+test_expect_success setup '\n+\n+\t: >path1 &&\n+\tgit add path1 &&\n+\ttest_tick &&\n+\tgit commit -a -m repo &&\n+\tthe_commit=$(git show-ref -s --verify refs/heads/master)\n+\n+'\n+\n+test_expect_success 'fetch without wildcard' '\n+\tmk_empty &&\n+\t(\n+\t\tcd testrepo &&\n+\t\tgit fetch .. refs/heads/master:refs/remotes/origin/master &&\n+\n+\t\tr=$(git show-ref -s --verify refs/remotes/origin/master) &&\n+\t\ttest \"z$r\" = \"z$the_commit\" &&\n+\n+\t\ttest 1 = $(git for-each-ref refs/remotes/origin | wc -l)\n+\t)\n+'\n+\n+test_expect_success 'fetch with wildcard' '\n+\tmk_empty &&\n+\t(\n+\t\tcd testrepo &&\n+\t\tgit config remote.up.url .. &&\n+\t\tgit config remote.up.fetch \"refs/heads/*:refs/remotes/origin/*\" &&\n+\t\tgit fetch up &&\n+\n+\t\tr=$(git show-ref -s --verify refs/remotes/origin/master) &&\n+\t\ttest \"z$r\" = \"z$the_commit\" &&\n+\n+\t\ttest 1 = $(git for-each-ref refs/remotes/origin | wc -l)\n+\t)\n+'\n+\n+test_expect_success 'push without wildcard' '\n+\tmk_empty &&\n+\n+\tgit push testrepo refs/heads/master:refs/remotes/origin/master &&\n+\t(\n+\t\tcd testrepo &&\n+\t\tr=$(git show-ref -s --verify refs/remotes/origin/master) &&\n+\t\ttest \"z$r\" = \"z$the_commit\" &&\n+\n+\t\ttest 1 = $(git for-each-ref refs/remotes/origin | wc -l)\n+\t)\n+'\n+\n+test_expect_success 'push with wildcard' '\n+\tmk_empty &&\n+\n+\tgit push testrepo \"refs/heads/*:refs/remotes/origin/*\" &&\n+\t(\n+\t\tcd testrepo &&\n+\t\tr=$(git show-ref -s --verify refs/remotes/origin/master) &&\n+\t\ttest \"z$r\" = \"z$the_commit\" &&\n+\n+\t\ttest 1 = $(git for-each-ref refs/remotes/origin | wc -l)\n+\t)\n+'\n+\n+test_done\n"},{"id":"44327","messageId":"20070608091332.GA2429@steel.home","threadId":"8492","inReplyTo":"7vodjqlwnc.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Fix push with refspecs containing wildcards","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-06-08T09:13:32Z","receivedAt":"2007-06-08T09:13:32Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Junio C Hamano, Fri, Jun 08, 2007 09:42:47 +0200:\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n> > Sending a fix is a good thing, but whenever doing one, could\n> > people please also do a testcase that demonstrates the original\n> > bug, and also a demonstration that the fix does not introduce\n> > regression?\n\nSorry. I actually have a test script I used to develop the fix,\nbut somehow missed to send it.\n\n> > For this one, obviously a test for push that uses such wildcard\n> > ref is needed but at the same time we would want a test for push\n> > that does _not_ use a wildcard, fetch that uses a wildcard, and\n> > a fetch that does not use a wildcard.\n> \n> How about this?\n> ---\n> \n>  t/t5516-fetch-push.sh |   82 +++++++++++++++++++++++++++++++++++++++++++++++++\n>  1 files changed, 82 insertions(+), 0 deletions(-)\n> \n\nGratefully-Acked-by: Alex Riesen <raa.lkml@gmail.com>\n"},{"id":"44367","messageId":"Pine.LNX.4.64.0706081357420.5848@iabervon.org","threadId":"8492","inReplyTo":"7v8xavlzu0.fsf@assigned-by-dhcp.cox.net","subject":"Re: git-send-pack: broken handling of ref specs with wildcards","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2007-06-08T18:47:46Z","receivedAt":"2007-06-08T18:47:46Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Thu, 7 Jun 2007, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> > On Fri, 8 Jun 2007, Alex Riesen wrote:\n> >\n> >> Try something like this:\n> >> \n> >>     git-send-pack --remote=origin --thin /some/other/repo \\\n> >>     'refs/heads/*:refs/remotes/child/*'\n> >> \n> >> The result looks broken: the sent reference are created not in\n> >> refs/remotes/child/ but just in refs/heads/ of /some/other/repo.\n> >\n> > I had the impression that it was git-push, a porcelain, which handles \n> > refspec wildcards, not send-pack, which is plumbing.\n> \n> Well, I do not think it is wrong per-se if plumbing send-pack\n> starts understanding 'a/*:b/*'.  Earlier it only understood\n> concrete refspecs, and such a refspec that has asterisks cannot\n> be concrete, as '*' is an invalid char in a refname.  So there\n> is no risk of confusion.\n> \n> That is, as long as it is done right, which was not true in this\n> case.\n\nGood catch; all of my configurations have a {foo}/*:{foo}/* pattern for \nthe push, and I hadn't figured out a way to write tests for push and \nfetch.\n\nThe code looks like it should do the right thing... now that somebody's \nfixed it before I got to it. You people are far too efficient.\n\nI'll probably break out a function for applying a pattern, since there are \nmultiple places that need to do it, but that can go into a series I'm \nworking on now.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"}]}