Use inotify-simple instead of inotifyx

There's little hope that it gets maintained because it contains a C extension whereas alternatives often use ctypes. In particular, it has no support for Python 3, and this is a blocker for us. At the beginning, [I though we were switching to pyinotify](0bb405fc): it is quite popular, and even used by fail2ban, but the code is ugly (big, crazy API, limited, and probably slow).

I didn't really know inotify and it's disappointing to see that created files (CREATE+WRITE+CLOSE_WRITE) can't be distinguished from hard links (CREATE). Acting upon new inodes is a common scenario and in the first case, you want to wait CLOSE_WRITE or you would read a partially written file. What I mean is that inotify is often unreliable, unless you detect changes done by your own software (e.g. you can make sure that files aren't hard-linked) but then some other IPC is probably simpler.

In any case, I open this MR because I haven't tested it. I only checked with pylint (hence the second commit).

/cc @alain.takoudjou @gabriel @luke (from Git history)

/reviewed-on nexedi/slapos!257

setup.py

@@ -55,11 +55,12 @@ setup(name=name,
       packages=find_packages(),
       include_package_data=True,
       install_requires=[
+        'enum34',  # for inotify-simple
         'jsonschema',
         'hexagonit.recipe.download',
         'netaddr', # to manipulate on IP addresses
         'setuptools', # namespaces
-        'inotifyx', # XXX use pyinotify instead
+        'inotify-simple',
         'lock_file', #another lockfile implementation for multiprocess
         'slapos.core', # uses internally
         'zc.buildout', # plays with buildout

slapos/recipe/librecipe/execute.py

@@ -3,47 +3,31 @@ import os
 import signal
 import subprocess
 import time
-
-import inotifyx
+from collections import defaultdict
+from inotify_simple import INotify, flags
 
 def _wait_files_creation(file_list):
-  # Etablish a list of directory and subfiles
-  directories = dict()
-  for dirname, filename in [os.path.split(f) for f in file_list]:
-    directories.setdefault(dirname, dict())
-    directories[dirname][filename] = False
+  # Establish a list of directory and subfiles.
+  # and test existence before watching, so that we don't miss an event.
+  directories = defaultdict(dict)
+  for f in file_list:
+    dirname, filename = os.path.split(f)
+    directories[dirname][filename] = os.path.lexists(f)
 
   def all_files_exists():
-    return all([all(files.values()) for files in directories.values()])
+    return all(all(files.itervalues()) for files in directories.itervalues())
 
-  fd = inotifyx.init()
-  try:
-    # Watch every directories where the file are
-    watchdescriptors = dict()
-    for dirname in directories.keys():
-      wd = inotifyx.add_watch(fd,
-                      dirname,
-                      inotifyx.IN_CREATE | inotifyx.IN_DELETE | inotifyx.IN_MOVE)
-      watchdescriptors[wd] = dirname
-
-    # Set to True the file wich exists
-    for dirname, filename in [os.path.split(f) for f in file_list]:
-      directories[dirname][filename] = os.path.exists(os.path.join(dirname,
-                                                                   filename))
-    # Let's wait for every file creation
-    while not all_files_exists():
-      events_list = inotifyx.get_events(fd)
-      for event in events_list:
-        dirname = watchdescriptors[event.wd]
-        if event.name in directories[dirname]:
-          # One of watched file was created or deleted
-          if event.mask & inotifyx.IN_DELETE:
-            directories[dirname][event.name] = False
-          else:
-            directories[dirname][event.name] = True
+  with INotify() as inotify:
+    watchdescriptors = {inotify.add_watch(dirname,
+        flags.CREATE | flags.DELETE | flags.MOVED_TO | flags.MOVED_FROM
+        ): dirname
+      for dirname in directories}
 
-  finally:
-    os.close(fd)
+    while not all_files_exists():
+      for event in inotify.read():
+        directory = directories[watchdescriptors[event.wd]]
+        if event.name in directory:
+          directory[event.name] = event.mask & (flags.CREATE | flags.MOVED_TO)
 
 def execute(args):
   """Portable execution with process replacement"""
@@ -83,8 +67,8 @@ def generic_exec(args):
 
   os.execve(exec_list[0], exec_list + sys.argv[1:], exec_env)
 
-def sig_handler(signal, frame):
-  print 'Received signal %r, killing children and exiting' % signal
+def sig_handler(sig, frame):
+  print 'Received signal %r, killing children and exiting' % sig
   if child_pg is not None:
     os.killpg(child_pg, signal.SIGHUP)
     os.killpg(child_pg, signal.SIGTERM)
@@ -97,6 +81,7 @@ signal.signal(signal.SIGTERM, sig_handler)
 
 def execute_with_signal_translation(args):
   """Run process as children and translate from SIGTERM to another signal"""
+  global child_pg
   child = subprocess.Popen(args, close_fds=True, preexec_fn=os.setsid)
   child_pg = child.pid
   try:

slapos/recipe/librecipe/inotify.py

@@ -25,8 +25,7 @@
 #
 ##############################################################################
 import os
-
-import inotifyx
+from inotify_simple import INotify, flags
 
 def subfiles(directory):
   """Return the list of subfiles of a directory, and wait for the newly created
@@ -34,18 +33,12 @@ def subfiles(directory):
 
   CAUTION : *DONT TRY TO CONVERT THE RESULT OF THIS FUNCTION INTO A LIST !
   ALWAYS ITERATE OVER IT !!!*"""
-  watchfd = inotifyx.init()
-  inotifyx.add_watch(watchfd, directory, inotifyx.IN_CREATE)
-  try:
 
-    subfiles = set(os.listdir(directory))
-    subfiles |= set([file_.name for file_ in inotifyx.get_events(watchfd, 0)])
+  with INotify() as inotify:
+    inotify.add_watch(directory, flags.CLOSE_WRITE | flags.MOVED_TO)
 
+    names = os.listdir(directory)
     while True:
-      for file_ in subfiles:
-        yield os.path.join(directory, file_)
-
-      subfiles = [file_.name for file_ in inotifyx.get_events(watchfd)]
-
-  finally:
-    os.close(watchfd)
+      for name in names:
+        yield os.path.join(directory, name)
+      names = (event.name for event in inotify.read())

slapos/test/recipe/test_inotify.py

+import os, shutil, tempfile, threading, unittest
+from slapos.recipe.librecipe import execute, inotify
+
+class TestInotify(unittest.TestCase):
+
+  def setUp(self):
+    self.tmp = tempfile.mkdtemp()
+
+  def tearDown(self):
+    shutil.rmtree(self.tmp)
+
+  def test_subfiles(self):
+    p = lambda x: os.path.join(self.tmp, x)
+    def create(name, text):
+      a = open(p(name), 'w')
+      a.write(text)
+      a.flush()
+      return a
+    def check(name, text):
+      path = next(notified)
+      self.assertEqual(path, p(name))
+      with open(path) as f:
+        self.assertEqual(f.read(), text)
+    a = create('first', 'blah')
+    a.write('...')
+    notified = inotify.subfiles(self.tmp)
+    check('first', 'blah')
+    os.link(p(a.name), p('a hard link')) # ignored
+    b = create('other', 'hello')
+    b.close()
+    check('other', 'hello')
+    c = create('last', '!!!')
+    a.close()
+    check('first', 'blah...')
+    os.rename(p(a.name), p(b.name))
+    check('other', 'blah...')
+    c.close()
+    check('last', '!!!')
+
+  def test_wait_files_creation(self):
+    file_list = (
+      'foo',
+      'bar',
+      'hello/world',
+      'hello/world!',
+      'a/b/c',
+    )
+    create = lambda x: open(x, 'w').close()
+    p = lambda x: os.path.join(self.tmp, x)
+    P = lambda x: p(file_list[x])
+    create(P(1))
+    os.mkdir(p('hello'))
+    os.makedirs(p('a/b'))
+    t = threading.Thread(target=execute._wait_files_creation,
+                         args=(map(p, file_list),))
+    t.daemon = True
+    t.start()
+    def check():
+      t.join(.2)
+      self.assertTrue(t.is_alive())
+    check()
+    for x in P(3), p('a/b/d'), P(0):
+      create(x)
+    check()
+    os.rename(P(3), P(2))
+    os.rename(p('a/b/d'), P(4))
+    check()
+    os.remove(P(1))
+    for x in P(3), P(1):
+      create(x)
+    t.join(10)
+    self.assertFalse(t.is_alive())

software/unstable/condor/software.cfg

@@ -51,7 +51,6 @@ MarkupSafe = 0.18
 Werkzeug = 0.8.3
 buildout-versions = 1.7
 hexagonit.recipe.cmmi = 2.0
-inotifyx = 0.2.0-1
 lxml = 3.2.1
 meld3 = 0.6.10
 netaddr = 0.7.10

software/unstable/zimbra-kvm/software.cfg

@@ -44,7 +44,6 @@ hexagonit.recipe.download = 1.6nxd002
 
 # Required by:
 # slapos.cookbook==0.73.1
-inotifyx = 0.2.0
 
 # Required by:
 # slapos.cookbook==0.73.1

stack/slapos.cfg

@@ -111,7 +111,7 @@ collective.recipe.template = 2.0
 cryptography = 2.1.1
 decorator = 4.0.11
 idna = 2.2
-inotifyx = 0.2.2
+inotify-simple = 1.1.1
 itsdangerous = 0.24
 lock-file = 2.0
 lxml = 3.7.3