Patchwork [V2] revset: evaluate filesets against each revision for 'file()' (issue5778)

login
register
mail settings
Submitter Matt Harbison
Date Feb. 1, 2018, 4:28 a.m.
Message ID <1cb013a6f74e22a18679.1517459333@Envy>
Download mbox | patch
Permalink /patch/27099/
State Accepted
Headers show

Comments

Matt Harbison - Feb. 1, 2018, 4:28 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1517166539 18000
#      Sun Jan 28 14:08:59 2018 -0500
# Branch stable
# Node ID 1cb013a6f74e22a1867962bdd9a810c787dd4944
# Parent  768326377e4d34e3724fe14033723e2c50adb98e
revset: evaluate filesets against each revision for 'file()' (issue5778)

After f2aeff8a87b6, the fileset was evaluated to a set of files against the
working directory, and then those files were applied against each revision.  The
result was nonsense.  For example, `hg log -r 'file("set:exec()")'` on the
Mercurial repo listed revision 0 because it has the `hg` script, which is
currently +x.  But that bit wasn't applied until revision 280 (which
'contains()' properly indicates).

This technique was borrowed from checkstatus(), which services adds(),
modifies(), and removes(), so it seems safe enough.  The 'r=' case is explicitly
assigned to wdirrev, freeing up rev=None to mean "re-evaluate at each revision".
The distinction is important to avoid behavior changes with `hg log set:...`
(test-largefiles-misc.t and test-fileset-generated.t drop current log output
without this).  I'm not sure what the right behavior for that is (1fd352aa08fc
explicitly enabled this behavior for graphlog), but the day before the release
isn't the time to experiment.
Yuya Nishihara - Feb. 1, 2018, 12:34 p.m.
On Wed, 31 Jan 2018 23:28:53 -0500, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1517166539 18000
> #      Sun Jan 28 14:08:59 2018 -0500
> # Branch stable
> # Node ID 1cb013a6f74e22a1867962bdd9a810c787dd4944
> # Parent  768326377e4d34e3724fe14033723e2c50adb98e
> revset: evaluate filesets against each revision for 'file()' (issue5778)

Queued modified version for stable, thanks.

> I'm not sure what the right behavior for that is (1fd352aa08fc
> explicitly enabled this behavior for graphlog), but the day before the release
> isn't the time to experiment.

It's desired behavior. Log filesets are evaluated against wctx no matter if
they are "slow" or not.

> diff --git a/mercurial/revset.py b/mercurial/revset.py
> --- a/mercurial/revset.py
> +++ b/mercurial/revset.py
> @@ -1065,8 +1065,7 @@
>              if rev is not None:
>                  raise error.ParseError('_matchfiles expected at most one '
>                                         'revision')
> -            if value != '': # empty means working directory; leave rev as None
> -                rev = value
> +            rev = value
>          elif prefix == 'd:':
>              if default is not None:
>                  raise error.ParseError('_matchfiles expected at most one '
> @@ -1076,9 +1075,11 @@
>              raise error.ParseError('invalid _matchfiles prefix: %s' % prefix)
>      if not default:
>          default = 'glob'
> +    if rev == '':
> +        rev = node.wdirrev

I've mode this back to the loop so _matchfiles('r:', 'r:bar') is rejected.

> +The fileset revset is evaluated for each revision, instead of once on wdir(),
> +and then patterns matched on each revision.  Here, no exec bits are set in
> +wdir(), but a matching revision is detected.

Added "chmod -x" here because .hgfs/large2 matched 'set:exec()' on Linux.
Perhaps this is a bug of largefiles.

#if execbit
  $ chmod -x .hglf/large2
#endif

> +
> +  $ hg file 'set:exec()'
> +  [1]
> +  $ hg log -r 'file("set:exec()")'

And made it -q because the revision 9 isn't tip.

> +  changeset:   9:be1b433a65b1
> +  tag:         tip

Patch

diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -1065,8 +1065,7 @@ 
             if rev is not None:
                 raise error.ParseError('_matchfiles expected at most one '
                                        'revision')
-            if value != '': # empty means working directory; leave rev as None
-                rev = value
+            rev = value
         elif prefix == 'd:':
             if default is not None:
                 raise error.ParseError('_matchfiles expected at most one '
@@ -1076,9 +1075,11 @@ 
             raise error.ParseError('invalid _matchfiles prefix: %s' % prefix)
     if not default:
         default = 'glob'
+    if rev == '':
+        rev = node.wdirrev
+    hasset = any(matchmod.patkind(p) == 'set' for p in pats + inc + exc)
 
-    m = matchmod.match(repo.root, repo.getcwd(), pats, include=inc,
-                       exclude=exc, ctx=repo[rev], default=default)
+    mcache = [None]
 
     # This directly read the changelog data as creating changectx for all
     # revisions is quite expensive.
@@ -1089,6 +1090,14 @@ 
             files = repo[x].files()
         else:
             files = getfiles(x)
+
+        if not mcache[0] or (hasset and rev is None):
+            r = x if rev is None else rev
+            mcache[0] = matchmod.match(repo.root, repo.getcwd(), pats,
+                                       include=inc, exclude=exc, ctx=repo[r],
+                                       default=default)
+        m = mcache[0]
+
         for f in files:
             if m(f):
                 return True
diff --git a/tests/test-glog.t b/tests/test-glog.t
--- a/tests/test-glog.t
+++ b/tests/test-glog.t
@@ -1675,7 +1675,7 @@ 
       (string 'p:c')))
   <filteredset
     <spanset- 0:5>,
-    <matchfiles patterns=['a', 'c'], include=[] exclude=[], default='relpath', rev=None>>
+    <matchfiles patterns=['a', 'c'], include=[] exclude=[], default='relpath', rev=2147483647>>
 
 Test multiple --include/--exclude/paths
 
@@ -1694,7 +1694,7 @@ 
       (string 'x:e')))
   <filteredset
     <spanset- 0:5>,
-    <matchfiles patterns=['a', 'e'], include=['a', 'e'] exclude=['b', 'e'], default='relpath', rev=None>>
+    <matchfiles patterns=['a', 'e'], include=['a', 'e'] exclude=['b', 'e'], default='relpath', rev=2147483647>>
 
 Test glob expansion of pats
 
@@ -1732,7 +1732,7 @@ 
       (string 'p:dir')))
   <filteredset
     <generatorsetdesc->,
-    <matchfiles patterns=['dir'], include=[] exclude=[], default='relpath', rev=None>>
+    <matchfiles patterns=['dir'], include=[] exclude=[], default='relpath', rev=2147483647>>
   $ hg up -q tip
 
 Test --follow on file not in parent revision
@@ -1754,7 +1754,7 @@ 
       (string 'p:glob:*')))
   <filteredset
     <generatorsetdesc->,
-    <matchfiles patterns=['glob:*'], include=[] exclude=[], default='relpath', rev=None>>
+    <matchfiles patterns=['glob:*'], include=[] exclude=[], default='relpath', rev=2147483647>>
 
 Test --follow on a single rename
 
@@ -1875,7 +1875,7 @@ 
       (string 'p:set:copied()')))
   <filteredset
     <spanset- 0:7>,
-    <matchfiles patterns=['set:copied()'], include=[] exclude=[], default='relpath', rev=None>>
+    <matchfiles patterns=['set:copied()'], include=[] exclude=[], default='relpath', rev=2147483647>>
   $ testlog --include "set:copied()"
   []
   (func
@@ -1886,11 +1886,13 @@ 
       (string 'i:set:copied()')))
   <filteredset
     <spanset- 0:7>,
-    <matchfiles patterns=[], include=['set:copied()'] exclude=[], default='relpath', rev=None>>
+    <matchfiles patterns=[], include=['set:copied()'] exclude=[], default='relpath', rev=2147483647>>
   $ testlog -r "sort(file('set:copied()'), -rev)"
   ["sort(file('set:copied()'), -rev)"]
   []
-  <baseset []>
+  <filteredset
+    <fullreposet- 0:7>,
+    <matchfiles patterns=['set:copied()'], include=[] exclude=[], default='glob', rev=None>>
 
 Test --removed
 
@@ -1908,7 +1910,7 @@ 
       (string 'p:a')))
   <filteredset
     <spanset- 0:7>,
-    <matchfiles patterns=['a'], include=[] exclude=[], default='relpath', rev=None>>
+    <matchfiles patterns=['a'], include=[] exclude=[], default='relpath', rev=2147483647>>
   $ testlog --removed --follow a
   []
   (func
@@ -1919,7 +1921,7 @@ 
       (string 'p:a')))
   <filteredset
     <generatorsetdesc->,
-    <matchfiles patterns=['a'], include=[] exclude=[], default='relpath', rev=None>>
+    <matchfiles patterns=['a'], include=[] exclude=[], default='relpath', rev=2147483647>>
 
 Test --patch and --stat with --follow and --follow-first
 
@@ -2290,7 +2292,7 @@ 
       (string 'p:.')))
   <filteredset
     <spanset- 0:9>,
-    <matchfiles patterns=['.'], include=[] exclude=[], default='relpath', rev=None>>
+    <matchfiles patterns=['.'], include=[] exclude=[], default='relpath', rev=2147483647>>
   $ testlog ../b
   []
   (func
diff --git a/tests/test-largefiles-update.t b/tests/test-largefiles-update.t
--- a/tests/test-largefiles-update.t
+++ b/tests/test-largefiles-update.t
@@ -726,6 +726,20 @@ 
 
 #endif
 
+The fileset revset is evaluated for each revision, instead of once on wdir(),
+and then patterns matched on each revision.  Here, no exec bits are set in
+wdir(), but a matching revision is detected.
+
+  $ hg file 'set:exec()'
+  [1]
+  $ hg log -r 'file("set:exec()")'
+  changeset:   9:be1b433a65b1
+  tag:         tip
+  parent:      4:07d6153b5c04
+  user:        test
+  date:        Thu Jan 01 00:00:00 1970 +0000
+  summary:     chmod +x large2
+  
 Test a fatal error interrupting an update. Verify that status report dirty
 files correctly after an interrupted update. Also verify that checking all
 hashes reveals it isn't clean.