Patchwork [6,of,6] fileset: detect unintentional existing() invocation at runtime

login
register
mail settings
Submitter Katsunori FUJIWARA
Date Dec. 21, 2015, 1:45 p.m.
Message ID <e9427ace57d99a76fe83.1450705536@feefifofum>
Download mbox | patch
Permalink /patch/12210/
State Accepted
Headers show

Comments

Katsunori FUJIWARA - Dec. 21, 2015, 1:45 p.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1450704676 -32400
#      Mon Dec 21 22:31:16 2015 +0900
# Node ID e9427ace57d99a76fe83ea0be889cb05a7d1e5f1
# Parent  6aef05b2ae92f7c9526147ec13fb02ea76a2f25e
fileset: detect unintentional existing() invocation at runtime

A fileset predicate can invoke 'matchctx.existing()' successfully,
even if it isn't marked as "existing caller". It is aborted only in
some corner cases: e.g. there were one deleted file in the working
directory (see 8a0513bf030a for detail).

This patch makes 'matchctx.existing()' invocation abort if not
'_existingenabled', which is true only while "existing caller"
running.

After this changes, non-"existing caller" predicate function is
aborted immediately, whenever it invokes 'matchctx.existing()'. This
prevent developer from forgetting to mark a predicate as "existing
caller".

BTW, unintentional 'matchctx.status()' invocation can be detected
easily without any additional trick like this patch, because it
returns 'None' if a predicate isn't marked as "status caller", and
referring field (e.g. '.modified') of it is always aborted.
Matt Mackall - Dec. 21, 2015, 5:47 p.m.
On Mon, 2015-12-21 at 22:45 +0900, FUJIWARA Katsunori wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1450704676 -32400
> #      Mon Dec 21 22:31:16 2015 +0900
> # Node ID e9427ace57d99a76fe83ea0be889cb05a7d1e5f1
> # Parent  6aef05b2ae92f7c9526147ec13fb02ea76a2f25e
> fileset: detect unintentional existing() invocation at runtime

Looks good, queued for default.

-- 
Mathematics is the supreme nostalgia of our time.

Patch

diff --git a/mercurial/fileset.py b/mercurial/fileset.py
--- a/mercurial/fileset.py
+++ b/mercurial/fileset.py
@@ -248,7 +248,13 @@  def clean(mctx, x):
 
 def func(mctx, a, b):
     if a[0] == 'symbol' and a[1] in symbols:
-        return symbols[a[1]](mctx, b)
+        funcname = a[1]
+        enabled = mctx._existingenabled
+        mctx._existingenabled = funcname in _existingcallers
+        try:
+            return symbols[funcname](mctx, b)
+        finally:
+            mctx._existingenabled = enabled
 
     keep = lambda fn: getattr(fn, '__doc__', None) is not None
 
@@ -497,6 +503,7 @@  class matchctx(object):
         self.ctx = ctx
         self.subset = subset
         self._status = status
+        self._existingenabled = False
     def status(self):
         return self._status
     def matcher(self, patterns):
@@ -504,6 +511,7 @@  class matchctx(object):
     def filter(self, files):
         return [f for f in files if f in self.subset]
     def existing(self):
+        assert self._existingenabled, 'unexpected existing() invocation'
         if self._status is not None:
             removed = set(self._status[3])
             unknown = set(self._status[4] + self._status[5])
diff --git a/tests/test-fileset.t b/tests/test-fileset.t
--- a/tests/test-fileset.t
+++ b/tests/test-fileset.t
@@ -328,3 +328,22 @@  Test safety of 'encoding' on removed fil
   b2link
   bin
   c1
+
+Test detection of unintentional 'matchctx.existing()' invocation
+
+  $ cat > $TESTTMP/existingcaller.py <<EOF
+  > from mercurial import fileset
+  > 
+  > @fileset.predicate('existingcaller()', callexisting=False)
+  > def existingcaller(mctx, x):
+  >     # this 'mctx.existing()' invocation is unintentional
+  >     return [f for f in mctx.existing()]
+  > EOF
+
+  $ cat >> .hg/hgrc <<EOF
+  > [extensions]
+  > existingcaller = $TESTTMP/existingcaller.py
+  > EOF
+
+  $ fileset 'existingcaller()' 2>&1 | tail -1
+  AssertionError: unexpected existing() invocation