Patchwork histedit: prevent parent guessed via --outgoing from being a revset (issue3770)

login
register
mail settings
Submitter Augie Fackler
Date Jan. 30, 2013, 3:58 p.m.
Message ID <d2de19a50f101734629c.1359561486@arthedain.pit.corp.google.com>
Download mbox | patch
Permalink /patch/762/
State Accepted
Commit c795c9f87792fef98b358ae88f90c4003e9bbe4d
Headers show

Comments

Augie Fackler - Jan. 30, 2013, 3:58 p.m.
# HG changeset patch
# User Augie Fackler <raf@durin42.com>
# Date 1359561448 18000
# Branch stable
# Node ID d2de19a50f101734629c8c4d05b91780c0d7fe13
# Parent  692cbda1eb50fe30c70792cb1e9380b28769467c
histedit: prevent parent guessed via --outgoing from being a revset (issue3770)

If the binary hash of the parent node guessed via --outgoing happened
to contain a special revset character (":" was specified in the bug),
the revset parser would abort. Hexlifying the node before passing it
to the revsingle call should fix that.
Augie Fackler - Jan. 30, 2013, 4 p.m.
This /feels/ like it'd be okay for the freeze, but then again it's not
a regression. I'm happy to push it myself post-freeze if that's the
right thing to do.

On Wed, Jan 30, 2013 at 10:58 AM, Augie Fackler <raf@durin42.com> wrote:
> # HG changeset patch
> # User Augie Fackler <raf@durin42.com>
> # Date 1359561448 18000
> # Branch stable
> # Node ID d2de19a50f101734629c8c4d05b91780c0d7fe13
> # Parent  692cbda1eb50fe30c70792cb1e9380b28769467c
> histedit: prevent parent guessed via --outgoing from being a revset (issue3770)
>
> If the binary hash of the parent node guessed via --outgoing happened
> to contain a special revset character (":" was specified in the bug),
> the revset parser would abort. Hexlifying the node before passing it
> to the revsingle call should fix that.
>
> diff --git a/hgext/histedit.py b/hgext/histedit.py
> --- a/hgext/histedit.py
> +++ b/hgext/histedit.py
> @@ -454,8 +454,12 @@
>          if revs:
>              revs = [repo.lookup(rev) for rev in revs]
>
> -        parent = discovery.findcommonoutgoing(
> -            repo, other, [], force=opts.get('force')).missing[0:1]
> +        # hexlify nodes from outgoing, because we're going to parse
> +        # parent[0] using revsingle below, and if the binary hash
> +        # contains special revset characters like ":" the revset
> +        # parser can choke.
> +        parent = map(node.hex, discovery.findcommonoutgoing(
> +            repo, other, [], force=opts.get('force')).missing[0:1])
>      else:
>          if opts.get('force'):
>              raise util.Abort(_('--force only allowed with --outgoing'))
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Pierre-Yves David - Jan. 30, 2013, 4:01 p.m.
On Wed, Jan 30, 2013 at 10:58:06AM -0500, Augie Fackler wrote:
> # HG changeset patch
> # User Augie Fackler <raf@durin42.com>
> # Date 1359561448 18000
> # Branch stable
> # Node ID d2de19a50f101734629c8c4d05b91780c0d7fe13
> # Parent  692cbda1eb50fe30c70792cb1e9380b28769467c
> histedit: prevent parent guessed via --outgoing from being a revset (issue3770)
> 
> If the binary hash of the parent node guessed via --outgoing happened
> to contain a special revset character (":" was specified in the bug),
> the revset parser would abort. Hexlifying the node before passing it
> to the revsingle call should fix that.

Why don't you just give a 'min(outgoing())' revset to singlerev and give up
your manual outgoing call?

> 
> diff --git a/hgext/histedit.py b/hgext/histedit.py
> --- a/hgext/histedit.py
> +++ b/hgext/histedit.py
> @@ -454,8 +454,12 @@
>          if revs:
>              revs = [repo.lookup(rev) for rev in revs]
>  
> -        parent = discovery.findcommonoutgoing(
> -            repo, other, [], force=opts.get('force')).missing[0:1]
> +        # hexlify nodes from outgoing, because we're going to parse
> +        # parent[0] using revsingle below, and if the binary hash
> +        # contains special revset characters like ":" the revset
> +        # parser can choke.
> +        parent = map(node.hex, discovery.findcommonoutgoing(
> +            repo, other, [], force=opts.get('force')).missing[0:1])
>      else:
>          if opts.get('force'):
>              raise util.Abort(_('--force only allowed with --outgoing'))
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Augie Fackler - Jan. 30, 2013, 4:15 p.m.
On Wed, Jan 30, 2013 at 11:01 AM, Pierre-Yves David
<pierre-yves.david@logilab.fr> wrote:
> Why don't you just give a 'min(outgoing())' revset to singlerev and give up
> your manual outgoing call?


Paranoia, trying to have a small delta.
Matt Mackall - Jan. 30, 2013, 8:25 p.m.
On Wed, 2013-01-30 at 10:58 -0500, Augie Fackler wrote:
> # HG changeset patch
> # User Augie Fackler <raf@durin42.com>
> # Date 1359561448 18000
> # Branch stable
> # Node ID d2de19a50f101734629c8c4d05b91780c0d7fe13
> # Parent  692cbda1eb50fe30c70792cb1e9380b28769467c
> histedit: prevent parent guessed via --outgoing from being a revset (issue3770)
> 
> If the binary hash of the parent node guessed via --outgoing happened
> to contain a special revset character (":" was specified in the bug),
> the revset parser would abort. Hexlifying the node before passing it
> to the revsingle call should fix that.

This looks fine. We generally prefer list comprehensions to map these
days, but post-2.5, the outgoing() revset approach is probably the
answer so we can put this in as-is.

> diff --git a/hgext/histedit.py b/hgext/histedit.py
> --- a/hgext/histedit.py
> +++ b/hgext/histedit.py
> @@ -454,8 +454,12 @@
>          if revs:
>              revs = [repo.lookup(rev) for rev in revs]
>  
> -        parent = discovery.findcommonoutgoing(
> -            repo, other, [], force=opts.get('force')).missing[0:1]
> +        # hexlify nodes from outgoing, because we're going to parse
> +        # parent[0] using revsingle below, and if the binary hash
> +        # contains special revset characters like ":" the revset
> +        # parser can choke.
> +        parent = map(node.hex, discovery.findcommonoutgoing(
> +            repo, other, [], force=opts.get('force')).missing[0:1])
>      else:
>          if opts.get('force'):
>              raise util.Abort(_('--force only allowed with --outgoing'))
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/hgext/histedit.py b/hgext/histedit.py
--- a/hgext/histedit.py
+++ b/hgext/histedit.py
@@ -454,8 +454,12 @@ 
         if revs:
             revs = [repo.lookup(rev) for rev in revs]
 
-        parent = discovery.findcommonoutgoing(
-            repo, other, [], force=opts.get('force')).missing[0:1]
+        # hexlify nodes from outgoing, because we're going to parse
+        # parent[0] using revsingle below, and if the binary hash
+        # contains special revset characters like ":" the revset
+        # parser can choke.
+        parent = map(node.hex, discovery.findcommonoutgoing(
+            repo, other, [], force=opts.get('force')).missing[0:1])
     else:
         if opts.get('force'):
             raise util.Abort(_('--force only allowed with --outgoing'))