| Submitter | Durham Goode |
|---|---|
| Date | Sept. 1, 2015, 11:53 p.m. |
| Message ID | <d4ca5e9300bd7a689834.1441151598@dev2000.prn2.facebook.com> |
| Download | mbox | patch |
| Permalink | /patch/10364/ |
| State | Accepted |
| Headers | show |
Comments
On Tue, 1 Sep 2015 16:53:18 -0700, Durham Goode wrote: > # HG changeset patch > # User Durham Goode <durham@fb.com> > # Date 1441151165 25200 > # Tue Sep 01 16:46:05 2015 -0700 > # Node ID d4ca5e9300bd7a689834ff1158bf9fada625028e > # Parent 049005de325ea400893f45bd6221215cc9b26db0 > revset: fix resolving strings from a list > > When using multiple revsets that get optimized into a list (like > hg log -r r1235 -r r1237 in hgsubversion), the revset list code was assuming the > strings were resolvable via repo[X]. hgsubversion and other extensions override > def stringset() to allow processing different revision identifiers (such as > r1235 or g<githash>), and there for the _list() implementation was circumventing > that resolution. > > The fix is to just call stringset(). The default implementaiton does the same > thing that _list was already doing (namely repo[X]). > > This has always been broken, but it was recently exposed by 4ee4f7415095 which > made "--rev X --rev Y" produce a combined revset "X | Y". > > diff --git a/mercurial/revset.py b/mercurial/revset.py > --- a/mercurial/revset.py > +++ b/mercurial/revset.py > @@ -2067,14 +2067,17 @@ def _list(repo, subset, x): > r = int(t) > if str(r) != t or r not in cl: > raise ValueError > + revs = [r] > except ValueError: > - r = repo[t].rev() > - if r in seen: > - continue > - if (r in subset > - or r == node.nullrev and isinstance(subset, fullreposet)): > - ls.append(r) > - seen.add(r) > + revs = stringset(repo, subset, t) > + > + for r in revs: > + if r in seen: > + continue > + if (r in subset > + or r == node.nullrev and isinstance(subset, fullreposet)): > + ls.append(r) > + seen.add(r) It is slightly slower because stringset() have to create a baseset object for every single revision. % echo "null$(hg log -r 0:1000 -T '+{node}')" | ./contrib/revsetbenchmarks.py plain 0) 0.033776 1) 0.035655 105%
On Tue, 2015-09-01 at 16:53 -0700, Durham Goode wrote: > # HG changeset patch > # User Durham Goode <durham@fb.com> > # Date 1441151165 25200 > # Tue Sep 01 16:46:05 2015 -0700 > # Node ID d4ca5e9300bd7a689834ff1158bf9fada625028e > # Parent 049005de325ea400893f45bd6221215cc9b26db0 > revset: fix resolving strings from a list I've queued this, thanks. We can address the minor perf regression in a follow-up.
Patch
diff --git a/mercurial/revset.py b/mercurial/revset.py --- a/mercurial/revset.py +++ b/mercurial/revset.py @@ -2067,14 +2067,17 @@ def _list(repo, subset, x): r = int(t) if str(r) != t or r not in cl: raise ValueError + revs = [r] except ValueError: - r = repo[t].rev() - if r in seen: - continue - if (r in subset - or r == node.nullrev and isinstance(subset, fullreposet)): - ls.append(r) - seen.add(r) + revs = stringset(repo, subset, t) + + for r in revs: + if r in seen: + continue + if (r in subset + or r == node.nullrev and isinstance(subset, fullreposet)): + ls.append(r) + seen.add(r) return baseset(ls) # for internal use