[PATCH] remote-hg: getbundle changed in mercurial 3.0

[PATCH] remote-hg: getbundle changed in mercurial 3.0

[William Giokas]

In mercurial 3.0, getbundle was moved to the changegroup module, and
gained a new argument. Due to this we cannot simply start using
getbundle(...) imported from either one unconditionally, as that would
cause errors in mercurial 3.0 without changing the syntax, and errors in
mercurial <3.0 if we do change it.

The try:except block at the beginning of git-remote-hg.py tries first to
import mercurial.changegroup.getbundle, and if that fails we set the
function 'getbundle' to work correctly with mercurial.repo.getbundle by
removing the first argument.

Signed-off-by: William Giokas <1007380@gmail.com>
---

I have tested this briefly with mercurial 3.0, but have not yet really run
through its paces. The tests that are included in next do pass with
mercurial 3.0.

 git-remote-hg.py | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/git-remote-hg.py b/git-remote-hg.py
index 34cda02..3dc9e11 100755
--- a/git-remote-hg.py
+++ b/git-remote-hg.py
@@ -14,6 +14,13 @@
 
 from mercurial import hg, ui, bookmarks, context, encoding, node, error, extensions, discovery, util
 
+try:
+    from mercurial.changegroup import getbundle
+
+except ImportError:
+    def getbundle(__empty__, **kwargs):
+        return repo.getbundle(**kwargs)
+
 import re
 import sys
 import os
@@ -985,7 +992,7 @@ def push_unsafe(repo, remote, parsed_refs, p_revs):
     if not checkheads(repo, remote, p_revs):
         return None
 
-    cg = repo.getbundle('push', heads=list(p_revs), common=common)
+    cg = getbundle(repo, 'push', heads=list(p_revs), common=common)
 
     unbundle = remote.capable('unbundle')
     if unbundle:
-- 
2.0.0.rc3

[PATCH v2] remote-hg: getbundle changed in mercurial 3.0

[William Giokas]

In mercurial 3.0, getbundle was moved to the changegroup module, and
gained a new argument. Due to this we cannot simply start using
getbundle(...) imported from either one unconditionally, as that would
cause errors in mercurial 3.0 without changing the syntax, and errors in
mercurial <3.0 if we do change it.

The try:except block at the beginning of git-remote-hg.py tries first to
import mercurial.changegroup.getbundle, and if that fails we set the
function 'getbundle' to work correctly with mercurial.repo.getbundle by
removing the first argument.

Signed-off-by: William Giokas <1007380@gmail.com>
---

So, what I had in there before would not work at all on repo.getbundle
because **kwargs only unpacks keyword args, not normal args. Sometimes my
brain works.

 git-remote-hg.py | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/git-remote-hg.py b/git-remote-hg.py
index 34cda02..32eeffb 100755
--- a/git-remote-hg.py
+++ b/git-remote-hg.py
@@ -14,6 +14,13 @@
 
 from mercurial import hg, ui, bookmarks, context, encoding, node, error, extensions, discovery, util
 
+try:
+    from mercurial.changegroup import getbundle
+
+except ImportError:
+    def getbundle(repo, **kwargs):
+        return repo.getbundle(**kwargs)
+
 import re
 import sys
 import os
@@ -985,7 +992,8 @@ def push_unsafe(repo, remote, parsed_refs, p_revs):
     if not checkheads(repo, remote, p_revs):
         return None
 
-    cg = repo.getbundle('push', heads=list(p_revs), common=common)
+    cg = getbundle(repo=repo, source='push', heads=list(p_revs),
+                   common=common)
 
     unbundle = remote.capable('unbundle')
     if unbundle:
-- 
2.0.0.rc3

Re: [PATCH] remote-hg: getbundle changed in mercurial 3.0

[Junio C Hamano]

William Giokas <1007380@gmail.com> writes:

> In mercurial 3.0, getbundle was moved to the changegroup module, and
> gained a new argument. Due to this we cannot simply start using
> getbundle(...) imported from either one unconditionally, as that would
> cause errors in mercurial 3.0 without changing the syntax, and errors in
> mercurial <3.0 if we do change it.
>
> The try:except block at the beginning of git-remote-hg.py tries first to
> import mercurial.changegroup.getbundle, and if that fails we set the
> function 'getbundle' to work correctly with mercurial.repo.getbundle by
> removing the first argument.
>
> Signed-off-by: William Giokas <1007380@gmail.com>
> ---
>
> I have tested this briefly with mercurial 3.0, but have not yet really run
> through its paces. The tests that are included in next do pass with
> mercurial 3.0.
>
>  git-remote-hg.py | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/git-remote-hg.py b/git-remote-hg.py
> index 34cda02..3dc9e11 100755
> --- a/git-remote-hg.py
> +++ b/git-remote-hg.py
> @@ -14,6 +14,13 @@
>  
>  from mercurial import hg, ui, bookmarks, context, encoding, node, error, extensions, discovery, util
>  
> +try:
> +    from mercurial.changegroup import getbundle
> +
> +except ImportError:
> +    def getbundle(__empty__, **kwargs):
> +        return repo.getbundle(**kwargs)
> +
>  import re
>  import sys
>  import os
> @@ -985,7 +992,7 @@ def push_unsafe(repo, remote, parsed_refs, p_revs):
>      if not checkheads(repo, remote, p_revs):
>          return None
>  
> -    cg = repo.getbundle('push', heads=list(p_revs), common=common)
> +    cg = getbundle(repo, 'push', heads=list(p_revs), common=common)
>  
>      unbundle = remote.capable('unbundle')
>      if unbundle:

Thanks.  I agree with you that this would probably be a better
way to future-proof the code without unconditionally including what
may not be used at all, as you said in the other thread.

I can be pursuaded to queue this particular patch for maintenance
tracks after 2.0 final to my tree, but I do not think I would want
to keep taking patches to this area myself in the longer run.

The way I envision the longer term shape of git-remote-hg.py in the
contrib/ is either one of these two:

 (1) manage it just like contrib/hooks/multimail/, keeping a
     reasonably fresh and known-to-be-good snapshot, while making it
     clear that my tree is not the authoritative copy people should
     work off of;

 (2) treat it just like contrib/emacs/vc-git.el, which found a
     better home and left my tree at 78513869 (emacs: Remove the no
     longer maintained vc-git package., 2009-02-07); or

The first one may be more preferrable between the two, if only
because distros would need time to adjust where they pick up the
source material to package up, but it still needs cooperation with
the "authoritative upstream" and this project to allow us to at
least learn when the good time to import such good snapshots.  In
the case of vc-git, the separation was not about lack of will to
coordinate, but because it was not necessary to keep it in my tree,
as the "better home" was where the users expect to find the latest
and greatest anyway.

If Felipe turns out to be a maintainer users do not trust for
remote-hg, it is possible that you and other people can maintain it
and work with git.git better by forking off of his tree.  It is far
early to know if that will be the case at this point, and I
personally do not think that will happen (no, I do not have a
concrete reason I can cite to explain why I think that way).

But even in such an extreme hypothetical case, the difference it
makes to my tree would only be to take (1) or (2) and point to that
forked remote-hg repository, instead of Felipe's, as the source of
the "authoritative copy".  And I wouldn't be taking individual
patches directly to my tree in either case.

Re: [PATCH] remote-hg: getbundle changed in mercurial 3.0

[Felipe Contreras]

Junio C Hamano wrote:

> The way I envision the longer term shape of git-remote-hg.py in the
> contrib/ is either one of these two:
> 
>  (1) manage it just like contrib/hooks/multimail/, keeping a
>      reasonably fresh and known-to-be-good snapshot, while making it
>      clear that my tree is not the authoritative copy people should
>      work off of;
> 
>  (2) treat it just like contrib/emacs/vc-git.el, which found a
>      better home and left my tree at 78513869 (emacs: Remove the no
>      longer maintained vc-git package., 2009-02-07); or
> 
> The first one may be more preferrable between the two, if only because
> distros would need time to adjust where they pick up the source
> material to package up, but it still needs cooperation with the
> "authoritative upstream" and this project to allow us to at least
> learn when the good time to import such good snapshots.

I will not do that.

If you want my help to improve *your* tree, you have to start by
answering the *one* question I've repeatedly asked you to clarify.

In fact if you go for this I would consider it an act of sabotage
against these new projects.

If you hadn't made me waste all this time chasing a non-attainable goal,
these projects would already be packaged by distributions, instead of
hidden in some corner of /usr/share.

Distributions wouldn't even be aware of the move, and it might take bug
reports to make them realize that. There will be already enough damage
to the reputation of these tools with Git v2.0 shipping them broken.

Not aligned at all with your previous statement that you wanted these to
"flourisn".

In fact, I think you should remove them already from v2.0. Because this
doesn't need release candidates. Unless you think user feedback will
make you change your mind about not graduating them, moving them in 2.0,
or 2.1 will be the same, since you are going to ignore the feedback
anyway.

This also has the advantage that you won't be shipping them broken in
v2.0.

At the very least there should be a big fat message warning each time
the tools are run, warning the users that they are unmaintained, and the
right location for them. And yes, I also think this should be done for
v2.0.

-- 
Felipe Contreras

Re: [PATCH v2] remote-hg: getbundle changed in mercurial 3.0

[Junio C Hamano]

William Giokas <1007380@gmail.com> writes:

> +try:
> +    from mercurial.changegroup import getbundle
> +
> +except ImportError:
> +    def getbundle(repo, **kwargs):
> +        return repo.getbundle(**kwargs)
> +
>  import re
>  import sys
>  import os
> @@ -985,7 +992,8 @@ def push_unsafe(repo, remote, parsed_refs, p_revs):
>      if not checkheads(repo, remote, p_revs):
>          return None
>  
> -    cg = repo.getbundle('push', heads=list(p_revs), common=common)
> +    cg = getbundle(repo=repo, source='push', heads=list(p_revs),
> +                   common=common)

Yikes, this is starting to look bad, but the thing being in Python,
perhaps that is the best we could do if we want a solution that is
viable in the longer term.

As a short-term band-aid to be merged/cherry-picked to maintenance
tracks post 2.0 final, I actually would prefer 58aee086 (remote-hg:
add support for hg v3.0, 2014-05-03) for its simplicity.

I dunno.  Thankfully I do not have to decide right now ;-)