{"thread":{"id":"36668","subject":"regression: request-pull with signed tag lacks tags/ in master","startedAt":"2014-05-15T16:39:01Z","lastAt":"2014-05-16T16:47:14Z","messageCount":6,"participants":["Michael S. Tsirkin","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"241702","messageId":"20140515163901.GA1403@redhat.com","threadId":"36668","inReplyTo":null,"subject":"regression: request-pull with signed tag lacks tags/ in master","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2014-05-15T16:39:01Z","receivedAt":"2014-05-15T16:39:01Z","isPatch":false,"sender":{"key":"mst@kernel.org","avatar":null},"body":"looks like pull requests with signed git got broken in git master:\n\n[mst@robin qemu]$ /usr/bin/git --version\ngit version 1.8.3.1\n[mst@robin qemu]$ git --version\ngit version 2.0.0.rc1.18.gac53fc6.dirty\n[mst@robin qemu]$ \n[mst@robin qemu]$ /usr/bin/git request-pull origin/master git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git for_upstream |grep git.kernel.org\n  git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream\n\n\n[mst@robin qemu]$ git request-pull origin/master git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git for_upstream |grep git.kernel.org\n  git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git for_upstream\n\nthis for_upstream syntax is a problem because it does not work\nfor older git clients who might get this request.\n\nDidn't bisect yet - anyone knows what broke this?\n\n-- \nMST\n"},{"id":"241762","messageId":"xmqqzjiiev1a.fsf@gitster.dls.corp.google.com","threadId":"36668","inReplyTo":"20140515163901.GA1403@redhat.com","subject":"Re: regression: request-pull with signed tag lacks tags/ in master","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-15T18:50:09Z","receivedAt":"2014-05-15T18:50:09Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Michael S. Tsirkin\" <mst@redhat.com> writes:\n\n> looks like pull requests with signed git got broken in git master:\n>\n> [mst@robin qemu]$ /usr/bin/git --version\n> git version 1.8.3.1\n> [mst@robin qemu]$ git --version\n> git version 2.0.0.rc1.18.gac53fc6.dirty\n> [mst@robin qemu]$ \n> [mst@robin qemu]$ /usr/bin/git request-pull origin/master git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git for_upstream |grep git.kernel.org\n>   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream\n>\n>\n> [mst@robin qemu]$ git request-pull origin/master git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git for_upstream |grep git.kernel.org\n>   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git for_upstream\n>\n> this for_upstream syntax is a problem because it does not work\n> for older git clients who might get this request.\n>\n> Didn't bisect yet - anyone knows what broke this?\n\nLinus ;-)  The series that ends with ec44507, I think.\n"},{"id":"241768","messageId":"xmqqr43uetyp.fsf@gitster.dls.corp.google.com","threadId":"36668","inReplyTo":"xmqqzjiiev1a.fsf@gitster.dls.corp.google.com","subject":"Re: regression: request-pull with signed tag lacks tags/ in master","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-15T19:13:18Z","receivedAt":"2014-05-15T19:13:18Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> \"Michael S. Tsirkin\" <mst@redhat.com> writes:\n>\n>> looks like pull requests with signed git got broken in git master:\n>>\n>> [mst@robin qemu]$ /usr/bin/git --version\n>> git version 1.8.3.1\n>> [mst@robin qemu]$ git --version\n>> git version 2.0.0.rc1.18.gac53fc6.dirty\n>> [mst@robin qemu]$ \n>> [mst@robin qemu]$ /usr/bin/git request-pull origin/master git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git for_upstream |grep git.kernel.org\n>>   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream\n>>\n>>\n>> [mst@robin qemu]$ git request-pull origin/master git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git for_upstream |grep git.kernel.org\n>>   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git for_upstream\n>>\n>> this for_upstream syntax is a problem because it does not work\n>> for older git clients who might get this request.\n>>\n>> Didn't bisect yet - anyone knows what broke this?\n>\n> Linus ;-)  The series that ends with ec44507, I think.\n\nMy reading of the earlier parts of the series is that Linus wanted\nus never dwim \"for-upstream\" to \"tags/for-upstream\" or any other ref\nthat happens to point at the same commit as for-upstream you have.\nThe changes done for that purpose covered various cases a bit too\nwidely, and \"request-pull ... tags/for_upstream\" were incorrectly\nstripped to a request to pull \"for_upstream\", which was fixed by\n5aae66bd (request-pull: resurrect \"pretty refname\" feature,\n2014-02-25).\n\nBut that fix does not resurrect the dwimming forbid by the earlier\nparts of the series to turn \"for_upstream\" into \"tags/for_upstream\".\n\nWhat would you get if you do this?\n\n    $ git request-pull origin/master \\\n      git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git \\\n      tags/for_upstream | grep git.kernel.org\n"},{"id":"241786","messageId":"20140515213925.GI1699@redhat.com","threadId":"36668","inReplyTo":"xmqqr43uetyp.fsf@gitster.dls.corp.google.com","subject":"Re: regression: request-pull with signed tag lacks tags/ in master","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2014-05-15T21:39:25Z","receivedAt":"2014-05-15T21:39:25Z","isPatch":false,"sender":{"key":"mst@kernel.org","avatar":null},"body":"On Thu, May 15, 2014 at 12:13:18PM -0700, Junio C Hamano wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n> > \"Michael S. Tsirkin\" <mst@redhat.com> writes:\n> >\n> >> looks like pull requests with signed git got broken in git master:\n> >>\n> >> [mst@robin qemu]$ /usr/bin/git --version\n> >> git version 1.8.3.1\n> >> [mst@robin qemu]$ git --version\n> >> git version 2.0.0.rc1.18.gac53fc6.dirty\n> >> [mst@robin qemu]$ \n> >> [mst@robin qemu]$ /usr/bin/git request-pull origin/master git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git for_upstream |grep git.kernel.org\n> >>   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream\n> >>\n> >>\n> >> [mst@robin qemu]$ git request-pull origin/master git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git for_upstream |grep git.kernel.org\n> >>   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git for_upstream\n> >>\n> >> this for_upstream syntax is a problem because it does not work\n> >> for older git clients who might get this request.\n> >>\n> >> Didn't bisect yet - anyone knows what broke this?\n> >\n> > Linus ;-)  The series that ends with ec44507, I think.\n> \n> My reading of the earlier parts of the series is that Linus wanted\n> us never dwim \"for-upstream\" to \"tags/for-upstream\" or any other ref\n> that happens to point at the same commit as for-upstream you have.\n> The changes done for that purpose covered various cases a bit too\n> widely, and \"request-pull ... tags/for_upstream\" were incorrectly\n> stripped to a request to pull \"for_upstream\", which was fixed by\n> 5aae66bd (request-pull: resurrect \"pretty refname\" feature,\n> 2014-02-25).\n> \n> But that fix does not resurrect the dwimming forbid by the earlier\n> parts of the series to turn \"for_upstream\" into \"tags/for_upstream\".\n> \n> What would you get if you do this?\n> \n>     $ git request-pull origin/master \\\n>       git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git \\\n>       tags/for_upstream | grep git.kernel.org\n\n\nI get\n git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream\n"},{"id":"241792","messageId":"xmqqtx8qd716.fsf@gitster.dls.corp.google.com","threadId":"36668","inReplyTo":"20140515213925.GI1699@redhat.com","subject":"Re: regression: request-pull with signed tag lacks tags/ in master","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-15T22:13:57Z","receivedAt":"2014-05-15T22:13:57Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Michael S. Tsirkin\" <mst@redhat.com> writes:\n\n>> My reading of the earlier parts of the series is that Linus wanted\n>> us never dwim \"for-upstream\" to \"tags/for-upstream\" or any other ref\n>> that happens to point at the same commit as for-upstream you have.\n>> The changes done for that purpose covered various cases a bit too\n>> widely, and \"request-pull ... tags/for_upstream\" were incorrectly\n>> stripped to a request to pull \"for_upstream\", which was fixed by\n>> 5aae66bd (request-pull: resurrect \"pretty refname\" feature,\n>> 2014-02-25).\n>> \n>> But that fix does not resurrect the dwimming forbid by the earlier\n>> parts of the series to turn \"for_upstream\" into \"tags/for_upstream\".\n>> \n>> What would you get if you do this?\n>> \n>>     $ git request-pull origin/master \\\n>>       git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git \\\n>>       tags/for_upstream | grep git.kernel.org\n>\n>\n> I get\n>  git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream\n\nThanks for double-checking.  Let's close this as working as\nintended, then.\n\nI personally feel that the \"intention\" tightened the logic a bit too\nmuch [*1*], and with the updates mentioned in [*2*], existing users\nmay find it still too tight, but that is what we have today.\n\n\n[References]\n\n*1* http://thread.gmane.org/gmane.comp.version-control.git/240860\n*2* http://thread.gmane.org/gmane.comp.version-control.git/240886\n"},{"id":"241992","messageId":"xmqqbnuxbrhp.fsf_-_@gitster.dls.corp.google.com","threadId":"36668","inReplyTo":"xmqqtx8qd716.fsf@gitster.dls.corp.google.com","subject":"Re* regression: request-pull with signed tag lacks tags/ in master","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-16T16:47:14Z","receivedAt":"2014-05-16T16:47:14Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> \"Michael S. Tsirkin\" <mst@redhat.com> writes:\n>\n>>> My reading of the earlier parts of the series is that Linus wanted\n>>> us never dwim \"for-upstream\" to \"tags/for-upstream\" or any other ref\n>>> that happens to point at the same commit as for-upstream you have.\n>>> The changes done for that purpose covered various cases a bit too\n>>> widely, and \"request-pull ... tags/for_upstream\" were incorrectly\n>>> stripped to a request to pull \"for_upstream\", which was fixed by\n>>> 5aae66bd (request-pull: resurrect \"pretty refname\" feature,\n>>> 2014-02-25).\n>>> \n>>> But that fix does not resurrect the dwimming forbid by the earlier\n>>> parts of the series to turn \"for_upstream\" into \"tags/for_upstream\".\n>>> \n>>> What would you get if you do this?\n>>> \n>>>     $ git request-pull origin/master \\\n>>>       git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git \\\n>>>       tags/for_upstream | grep git.kernel.org\n>>\n>>\n>> I get\n>>  git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream\n>\n> Thanks for double-checking.  Let's close this as working as\n> intended, then.\n>\n> I personally feel that the \"intention\" tightened the logic a bit too\n> much [*1*], and with the updates mentioned in [*2*], existing users\n> may find it still too tight, but that is what we have today.\n>\n>\n> [References]\n>\n> *1* http://thread.gmane.org/gmane.comp.version-control.git/240860\n> *2* http://thread.gmane.org/gmane.comp.version-control.git/240886\n\nAn update.\n\nIn the ideal world, I think it would be nice to make\n\n    $ git request-pull $mine $URL for_upstream\n\nexplicitly say \"Please pull tags/for_upstream\" rather than without\n\"tags/\" prefix to accomodate older Git, where\n\n    $ git pull $URL for_upstream\n\ndid not DWIM to fetch and merge tags/for_upstream and the user had\nto say \"tags/for_upstream\" instead.\n\nThat \"older Git\" refers to those without 47d84b6a (fetch: allow \"git\nfetch $there v1.0\" to fetch a tag, 2011-11-04).  v1.7.10 (tagged on\nApril 6th, 2012) and later versions of Git will notice that the name\nrefers to the tags/for_upstream just fine, so I think it is fair to\nlabel this as a minor cosmetic regression whose fix can wait for a\nfuture maintenance release, rather than a blocker for the upcoming\nrelease.\n\nI _think_ the fix, without breaking the spirit of Linus's \"I do not\nwant the thing DWIM based on what the remote end has\" original,\nwould be as simple as this patch.  We can queue it as a regression\nfix and do another round of -rc4 if those who depend on request-pull\nheavily feel strongly about it.\n\nComments?\n\n\n git-request-pull.sh | 6 ++++++\n 1 file changed, 6 insertions(+)\n\ndiff --git a/git-request-pull.sh b/git-request-pull.sh\nindex 5c15997..d5500fd 100755\n--- a/git-request-pull.sh\n+++ b/git-request-pull.sh\n@@ -119,6 +119,12 @@ then\n \tstatus=1\n fi\n \n+# Special case: turn \"for_linus\" to \"tags/for_linus\" when it is correct\n+if test \"$ref\" = \"refs/tags/$pretty_remote\"\n+then\n+\tpretty_remote=tags/$pretty_remote\n+fi\n+\n url=$(git ls-remote --get-url \"$url\")\n \n git show -s --format='The following changes since commit %H:\n"}]}