{"thread":{"id":"18162","subject":"[PATCH 0/5] Extend pattern refspecs","startedAt":"2009-03-06T04:56:13Z","lastAt":"2009-03-09T15:46:25Z","messageCount":9,"participants":["Daniel Barkalow","Jay Soffian","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"107140","messageId":"alpine.LNX.1.00.0903052346270.19665@iabervon.org","threadId":"18162","inReplyTo":null,"subject":"[PATCH 0/5] Extend pattern refspecs","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-03-06T04:56:13Z","receivedAt":"2009-03-06T04:56:13Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"This series only supports the narrowest case of having the * in the middle \nof a side of a refspec: having it as a full path component on each side.\n\nPatches 1-3 centralize all of the parsing and matching rules to a pair of \nfunctions; patch 4 makes the stored representation more convenient (and \nserves as a distinguished bisection outcome for anything I missed that was \nrelying on the contents of struct refspec for patterns); and patch 5 \nextends the matching implementation and loosens the ref format \nrequirements to allow the * to be in the middle.\n\nAn easy followup would relax the restrictions further without requiring \nany particularly tricky further changes.\n\nDaniel Barkalow (5):\n  Make clone parse the default fetch refspec with the regular code\n  Use a single function to match names against patterns\n  Use the matching function to generate the match results\n  Keep '*' in pattern refspecs\n  Support '*' in the middle of a refspec\n\n builtin-clone.c    |   25 ++++++++--------\n refs.c             |   15 +++++----\n remote.c           |   78 +++++++++++++++++++++++++++++----------------------\n t/t5511-refspec.sh |   12 ++++++++\n 4 files changed, 77 insertions(+), 53 deletions(-)\n"},{"id":"107146","messageId":"76718490903052119y4d6a7e0ck24bfeb1c0964e413@mail.gmail.com","threadId":"18162","inReplyTo":"alpine.LNX.1.00.0903052346270.19665@iabervon.org","subject":"Re: [PATCH 0/5] Extend pattern refspecs","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-03-06T05:19:42Z","receivedAt":"2009-03-06T05:19:42Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Thu, Mar 5, 2009 at 11:56 PM, Daniel Barkalow <barkalow@iabervon.org> wrote:\n> This series only supports the narrowest case of having the * in the middle\n> of a side of a refspec: having it as a full path component on each side.\n>\n> Patches 1-3 centralize all of the parsing and matching rules to a pair of\n> functions; patch 4 makes the stored representation more convenient (and\n> serves as a distinguished bisection outcome for anything I missed that was\n> relying on the contents of struct refspec for patterns); and patch 5\n> extends the matching implementation and loosens the ref format\n> requirements to allow the * to be in the middle.\n>\n> An easy followup would relax the restrictions further without requiring\n> any particularly tricky further changes.\n\nThis series and js/remote-improvements (e5dcbfd) in pu may not get\nalong completely. \"git remote show\" tries to show how the refspecs\nexpand out. And actually, that should be fine since builtin-remote now\nuses the same code as fetch/push to expand the refs.\n\nHowever, \"git remote show -n\" (-n means don't query the remote) makes\nuse of a new function, get_push_ref_states_noquery(), which more or\nless tries to reverse the parsed refspec back into the original\nstring. That function relies on the current (before your patch)\nrefspec semantics and assumes if refspec.pattern is set, then the '*'\nis at the end. So it will need tweaking.\n\nj.\n"},{"id":"107151","messageId":"alpine.LNX.1.00.0903060038510.19665@iabervon.org","threadId":"18162","inReplyTo":"76718490903052119y4d6a7e0ck24bfeb1c0964e413@mail.gmail.com","subject":"Re: [PATCH 0/5] Extend pattern refspecs","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-03-06T06:07:20Z","receivedAt":"2009-03-06T06:07:20Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Fri, 6 Mar 2009, Jay Soffian wrote:\n\n> On Thu, Mar 5, 2009 at 11:56 PM, Daniel Barkalow <barkalow@iabervon.org> wrote:\n> > This series only supports the narrowest case of having the * in the middle\n> > of a side of a refspec: having it as a full path component on each side.\n> >\n> > Patches 1-3 centralize all of the parsing and matching rules to a pair of\n> > functions; patch 4 makes the stored representation more convenient (and\n> > serves as a distinguished bisection outcome for anything I missed that was\n> > relying on the contents of struct refspec for patterns); and patch 5\n> > extends the matching implementation and loosens the ref format\n> > requirements to allow the * to be in the middle.\n> >\n> > An easy followup would relax the restrictions further without requiring\n> > any particularly tricky further changes.\n> \n> This series and js/remote-improvements (e5dcbfd) in pu may not get\n> along completely. \"git remote show\" tries to show how the refspecs\n> expand out. And actually, that should be fine since builtin-remote now\n> uses the same code as fetch/push to expand the refs.\n> \n> However, \"git remote show -n\" (-n means don't query the remote) makes\n> use of a new function, get_push_ref_states_noquery(), which more or\n> less tries to reverse the parsed refspec back into the original\n> string. That function relies on the current (before your patch)\n> refspec semantics and assumes if refspec.pattern is set, then the '*'\n> is at the end. So it will need tweaking.\n\nActually, you should be able to just drop your \"buf\" and use spec->src and \nspec->dst, since it just stores the original strings. So that should be \neasy enough, although it might be good to go through a remote.c function \njust in case it becomes more complicated later. On the other hand, \nget_head_names() should probably get a patch like my 1/5 to have it use \nthe remote.c parser, or should use a constant \"head mirror\" refspec like \nthat tag_refspec already in remote.c\n\nDo you have tests for \"git remote show -n\"? Merging my series (on top of \norigin/master) and e5dcbfd and adding a final '*' to the string in \nget_head_names() made everything pass for me, without doing anything about \nthe extra *s, but the output is clearly not quite right.\n\nI'm not seeing anything that makes assumptions about the matching \nsemantics of pattern refspecs, just stuff about how the stored form \nrelates to the config-file form.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"107154","messageId":"76718490903052252y1778aa41g8f3e52329f7bf288@mail.gmail.com","threadId":"18162","inReplyTo":"alpine.LNX.1.00.0903060038510.19665@iabervon.org","subject":"Re: [PATCH 0/5] Extend pattern refspecs","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-03-06T06:52:49Z","receivedAt":"2009-03-06T06:52:49Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Fri, Mar 6, 2009 at 1:07 AM, Daniel Barkalow <barkalow@iabervon.org> wrote:\n> On Fri, 6 Mar 2009, Jay Soffian wrote:\n>\n> Actually, you should be able to just drop your \"buf\" and use spec->src and\n> spec->dst, since it just stores the original strings. So that should be\n> easy enough, although it might be good to go through a remote.c function\n> just in case it becomes more complicated later. On the other hand,\n> get_head_names() should probably get a patch like my 1/5 to have it use\n> the remote.c parser, or should use a constant \"head mirror\" refspec like\n> that tag_refspec already in remote.c\n\nOkay.\n\n> Do you have tests for \"git remote show -n\"?\n\nYes. Apparently not enough of them though if nothing is failing.\n\n> Merging my series (on top of\n> origin/master) and e5dcbfd and adding a final '*' to the string in\n> get_head_names() made everything pass for me, without doing anything about\n> the extra *s, but the output is clearly not quite right.\n\nHmm, alright.\n\n> I'm not seeing anything that makes assumptions about the matching\n> semantics of pattern refspecs, just stuff about how the stored form\n> relates to the config-file form.\n\nOkay, that sounds right.\n\nI assume your series will end up in pu soon enough, and I think my\nseries is about to hop to next. What's the right way to to have them\nbe happy together?\n\nj.\n"},{"id":"107156","messageId":"alpine.LNX.1.00.0903060153490.19665@iabervon.org","threadId":"18162","inReplyTo":"76718490903052252y1778aa41g8f3e52329f7bf288@mail.gmail.com","subject":"Re: [PATCH 0/5] Extend pattern refspecs","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-03-06T07:03:04Z","receivedAt":"2009-03-06T07:03:04Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Fri, 6 Mar 2009, Jay Soffian wrote:\n\n> On Fri, Mar 6, 2009 at 1:07 AM, Daniel Barkalow <barkalow@iabervon.org> wrote:\n> > On Fri, 6 Mar 2009, Jay Soffian wrote:\n> >\n> > Actually, you should be able to just drop your \"buf\" and use spec->src and\n> > spec->dst, since it just stores the original strings. So that should be\n> > easy enough, although it might be good to go through a remote.c function\n> > just in case it becomes more complicated later. On the other hand,\n> > get_head_names() should probably get a patch like my 1/5 to have it use\n> > the remote.c parser, or should use a constant \"head mirror\" refspec like\n> > that tag_refspec already in remote.c\n> \n> Okay.\n> \n> > Do you have tests for \"git remote show -n\"?\n> \n> Yes. Apparently not enough of them though if nothing is failing.\n\nIt only seems to be off by saying:\n\n  Local ref configured for 'git push' (status not queried):\n    refs/heads/** forces to refs/heads/**\n\nso you didn't necessarily miss much, just the one thing I seem to have \nbroken.\n\n> > Merging my series (on top of\n> > origin/master) and e5dcbfd and adding a final '*' to the string in\n> > get_head_names() made everything pass for me, without doing anything about\n> > the extra *s, but the output is clearly not quite right.\n> \n> Hmm, alright.\n> \n> > I'm not seeing anything that makes assumptions about the matching\n> > semantics of pattern refspecs, just stuff about how the stored form\n> > relates to the config-file form.\n> \n> Okay, that sounds right.\n> \n> I assume your series will end up in pu soon enough, and I think my\n> series is about to hop to next. What's the right way to to have them\n> be happy together?\n\nThe only \"correctness of outcome\" issue is the open-coded refspec \ninitialization, I think, which is probably actually cleaner to have as a \nconstant in remote.c anyway (unlike in builtin-clone, there's no \nvariability at all, so it might as well be a constant.\n\nI can amend my series to avoid adding the extra * in the message when your \nseries graduates, and it should be clean enough to deal with if your \nseries wound up getting dropped later; it'll be the only change in that \nfile for my series, so I'd be able to drop it easily.\n\nIt'd be useful to have that message tested by your series, though, so I \ncan verify my series reliably without worrying about whether I \naccidentally dropped both the fix and the test.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"107160","messageId":"76718490903052359r27153d01m468f61889ee2f196@mail.gmail.com","threadId":"18162","inReplyTo":"alpine.LNX.1.00.0903060153490.19665@iabervon.org","subject":"Re: [PATCH 0/5] Extend pattern refspecs","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-03-06T07:59:26Z","receivedAt":"2009-03-06T07:59:26Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Fri, Mar 6, 2009 at 2:03 AM, Daniel Barkalow <barkalow@iabervon.org> wrote:\n> It'd be useful to have that message tested by your series, though, so I\n> can verify my series reliably without worrying about whether I\n> accidentally dropped both the fix and the test.\n\nI'll send a patch for it in the next day or two.\n\nj.\n"},{"id":"107374","messageId":"7vocwcl8ku.fsf@gitster.siamese.dyndns.org","threadId":"18162","inReplyTo":"76718490903052119y4d6a7e0ck24bfeb1c0964e413@mail.gmail.com","subject":"Re: [PATCH 0/5] Extend pattern refspecs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-08T08:31:13Z","receivedAt":"2009-03-08T08:31:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jay Soffian <jaysoffian@gmail.com> writes:\n\n> This series and js/remote-improvements (e5dcbfd) in pu may not get\n> along completely. \"git remote show\" tries to show how the refspecs\n> expand out.\n\nI've created an \"early semantic conflict resolution\" topic branch that is\na cross between this series and js/remote-improvements, like so:\n\n    $ git checkout -b xx/db-refspec-vs-js-remote db/refspec-wildcard-in-the-middle\n    $ git merge js/remote-improvements\n    $ git apply evil-fixup.diff\n    $ git commit --amend -a -m \"Evil merge.\"\n\nwith the following \"fixup-as-an-evil-merge patch\", which I'd appreciate if\nyou two can sanity check.\n\nThe result will be queued at the tip of 'pu', and hopefully I can merge it\nto 'next' when this series goes to 'next'.  Similarly, whichever one goes\nfirst to 'master' can be merged straight, but the merge of the other one\nneeds to merge this branch as well.\n\ndiff --git a/builtin-remote.c b/builtin-remote.c\nindex 7e82a52..3e4a41b 100644\n--- a/builtin-remote.c\n+++ b/builtin-remote.c\n@@ -359,14 +358,9 @@ static int get_push_ref_states_noquery(struct ref_states *states)\n \t}\n \tfor (i = 0; i < remote->push_refspec_nr; i++) {\n \t\tstruct refspec *spec = remote->push + i;\n-\t\tchar buf[PATH_MAX];\n \t\tif (spec->matching)\n \t\t\titem = string_list_append(\"(matching)\", &states->push);\n-\t\telse if (spec->pattern) {\n-\t\t\tsnprintf(buf, (sizeof(buf)), \"%s*\", spec->src);\n-\t\t\titem = string_list_append(buf, &states->push);\n-\t\t\tsnprintf(buf, (sizeof(buf)), \"%s*\", spec->dst);\n-\t\t} else if (strlen(spec->src))\n+\t\telse if (strlen(spec->src))\n \t\t\titem = string_list_append(spec->src, &states->push);\n \t\telse\n \t\t\titem = string_list_append(\"(delete)\", &states->push);\n@@ -374,10 +368,7 @@ static int get_push_ref_states_noquery(struct ref_states *states)\n \t\tinfo = item->util = xcalloc(sizeof(struct push_info), 1);\n \t\tinfo->forced = spec->force;\n \t\tinfo->status = PUSH_STATUS_NOTQUERIED;\n-\t\tif (spec->pattern)\n-\t\t\tinfo->dest = xstrdup(buf);\n-\t\telse\n-\t\t\tinfo->dest = xstrdup(spec->dst ? spec->dst : item->string);\n+\t\tinfo->dest = xstrdup(spec->dst ? spec->dst : item->string);\n \t}\n \treturn 0;\n }\n@@ -390,7 +381,7 @@ static int get_head_names(const struct ref *remote_refs, struct ref_states *stat\n \n \trefspec.force = 0;\n \trefspec.pattern = 1;\n-\trefspec.src = refspec.dst = \"refs/heads/\";\n+\trefspec.src = refspec.dst = \"refs/heads/*\";\n \tstates->heads.strdup_strings = 1;\n \tget_fetch_map(remote_refs, &refspec, &fetch_map_tail, 0);\n \tmatches = guess_remote_head(find_ref_by_name(remote_refs, \"HEAD\"),\n"},{"id":"107375","messageId":"alpine.LNX.1.00.0903080447450.19665@iabervon.org","threadId":"18162","inReplyTo":"7vocwcl8ku.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 0/5] Extend pattern refspecs","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-03-08T08:49:26Z","receivedAt":"2009-03-08T08:49:26Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Sun, 8 Mar 2009, Junio C Hamano wrote:\n\n> Jay Soffian <jaysoffian@gmail.com> writes:\n> \n> > This series and js/remote-improvements (e5dcbfd) in pu may not get\n> > along completely. \"git remote show\" tries to show how the refspecs\n> > expand out.\n> \n> I've created an \"early semantic conflict resolution\" topic branch that is\n> a cross between this series and js/remote-improvements, like so:\n> \n>     $ git checkout -b xx/db-refspec-vs-js-remote db/refspec-wildcard-in-the-middle\n>     $ git merge js/remote-improvements\n>     $ git apply evil-fixup.diff\n>     $ git commit --amend -a -m \"Evil merge.\"\n> \n> with the following \"fixup-as-an-evil-merge patch\", which I'd appreciate if\n> you two can sanity check.\n\nThat looks like what I'd come up with as a resolution, too, so I think \nit's sane unless Jay knows of another way to get remote to care about \npatterns.\n\n> The result will be queued at the tip of 'pu', and hopefully I can merge it\n> to 'next' when this series goes to 'next'.  Similarly, whichever one goes\n> first to 'master' can be merged straight, but the merge of the other one\n> needs to merge this branch as well.\n> \n> diff --git a/builtin-remote.c b/builtin-remote.c\n> index 7e82a52..3e4a41b 100644\n> --- a/builtin-remote.c\n> +++ b/builtin-remote.c\n> @@ -359,14 +358,9 @@ static int get_push_ref_states_noquery(struct ref_states *states)\n>  \t}\n>  \tfor (i = 0; i < remote->push_refspec_nr; i++) {\n>  \t\tstruct refspec *spec = remote->push + i;\n> -\t\tchar buf[PATH_MAX];\n>  \t\tif (spec->matching)\n>  \t\t\titem = string_list_append(\"(matching)\", &states->push);\n> -\t\telse if (spec->pattern) {\n> -\t\t\tsnprintf(buf, (sizeof(buf)), \"%s*\", spec->src);\n> -\t\t\titem = string_list_append(buf, &states->push);\n> -\t\t\tsnprintf(buf, (sizeof(buf)), \"%s*\", spec->dst);\n> -\t\t} else if (strlen(spec->src))\n> +\t\telse if (strlen(spec->src))\n>  \t\t\titem = string_list_append(spec->src, &states->push);\n>  \t\telse\n>  \t\t\titem = string_list_append(\"(delete)\", &states->push);\n> @@ -374,10 +368,7 @@ static int get_push_ref_states_noquery(struct ref_states *states)\n>  \t\tinfo = item->util = xcalloc(sizeof(struct push_info), 1);\n>  \t\tinfo->forced = spec->force;\n>  \t\tinfo->status = PUSH_STATUS_NOTQUERIED;\n> -\t\tif (spec->pattern)\n> -\t\t\tinfo->dest = xstrdup(buf);\n> -\t\telse\n> -\t\t\tinfo->dest = xstrdup(spec->dst ? spec->dst : item->string);\n> +\t\tinfo->dest = xstrdup(spec->dst ? spec->dst : item->string);\n>  \t}\n>  \treturn 0;\n>  }\n> @@ -390,7 +381,7 @@ static int get_head_names(const struct ref *remote_refs, struct ref_states *stat\n>  \n>  \trefspec.force = 0;\n>  \trefspec.pattern = 1;\n> -\trefspec.src = refspec.dst = \"refs/heads/\";\n> +\trefspec.src = refspec.dst = \"refs/heads/*\";\n>  \tstates->heads.strdup_strings = 1;\n>  \tget_fetch_map(remote_refs, &refspec, &fetch_map_tail, 0);\n>  \tmatches = guess_remote_head(find_ref_by_name(remote_refs, \"HEAD\"),\n> \n"},{"id":"107467","messageId":"76718490903090846x2e84816emed5e21da9788680d@mail.gmail.com","threadId":"18162","inReplyTo":"alpine.LNX.1.00.0903080447450.19665@iabervon.org","subject":"Re: [PATCH 0/5] Extend pattern refspecs","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-03-09T15:46:25Z","receivedAt":"2009-03-09T15:46:25Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Sun, Mar 8, 2009 at 4:49 AM, Daniel Barkalow <barkalow@iabervon.org> wrote:\n> On Sun, 8 Mar 2009, Junio C Hamano wrote:\n>\n>> Jay Soffian <jaysoffian@gmail.com> writes:\n>>\n>> > This series and js/remote-improvements (e5dcbfd) in pu may not get\n>> > along completely. \"git remote show\" tries to show how the refspecs\n>> > expand out.\n>>\n>> I've created an \"early semantic conflict resolution\" topic branch that is\n>> a cross between this series and js/remote-improvements, like so:\n>>\n>>     $ git checkout -b xx/db-refspec-vs-js-remote db/refspec-wildcard-in-the-middle\n>>     $ git merge js/remote-improvements\n>>     $ git apply evil-fixup.diff\n>>     $ git commit --amend -a -m \"Evil merge.\"\n>>\n>> with the following \"fixup-as-an-evil-merge patch\", which I'd appreciate if\n>> you two can sanity check.\n>\n> That looks like what I'd come up with as a resolution, too, so I think\n> it's sane unless Jay knows of another way to get remote to care about\n> patterns.\n\nLooks good to me too.\n\nj.\n"}]}