From 861bf3595a900a89628ca85419f63202d37eb32c Mon Sep 17 00:00:00 2001 From: Jun Wu Date: Tue, 16 Apr 2019 22:15:44 -0700 Subject: [PATCH] revset: move "log dir" special path to "follow()" revset Summary: The "hg log" implementation chooses to not use the "follow" revset when logging a directory, but use "_matchfiles" instead. In an upcoming change, we'd like "follow" to handle directories so fastlog only needs to patch the "follow" revset. The "follow" revset can take a directory pattern just fine. The problem is "follow" will follow *every* file inside the specified directory, which is quite expensive. For now, I just moved the "_matchfiles" fallback path to "follow" so when it detects there are too many files to follow, it will switch to "_matchfiles" directly. In theory, tree manfiest repos would have "tree manifest" infomation that speed up "follow" on a directory. But that's a bigger change, and it's probably very slow in our setup because our trees are lazy. This changes some behaviors subtly, as reflected in tests: - `-f path` can use DAG range instead of rev range, which is a good thing as rev range does not make much sense to end-users. This removes a "BUG" in test-commit-amend.t - `-f dir` can follow renames if the directory contains just a few files. This looks like a good thing, and is reflected in `test-log.t`. Reviewed By: sfilipco Differential Revision: D14863134 fbshipit-source-id: 99ddff46d43f63ce03dc7bf005e3ac1cb9b39d03 --- edenscm/mercurial/cmdutil.py | 14 +++++++++++--- edenscm/mercurial/revset.py | 22 +++++++++++++++++++++- tests/test-commit-amend.t | 18 ++++-------------- tests/test-glog.t | 12 +++--------- tests/test-log.t | 33 +++++++++++++++++++++++++++++++++ 5 files changed, 72 insertions(+), 27 deletions(-) diff --git a/edenscm/mercurial/cmdutil.py b/edenscm/mercurial/cmdutil.py index 4e55db4439..d5a8131a3d 100644 --- a/edenscm/mercurial/cmdutil.py +++ b/edenscm/mercurial/cmdutil.py @@ -2620,6 +2620,14 @@ def _makefollowlogfilematcher(repo, files, followfirst): fcacheready = [False] pctx = repo["."] + # "files" might be directories. Normalize them to actual files. + for path in files: + if path not in pctx: + # "path" could be a prefix matching a directory, use the + # real matcher interface to handle it. + files = list(pctx.walk(scmutil.match(pctx, files, default="path"))) + break + def populate(): for fn in files: fctx = pctx[fn] @@ -2695,10 +2703,10 @@ def _makelogrevset(repo, pats, opts, revs): if not slowpath: for f in match.files(): if follow and f not in wctx: - # If the file exists, it may be a directory, so let it - # take the slow path. + # If the file exists, it may be a directory. The "follow" + # revset can handle directories fine. So no need to use + # the slow path. if os.path.exists(repo.wjoin(f)): - slowpath = True continue else: raise error.Abort( diff --git a/edenscm/mercurial/revset.py b/edenscm/mercurial/revset.py index 00193081a9..f58191e472 100644 --- a/edenscm/mercurial/revset.py +++ b/edenscm/mercurial/revset.py @@ -1064,7 +1064,27 @@ def _follow(repo, subset, x, name, followfirst=False): if r is None: ctx = repo["."] m = matchmod.match(repo.root, repo.getcwd(), [x], ctx=mctx, default="path") - fctxs.extend(ctx[f].introfilectx() for f in ctx.manifest().walk(m)) + files = [] + for f in ctx.manifest().walk(m): + files.append(f) + # 3 is not chosen scientifically. But it looks sane. + if len(files) >= 3: + # Too many files. The "file" might be a prefix pattern + # matching every file in a directory. Use a different code + # path that might be cheaper. + # + # See cmdutil._makelogrevset for the example use of + # _matchfiles. + pat = "p:%s" % x + matchfiles = revsetlang.formatspec( + '_matchfiles("r:", "d:relpath", %s)', pat + ) + if revs == [None]: + s = repo.revs("reverse(ancestors(.)) & %r", matchfiles) + else: + s = repo.revs("reverse(ancestors(%ld)) & %r", revs, matchfiles) + return subset & s + fctxs.extend(ctx[f].introfilectx() for f in files) s = dagop.filerevancestors(fctxs, followfirst) else: if revs is None: diff --git a/tests/test-commit-amend.t b/tests/test-commit-amend.t index 6436c8debd..127246ac67 100644 --- a/tests/test-commit-amend.t +++ b/tests/test-commit-amend.t @@ -461,30 +461,20 @@ Sadly, this test shows internals are inconsistent. Undo the removal - (BUG: Merge commit does not match its original state) $ printf C > B $ printf D > A $ hg ci --amend -m E3 - $ hg log -f -T '{desc}' -G A + $ hg log -fr tip -T '{desc}' -G A o D |\ | ~ - | o C - |/| - | ~ o A - $ hg log -f -T '{desc}' -G B - @ E3 + $ hg log -fr tip -T '{desc}' -G B + o C |\ - | o D - | |\ - | | ~ - o | C - |\| - ~ | - / + | ~ o B diff --git a/tests/test-glog.t b/tests/test-glog.t index 78854613a9..6e942edf35 100644 --- a/tests/test-glog.t +++ b/tests/test-glog.t @@ -1645,16 +1645,10 @@ Test --follow on a directory $ testlog -f dir [] (group - (and + (group (func - (symbol 'ancestors') - (symbol '.')) - (func - (symbol '_matchfiles') - (list - (string 'r:') - (string 'd:relpath') - (string 'p:dir'))))) + (symbol 'follow') + (string 'dir')))) $ hg up -q tip Test --follow on file not in parent revision diff --git a/tests/test-log.t b/tests/test-log.t index c92d63dd69..6a7fa57d85 100644 --- a/tests/test-log.t +++ b/tests/test-log.t @@ -104,6 +104,7 @@ log on directory [255] -f, directory +(The code path using "follow()" revset will follow file renames, so 'b' and 'a' show up) $ hg up -q 3 $ hg log -f dir @@ -112,6 +113,16 @@ log on directory date: Thu Jan 01 00:00:03 1970 +0000 summary: c + changeset: 1:d89b0a12d229 + user: test + date: Thu Jan 01 00:00:02 1970 +0000 + summary: b + + changeset: 0:9161b9aeaf16 + user: test + date: Thu Jan 01 00:00:01 1970 +0000 + summary: a + -f, directory with --patch $ hg log -f dir -p @@ -126,6 +137,28 @@ log on directory @@ -0,0 +1,1 @@ +a + changeset: 1:d89b0a12d229 + user: test + date: Thu Jan 01 00:00:02 1970 +0000 + summary: b + + diff -r 9161b9aeaf16 -r d89b0a12d229 b + --- /dev/null Thu Jan 01 00:00:00 1970 +0000 + +++ b/b Thu Jan 01 00:00:02 1970 +0000 + @@ -0,0 +1,1 @@ + +a + + changeset: 0:9161b9aeaf16 + user: test + date: Thu Jan 01 00:00:01 1970 +0000 + summary: a + + diff -r 000000000000 -r 9161b9aeaf16 a + --- /dev/null Thu Jan 01 00:00:00 1970 +0000 + +++ b/a Thu Jan 01 00:00:01 1970 +0000 + @@ -0,0 +1,1 @@ + +a + -f, pattern