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
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.
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 --------------------------------------------------------------------