{"thread":{"id":"36650","subject":"[PATCH] remote-hg: getbundle changed in mercurial 3.0","startedAt":"2014-05-13T19:12:39Z","lastAt":"2014-05-13T21:00:18Z","messageCount":5,"participants":["William Giokas","Junio C Hamano","Felipe Contreras"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"241391","messageId":"1400008359-18267-1-git-send-email-1007380@gmail.com","threadId":"36650","inReplyTo":null,"subject":"[PATCH] remote-hg: getbundle changed in mercurial 3.0","fromName":"William Giokas","fromEmail":"1007380@gmail.com","sentAt":"2014-05-13T19:12:39Z","receivedAt":"2014-05-13T19:12:39Z","isPatch":true,"sender":{"key":"1007380@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1731430?v=4"},"body":"In mercurial 3.0, getbundle was moved to the changegroup module, and\ngained a new argument. Due to this we cannot simply start using\ngetbundle(...) imported from either one unconditionally, as that would\ncause errors in mercurial 3.0 without changing the syntax, and errors in\nmercurial <3.0 if we do change it.\n\nThe try:except block at the beginning of git-remote-hg.py tries first to\nimport mercurial.changegroup.getbundle, and if that fails we set the\nfunction 'getbundle' to work correctly with mercurial.repo.getbundle by\nremoving the first argument.\n\nSigned-off-by: William Giokas <1007380@gmail.com>\n---\n\nI have tested this briefly with mercurial 3.0, but have not yet really run\nthrough its paces. The tests that are included in next do pass with\nmercurial 3.0.\n\n git-remote-hg.py | 9 ++++++++-\n 1 file changed, 8 insertions(+), 1 deletion(-)\n\ndiff --git a/git-remote-hg.py b/git-remote-hg.py\nindex 34cda02..3dc9e11 100755\n--- a/git-remote-hg.py\n+++ b/git-remote-hg.py\n@@ -14,6 +14,13 @@\n \n from mercurial import hg, ui, bookmarks, context, encoding, node, error, extensions, discovery, util\n \n+try:\n+    from mercurial.changegroup import getbundle\n+\n+except ImportError:\n+    def getbundle(__empty__, **kwargs):\n+        return repo.getbundle(**kwargs)\n+\n import re\n import sys\n import os\n@@ -985,7 +992,7 @@ def push_unsafe(repo, remote, parsed_refs, p_revs):\n     if not checkheads(repo, remote, p_revs):\n         return None\n \n-    cg = repo.getbundle('push', heads=list(p_revs), common=common)\n+    cg = getbundle(repo, 'push', heads=list(p_revs), common=common)\n \n     unbundle = remote.capable('unbundle')\n     if unbundle:\n-- \n2.0.0.rc3\n"},{"id":"241398","messageId":"1400012506-20705-1-git-send-email-1007380@gmail.com","threadId":"36650","inReplyTo":"1400008359-18267-1-git-send-email-1007380@gmail.com","subject":"[PATCH v2] remote-hg: getbundle changed in mercurial 3.0","fromName":"William Giokas","fromEmail":"1007380@gmail.com","sentAt":"2014-05-13T20:21:46Z","receivedAt":"2014-05-13T20:21:46Z","isPatch":true,"sender":{"key":"1007380@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1731430?v=4"},"body":"In mercurial 3.0, getbundle was moved to the changegroup module, and\ngained a new argument. Due to this we cannot simply start using\ngetbundle(...) imported from either one unconditionally, as that would\ncause errors in mercurial 3.0 without changing the syntax, and errors in\nmercurial <3.0 if we do change it.\n\nThe try:except block at the beginning of git-remote-hg.py tries first to\nimport mercurial.changegroup.getbundle, and if that fails we set the\nfunction 'getbundle' to work correctly with mercurial.repo.getbundle by\nremoving the first argument.\n\nSigned-off-by: William Giokas <1007380@gmail.com>\n---\n\nSo, what I had in there before would not work at all on repo.getbundle\nbecause **kwargs only unpacks keyword args, not normal args. Sometimes my\nbrain works.\n\n git-remote-hg.py | 10 +++++++++-\n 1 file changed, 9 insertions(+), 1 deletion(-)\n\ndiff --git a/git-remote-hg.py b/git-remote-hg.py\nindex 34cda02..32eeffb 100755\n--- a/git-remote-hg.py\n+++ b/git-remote-hg.py\n@@ -14,6 +14,13 @@\n \n from mercurial import hg, ui, bookmarks, context, encoding, node, error, extensions, discovery, util\n \n+try:\n+    from mercurial.changegroup import getbundle\n+\n+except ImportError:\n+    def getbundle(repo, **kwargs):\n+        return repo.getbundle(**kwargs)\n+\n import re\n import sys\n import os\n@@ -985,7 +992,8 @@ def push_unsafe(repo, remote, parsed_refs, p_revs):\n     if not checkheads(repo, remote, p_revs):\n         return None\n \n-    cg = repo.getbundle('push', heads=list(p_revs), common=common)\n+    cg = getbundle(repo=repo, source='push', heads=list(p_revs),\n+                   common=common)\n \n     unbundle = remote.capable('unbundle')\n     if unbundle:\n-- \n2.0.0.rc3\n"},{"id":"241407","messageId":"xmqqzjilmnad.fsf@gitster.dls.corp.google.com","threadId":"36650","inReplyTo":"1400008359-18267-1-git-send-email-1007380@gmail.com","subject":"Re: [PATCH] remote-hg: getbundle changed in mercurial 3.0","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-13T20:33:30Z","receivedAt":"2014-05-13T20:33:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"William Giokas <1007380@gmail.com> writes:\n\n> In mercurial 3.0, getbundle was moved to the changegroup module, and\n> gained a new argument. Due to this we cannot simply start using\n> getbundle(...) imported from either one unconditionally, as that would\n> cause errors in mercurial 3.0 without changing the syntax, and errors in\n> mercurial <3.0 if we do change it.\n>\n> The try:except block at the beginning of git-remote-hg.py tries first to\n> import mercurial.changegroup.getbundle, and if that fails we set the\n> function 'getbundle' to work correctly with mercurial.repo.getbundle by\n> removing the first argument.\n>\n> Signed-off-by: William Giokas <1007380@gmail.com>\n> ---\n>\n> I have tested this briefly with mercurial 3.0, but have not yet really run\n> through its paces. The tests that are included in next do pass with\n> mercurial 3.0.\n>\n>  git-remote-hg.py | 9 ++++++++-\n>  1 file changed, 8 insertions(+), 1 deletion(-)\n>\n> diff --git a/git-remote-hg.py b/git-remote-hg.py\n> index 34cda02..3dc9e11 100755\n> --- a/git-remote-hg.py\n> +++ b/git-remote-hg.py\n> @@ -14,6 +14,13 @@\n>  \n>  from mercurial import hg, ui, bookmarks, context, encoding, node, error, extensions, discovery, util\n>  \n> +try:\n> +    from mercurial.changegroup import getbundle\n> +\n> +except ImportError:\n> +    def getbundle(__empty__, **kwargs):\n> +        return repo.getbundle(**kwargs)\n> +\n>  import re\n>  import sys\n>  import os\n> @@ -985,7 +992,7 @@ def push_unsafe(repo, remote, parsed_refs, p_revs):\n>      if not checkheads(repo, remote, p_revs):\n>          return None\n>  \n> -    cg = repo.getbundle('push', heads=list(p_revs), common=common)\n> +    cg = getbundle(repo, 'push', heads=list(p_revs), common=common)\n>  \n>      unbundle = remote.capable('unbundle')\n>      if unbundle:\n\nThanks.  I agree with you that this would probably be a better\nway to future-proof the code without unconditionally including what\nmay not be used at all, as you said in the other thread.\n\nI can be pursuaded to queue this particular patch for maintenance\ntracks after 2.0 final to my tree, but I do not think I would want\nto keep taking patches to this area myself in the longer run.\n\nThe way I envision the longer term shape of git-remote-hg.py in the\ncontrib/ is either one of these two:\n\n (1) manage it just like contrib/hooks/multimail/, keeping a\n     reasonably fresh and known-to-be-good snapshot, while making it\n     clear that my tree is not the authoritative copy people should\n     work off of;\n\n (2) treat it just like contrib/emacs/vc-git.el, which found a\n     better home and left my tree at 78513869 (emacs: Remove the no\n     longer maintained vc-git package., 2009-02-07); or\n\nThe first one may be more preferrable between the two, if only\nbecause distros would need time to adjust where they pick up the\nsource material to package up, but it still needs cooperation with\nthe \"authoritative upstream\" and this project to allow us to at\nleast learn when the good time to import such good snapshots.  In\nthe case of vc-git, the separation was not about lack of will to\ncoordinate, but because it was not necessary to keep it in my tree,\nas the \"better home\" was where the users expect to find the latest\nand greatest anyway.\n\nIf Felipe turns out to be a maintainer users do not trust for\nremote-hg, it is possible that you and other people can maintain it\nand work with git.git better by forking off of his tree.  It is far\nearly to know if that will be the case at this point, and I\npersonally do not think that will happen (no, I do not have a\nconcrete reason I can cite to explain why I think that way).\n\nBut even in such an extreme hypothetical case, the difference it\nmakes to my tree would only be to take (1) or (2) and point to that\nforked remote-hg repository, instead of Felipe's, as the source of\nthe \"authoritative copy\".  And I wouldn't be taking individual\npatches directly to my tree in either case.\n"},{"id":"241440","messageId":"537287cba3013_76dad6b2f8dd@nysa.notmuch","threadId":"36650","inReplyTo":"xmqqzjilmnad.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] remote-hg: getbundle changed in mercurial 3.0","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2014-05-13T20:59:55Z","receivedAt":"2014-05-13T20:59:55Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Junio C Hamano wrote:\n\n> The way I envision the longer term shape of git-remote-hg.py in the\n> contrib/ is either one of these two:\n> \n>  (1) manage it just like contrib/hooks/multimail/, keeping a\n>      reasonably fresh and known-to-be-good snapshot, while making it\n>      clear that my tree is not the authoritative copy people should\n>      work off of;\n> \n>  (2) treat it just like contrib/emacs/vc-git.el, which found a\n>      better home and left my tree at 78513869 (emacs: Remove the no\n>      longer maintained vc-git package., 2009-02-07); or\n> \n> The first one may be more preferrable between the two, if only because\n> distros would need time to adjust where they pick up the source\n> material to package up, but it still needs cooperation with the\n> \"authoritative upstream\" and this project to allow us to at least\n> learn when the good time to import such good snapshots.\n\nI will not do that.\n\nIf you want my help to improve *your* tree, you have to start by\nanswering the *one* question I've repeatedly asked you to clarify.\n\nIn fact if you go for this I would consider it an act of sabotage\nagainst these new projects.\n\nIf you hadn't made me waste all this time chasing a non-attainable goal,\nthese projects would already be packaged by distributions, instead of\nhidden in some corner of /usr/share.\n\nDistributions wouldn't even be aware of the move, and it might take bug\nreports to make them realize that. There will be already enough damage\nto the reputation of these tools with Git v2.0 shipping them broken.\n\nNot aligned at all with your previous statement that you wanted these to\n\"flourisn\".\n\nIn fact, I think you should remove them already from v2.0. Because this\ndoesn't need release candidates. Unless you think user feedback will\nmake you change your mind about not graduating them, moving them in 2.0,\nor 2.1 will be the same, since you are going to ignore the feedback\nanyway.\n\nThis also has the advantage that you won't be shipping them broken in\nv2.0.\n\nAt the very least there should be a big fat message warning each time\nthe tools are run, warning the users that they are unmaintained, and the\nright location for them. And yes, I also think this should be done for\nv2.0.\n\n-- \nFelipe Contreras\n"},{"id":"241436","messageId":"xmqqmwelmm1p.fsf@gitster.dls.corp.google.com","threadId":"36650","inReplyTo":"1400012506-20705-1-git-send-email-1007380@gmail.com","subject":"Re: [PATCH v2] remote-hg: getbundle changed in mercurial 3.0","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-13T21:00:18Z","receivedAt":"2014-05-13T21:00:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"William Giokas <1007380@gmail.com> writes:\n\n> +try:\n> +    from mercurial.changegroup import getbundle\n> +\n> +except ImportError:\n> +    def getbundle(repo, **kwargs):\n> +        return repo.getbundle(**kwargs)\n> +\n>  import re\n>  import sys\n>  import os\n> @@ -985,7 +992,8 @@ def push_unsafe(repo, remote, parsed_refs, p_revs):\n>      if not checkheads(repo, remote, p_revs):\n>          return None\n>  \n> -    cg = repo.getbundle('push', heads=list(p_revs), common=common)\n> +    cg = getbundle(repo=repo, source='push', heads=list(p_revs),\n> +                   common=common)\n\nYikes, this is starting to look bad, but the thing being in Python,\nperhaps that is the best we could do if we want a solution that is\nviable in the longer term.\n\nAs a short-term band-aid to be merged/cherry-picked to maintenance\ntracks post 2.0 final, I actually would prefer 58aee086 (remote-hg:\nadd support for hg v3.0, 2014-05-03) for its simplicity.\n\nI dunno.  Thankfully I do not have to decide right now ;-)\n"}]}