Patchwork [v2] histedit: select the lowest rev when looking for a root in a revset

login
register
mail settings
Submitter David Soria Parra
Date March 19, 2014, 6:32 p.m.
Message ID <cd5d791af790d7a8bac5.1395253939@dev544.prn1.facebook.com>
Download mbox | patch
Permalink /patch/3992/
State Superseded
Headers show

Comments

David Soria Parra - March 19, 2014, 6:32 p.m.
# HG changeset patch
# User David Soria Parra <davidsp@fb.com>
# Date 1394751906 25200
#      Thu Mar 13 16:05:06 2014 -0700
# Node ID cd5d791af790d7a8bac50ebf4a66e71f1b51a25b
# Parent  c152e538b85b099ce20b51104b8b7dd3666aad7c
histedit: select the lowest rev when looking for a root in a revset

When we specify a revision or a revset we just get the last element from the
list. For revsets this can lead to unintended effects where you specify a
revset like only() but instead histedit selects the highest revision in the
set as root. Therefore we should always use the lowest revision number as
root.
Matt Mackall - March 19, 2014, 6:56 p.m.
On Wed, 2014-03-19 at 11:32 -0700, David Soria Parra wrote:
> # HG changeset patch
> # User David Soria Parra <davidsp@fb.com>
> # Date 1394751906 25200
> #      Thu Mar 13 16:05:06 2014 -0700
> # Node ID cd5d791af790d7a8bac50ebf4a66e71f1b51a25b
> # Parent  c152e538b85b099ce20b51104b8b7dd3666aad7c
> histedit: select the lowest rev when looking for a root in a revset
> 
> When we specify a revision or a revset we just get the last element from the
> list. For revsets this can lead to unintended effects where you specify a
> revset like only() but instead histedit selects the highest revision in the
> set as root. Therefore we should always use the lowest revision number as
> root.
> 
> diff --git a/hgext/histedit.py b/hgext/histedit.py
> --- a/hgext/histedit.py
> +++ b/hgext/histedit.py
> @@ -565,8 +565,11 @@
>                  remote = None
>              root = findoutgoing(ui, repo, remote, force, opts)
>          else:
> -            root = revs[0]
> -            root = scmutil.revsingle(repo, root).node()
> +            rev = '.'
> +            revrange = scmutil.revrange(repo, [revs[0]])
> +            if len(revrange) > 0:
> +                rev = sorted(revrange)[0]
> +            root = repo[rev].node()

I think what actually matters here is that:

- we choose the root of the provided set
- and there is only one root

That'd be something like:

 rootrevs = repo.set('roots(%ld)', revs)
 if len(rootrevs) != 1:
     ...

This will also catch both bifurcated and disjoint sets. I presume
there's another check somewhere else that will check for linearity of
the root's descendants.
Augie Fackler - March 19, 2014, 6:56 p.m.
On Wed, Mar 19, 2014 at 2:56 PM, Matt Mackall <mpm@selenic.com> wrote:

> I think what actually matters here is that:
>
> - we choose the root of the provided set
> - and there is only one root
>
> That'd be something like:
>
>  rootrevs = repo.set('roots(%ld)', revs)
>  if len(rootrevs) != 1:
>


Oooh, good catch. I agree.

(Sorry I didn't spot that in my first review.)

Patch

diff --git a/hgext/histedit.py b/hgext/histedit.py
--- a/hgext/histedit.py
+++ b/hgext/histedit.py
@@ -565,8 +565,11 @@ 
                 remote = None
             root = findoutgoing(ui, repo, remote, force, opts)
         else:
-            root = revs[0]
-            root = scmutil.revsingle(repo, root).node()
+            rev = '.'
+            revrange = scmutil.revrange(repo, [revs[0]])
+            if len(revrange) > 0:
+                rev = sorted(revrange)[0]
+            root = repo[rev].node()
 
         keep = opts.get('keep', False)
         revs = between(repo, root, topmost, keep)
diff --git a/tests/test-histedit-arguments.t b/tests/test-histedit-arguments.t
--- a/tests/test-histedit-arguments.t
+++ b/tests/test-histedit-arguments.t
@@ -72,6 +72,26 @@ 
   [255]
   $ hg up --quiet
 
+
+Test that we pick the minimum of a revrange
+---------------------------------------
+
+  $ HGEDITOR=cat hg histedit '2::' --commands - << EOF
+  > pick eb57da33312f 2 three
+  > pick c8e68270e35a 3 four
+  > pick 08d98a8350f3 4 five
+  > EOF
+  0 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  $ hg up --quiet
+
+  $ HGEDITOR=cat hg histedit 'tip:2' --commands - << EOF
+  > pick eb57da33312f 2 three
+  > pick c8e68270e35a 3 four
+  > pick 08d98a8350f3 4 five
+  > EOF
+  0 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  $ hg up --quiet
+
 Run on a revision not descendants of the initial parent
 --------------------------------------------------------------------