Patchwork [RFC] log: changed implementation to use graphlog code

login
register
mail settings
Submitter Lucas Moscovicz
Date March 4, 2014, 5:30 p.m.
Message ID <930cbe99a23baad3de7c.1393954237@dev1037.prn2.facebook.com>
Download mbox | patch
Permalink /patch/3839/
State Superseded
Commit 69402eb72115417b67c86eef6161e5f11394fb2f
Headers show

Comments

Lucas Moscovicz - March 4, 2014, 5:30 p.m.
# HG changeset patch
# User Lucas Moscovicz <lmoscovicz@fb.com>
# Date 1393629056 28800
#      Fri Feb 28 15:10:56 2014 -0800
# Node ID 930cbe99a23baad3de7c5fc716e2b7b23fe13cfe
# Parent  ed71c40f5bcfe3558484b892dacd4cb21b6b14ef
log: changed implementation to use graphlog code

Now that revsets work in a lazy way, log code can be changed to parse every
option into a revset and then evaluate it lazily.

Now expressions like

  "hg log -b default -b ."

are converted into a revset using the same code as graphlog.

There is still a test failure associated with this change, I still wasn't able
to fix this.
Mads Kiilerich - March 4, 2014, 5:39 p.m.
On 03/04/2014 06:30 PM, Lucas Moscovicz wrote:
> There is still a test failure associated with this change, I still wasn't able
> to fix this.
>
> --- hg/tests/test-largefiles.t
> +++ hg/tests/test-largefiles.t.err

Yes, largefiles should be renamed to \U0001F4A9. The matcher monkey 
patching it does is horrible but unfortunately necessary with its basic 
design decisions.

/Mads

Patch

--- hg/tests/test-largefiles.t
+++ hg/tests/test-largefiles.t.err
@@ -1045,10 +1045,6 @@ 
   5:9d5af5072dbd  edit files again
   4:74c02385b94c  move files
   $ hg log --template '{rev}:{node|short}  {desc|firstline}\n' sub/large4
-  8:a381d2c8c80e  modify normal file and largefile in repo b
-  6:4355d653f84f  edit files yet again
-  5:9d5af5072dbd  edit files again
-  4:74c02385b94c  move files

 - .hglf only matches largefiles, without .hglf it matches 9 bco sub/normal
   $ hg log --template '{rev}:{node|short}  {desc|firstline}\n' .hglf/sub
@@ -1060,7 +1056,6 @@ 
   0:30d30fe6a5be  add files
   $ hg log --template '{rev}:{node|short}  {desc|firstline}\n' sub
   9:598410d3eb9a  modify normal file largefile in repo d
-  8:a381d2c8c80e  modify normal file and largefile in repo b
   6:4355d653f84f  edit files yet again
   5:9d5af5072dbd  edit files again
   4:74c02385b94c  move files
@@ -1070,7 +1065,6 @@ 
 - globbing gives same result
   $ hg log --template '{rev}:{node|short}  {desc|firstline}\n' 'glob:sub/*'
   9:598410d3eb9a  modify normal file largefile in repo d
-  8:a381d2c8c80e  modify normal file and largefile in repo b
   6:4355d653f84f  edit files yet again
   5:9d5af5072dbd  edit files again
   4:74c02385b94c  move files
@@ -2091,12 +2085,6 @@ 
   Invoking status precommit hook
   A sub/anotherlarge
   $ hg log anotherlarge
-  changeset:   1:9627a577c5e9
-  tag:         tip
-  user:        test
-  date:        Thu Jan 01 00:00:00 1970 +0000
-  summary:     anotherlarge
-
   $ echo more >> anotherlarge
   $ hg st .
   M anotherlarge

ERROR: test-largefiles.t output changed
!
Failed test-largefiles.t: output changed

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -1451,7 +1451,7 @@ 
 
     return filematcher
 
-def _makegraphlogrevset(repo, pats, opts, revs):
+def _makelogrevset(repo, pats, opts, revs):
     """Return (expr, filematcher) where expr is a revset string built
     from log options and file patterns or None. If --stat or --patch
     are not passed filematcher is None. Otherwise it is a callable
@@ -1498,7 +1498,7 @@ 
     # scmutil.match(). The difference is input pats are globbed on
     # platforms without shell expansion (windows).
     pctx = repo[None]
-    match, pats = scmutil.matchandpats(pctx, pats, opts)
+    match = scmutil.match(pctx, pats, opts)
     slowpath = match.anypats() or (match.files() and opts.get('removed'))
     if not slowpath:
         for f in match.files():
@@ -1606,7 +1606,7 @@ 
     possiblyunsorted = False # whether revs might need sorting
     if opts.get('rev'):
         revs = scmutil.revrange(repo, opts['rev'])
-        # Don't sort here because _makegraphlogrevset might depend on the
+        # Don't sort here because _makelogrevset might depend on the
         # order of revs
         possiblyunsorted = True
     else:
@@ -1617,7 +1617,7 @@ 
             revs.reverse()
     if not revs:
         return revset.baseset([]), None, None
-    expr, filematcher = _makegraphlogrevset(repo, pats, opts, revs)
+    expr, filematcher = _makelogrevset(repo, pats, opts, revs)
     if possiblyunsorted:
         revs.sort(reverse=True)
     if expr:
@@ -1644,6 +1644,54 @@ 
 
     return revs, expr, filematcher
 
+def getlogrevs(repo, pats, opts):
+    """Return (revs, expr, filematcher) where revs is an iterable of
+    revision numbers, expr is a revset string built from log options
+    and file patterns or None, and used to filter 'revs'. If --stat or
+    --patch are not passed filematcher is None. Otherwise it is a
+    callable taking a revision number and returning a match objects
+    filtering the files to be detailed when displaying the revision.
+    """
+    limit = loglimit(opts)
+    # Default --rev value depends on --follow but --follow behaviour
+    # depends on revisions resolved from --rev...
+    follow = opts.get('follow') or opts.get('follow_first')
+    if opts.get('rev'):
+        revs = scmutil.revrange(repo, opts['rev'])
+    elif follow:
+        revs = revset.baseset(repo.revs('reverse(:.)'))
+    else:
+        revs = revset.spanset(repo)
+        revs.reverse()
+    if not revs:
+        return revset.baseset([]), None, None
+    expr, filematcher = _makelogrevset(repo, pats, opts, revs)
+    if expr:
+        # Revset matchers often operate faster on revisions in changelog
+        # order, because most filters deal with the changelog.
+        if not opts.get('rev'):
+            revs.reverse()
+        matcher = revset.match(repo.ui, expr)
+        # Revset matches can reorder revisions. "A or B" typically returns
+        # returns the revision matching A then the revision matching B. Sort
+        # again to fix that.
+        revs = matcher(repo, revs)
+        if not opts.get('rev'):
+            revs.sort(reverse=True)
+    if limit is not None:
+        count = 0
+        limitedrevs = revset.baseset([])
+        it = iter(revs)
+        while count < limit:
+            try:
+                limitedrevs.append(it.next())
+            except (StopIteration):
+                break
+            count += 1
+        revs = limitedrevs
+
+    return revs, expr, filematcher
+
 def displaygraph(ui, dag, displayer, showparents, edgefn, getrenamed=None,
                  filematcher=None):
     seen, state = [], graphmod.asciistate()
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -4054,55 +4054,22 @@ 
     if opts.get('graph'):
         return cmdutil.graphlog(ui, repo, *pats, **opts)
 
-    matchfn = scmutil.match(repo[None], pats, opts)
+    revs, expr, filematcher = cmdutil.getlogrevs(repo, pats, opts)
     limit = cmdutil.loglimit(opts)
     count = 0
 
-    getrenamed, endrev = None, None
+    getrenamed = None
     if opts.get('copies'):
+        endrev = None
         if opts.get('rev'):
-            endrev = max(scmutil.revrange(repo, opts.get('rev'))) + 1
+            endrev = scmutil.revrange(repo, opts.get('rev')).max() + 1
         getrenamed = templatekw.getrenamedfn(repo, endrev=endrev)
 
-    df = False
-    if opts.get("date"):
-        df = util.matchdate(opts["date"])
-
-    branches = opts.get('branch', []) + opts.get('only_branch', [])
-    opts['branch'] = [repo.lookupbranch(b) for b in branches]
-
-    displayer = cmdutil.show_changeset(ui, repo, opts, True)
-    def prep(ctx, fns):
-        rev = ctx.rev()
-        parents = [p for p in repo.changelog.parentrevs(rev)
-                   if p != nullrev]
-        if opts.get('no_merges') and len(parents) == 2:
-            return
-        if opts.get('only_merges') and len(parents) != 2:
-            return
-        if opts.get('branch') and ctx.branch() not in opts['branch']:
-            return
-        if df and not df(ctx.date()[0]):
-            return
-
-        lower = encoding.lower
-        if opts.get('user'):
-            luser = lower(ctx.user())
-            for k in [lower(x) for x in opts['user']]:
-                if (k in luser):
-                    break
-            else:
-                return
-        if opts.get('keyword'):
-            luser = lower(ctx.user())
-            ldesc = lower(ctx.description())
-            lfiles = lower(" ".join(ctx.files()))
-            for k in [lower(x) for x in opts['keyword']]:
-                if (k in luser or k in ldesc or k in lfiles):
-                    break
-            else:
-                return
-
+    displayer = cmdutil.show_changeset(ui, repo, opts, buffered=True)
+    for rev in revs:
+        if count == limit:
+            break
+        ctx = repo[rev]
         copies = None
         if getrenamed is not None and rev:
             copies = []
@@ -4110,22 +4077,11 @@ 
                 rename = getrenamed(fn, rev)
                 if rename:
                     copies.append((fn, rename[0]))
-
-        revmatchfn = None
-        if opts.get('patch') or opts.get('stat'):
-            if opts.get('follow') or opts.get('follow_first'):
-                # note: this might be wrong when following through merges
-                revmatchfn = scmutil.match(repo[None], fns, default='path')
-            else:
-                revmatchfn = matchfn
-
+        revmatchfn = filematcher and filematcher(ctx.rev()) or None
         displayer.show(ctx, copies=copies, matchfn=revmatchfn)
-
-    for ctx in cmdutil.walkchangerevs(repo, matchfn, opts, prep):
-        if displayer.flush(ctx.rev()):
+        if displayer.flush(rev):
             count += 1
-        if count == limit:
-            break
+
     displayer.close()
 
 @command('manifest',