Re* regression: request-pull with signed tag lacks tags/ in master
Junio C Hamano gits...@pobox.com writes: Michael S. Tsirkin m...@redhat.com writes: My reading of the earlier parts of the series is that Linus wanted us never dwim for-upstream to tags/for-upstream or any other ref that happens to point at the same commit as for-upstream you have. The changes done for that purpose covered various cases a bit too widely, and request-pull ... tags/for_upstream were incorrectly stripped to a request to pull for_upstream, which was fixed by 5aae66bd (request-pull: resurrect pretty refname feature, 2014-02-25). But that fix does not resurrect the dwimming forbid by the earlier parts of the series to turn for_upstream into tags/for_upstream. What would you get if you do this? $ git request-pull origin/master \ git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git \ tags/for_upstream | grep git.kernel.org I get git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream Thanks for double-checking. Let's close this as working as intended, then. I personally feel that the intention tightened the logic a bit too much [*1*], and with the updates mentioned in [*2*], existing users may find it still too tight, but that is what we have today. [References] *1* http://thread.gmane.org/gmane.comp.version-control.git/240860 *2* http://thread.gmane.org/gmane.comp.version-control.git/240886 An update. In the ideal world, I think it would be nice to make $ git request-pull $mine $URL for_upstream explicitly say Please pull tags/for_upstream rather than without tags/ prefix to accomodate older Git, where $ git pull $URL for_upstream did not DWIM to fetch and merge tags/for_upstream and the user had to say tags/for_upstream instead. That older Git refers to those without 47d84b6a (fetch: allow git fetch $there v1.0 to fetch a tag, 2011-11-04). v1.7.10 (tagged on April 6th, 2012) and later versions of Git will notice that the name refers to the tags/for_upstream just fine, so I think it is fair to label this as a minor cosmetic regression whose fix can wait for a future maintenance release, rather than a blocker for the upcoming release. I _think_ the fix, without breaking the spirit of Linus's I do not want the thing DWIM based on what the remote end has original, would be as simple as this patch. We can queue it as a regression fix and do another round of -rc4 if those who depend on request-pull heavily feel strongly about it. Comments? git-request-pull.sh | 6 ++ 1 file changed, 6 insertions(+) diff --git a/git-request-pull.sh b/git-request-pull.sh index 5c15997..d5500fd 100755 --- a/git-request-pull.sh +++ b/git-request-pull.sh @@ -119,6 +119,12 @@ then status=1 fi +# Special case: turn for_linus to tags/for_linus when it is correct +if test $ref = refs/tags/$pretty_remote +then + pretty_remote=tags/$pretty_remote +fi + url=$(git ls-remote --get-url $url) git show -s --format='The following changes since commit %H: -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: regression: request-pull with signed tag lacks tags/ in master
Michael S. Tsirkin m...@redhat.com writes: looks like pull requests with signed git got broken in git master: [mst@robin qemu]$ /usr/bin/git --version git version 1.8.3.1 [mst@robin qemu]$ git --version git version 2.0.0.rc1.18.gac53fc6.dirty [mst@robin qemu]$ [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 git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream [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 git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git for_upstream this for_upstream syntax is a problem because it does not work for older git clients who might get this request. Didn't bisect yet - anyone knows what broke this? Linus ;-) The series that ends with ec44507, I think. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: regression: request-pull with signed tag lacks tags/ in master
Junio C Hamano gits...@pobox.com writes: Michael S. Tsirkin m...@redhat.com writes: looks like pull requests with signed git got broken in git master: [mst@robin qemu]$ /usr/bin/git --version git version 1.8.3.1 [mst@robin qemu]$ git --version git version 2.0.0.rc1.18.gac53fc6.dirty [mst@robin qemu]$ [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 git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream [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 git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git for_upstream this for_upstream syntax is a problem because it does not work for older git clients who might get this request. Didn't bisect yet - anyone knows what broke this? Linus ;-) The series that ends with ec44507, I think. My reading of the earlier parts of the series is that Linus wanted us never dwim for-upstream to tags/for-upstream or any other ref that happens to point at the same commit as for-upstream you have. The changes done for that purpose covered various cases a bit too widely, and request-pull ... tags/for_upstream were incorrectly stripped to a request to pull for_upstream, which was fixed by 5aae66bd (request-pull: resurrect pretty refname feature, 2014-02-25). But that fix does not resurrect the dwimming forbid by the earlier parts of the series to turn for_upstream into tags/for_upstream. What would you get if you do this? $ git request-pull origin/master \ git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git \ tags/for_upstream | grep git.kernel.org -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: regression: request-pull with signed tag lacks tags/ in master
On Thu, May 15, 2014 at 12:13:18PM -0700, Junio C Hamano wrote: Junio C Hamano gits...@pobox.com writes: Michael S. Tsirkin m...@redhat.com writes: looks like pull requests with signed git got broken in git master: [mst@robin qemu]$ /usr/bin/git --version git version 1.8.3.1 [mst@robin qemu]$ git --version git version 2.0.0.rc1.18.gac53fc6.dirty [mst@robin qemu]$ [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 git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream [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 git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git for_upstream this for_upstream syntax is a problem because it does not work for older git clients who might get this request. Didn't bisect yet - anyone knows what broke this? Linus ;-) The series that ends with ec44507, I think. My reading of the earlier parts of the series is that Linus wanted us never dwim for-upstream to tags/for-upstream or any other ref that happens to point at the same commit as for-upstream you have. The changes done for that purpose covered various cases a bit too widely, and request-pull ... tags/for_upstream were incorrectly stripped to a request to pull for_upstream, which was fixed by 5aae66bd (request-pull: resurrect pretty refname feature, 2014-02-25). But that fix does not resurrect the dwimming forbid by the earlier parts of the series to turn for_upstream into tags/for_upstream. What would you get if you do this? $ git request-pull origin/master \ git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git \ tags/for_upstream | grep git.kernel.org I get git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: regression: request-pull with signed tag lacks tags/ in master
Michael S. Tsirkin m...@redhat.com writes: My reading of the earlier parts of the series is that Linus wanted us never dwim for-upstream to tags/for-upstream or any other ref that happens to point at the same commit as for-upstream you have. The changes done for that purpose covered various cases a bit too widely, and request-pull ... tags/for_upstream were incorrectly stripped to a request to pull for_upstream, which was fixed by 5aae66bd (request-pull: resurrect pretty refname feature, 2014-02-25). But that fix does not resurrect the dwimming forbid by the earlier parts of the series to turn for_upstream into tags/for_upstream. What would you get if you do this? $ git request-pull origin/master \ git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git \ tags/for_upstream | grep git.kernel.org I get git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream Thanks for double-checking. Let's close this as working as intended, then. I personally feel that the intention tightened the logic a bit too much [*1*], and with the updates mentioned in [*2*], existing users may find it still too tight, but that is what we have today. [References] *1* http://thread.gmane.org/gmane.comp.version-control.git/240860 *2* http://thread.gmane.org/gmane.comp.version-control.git/240886 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
