Modified cleanup feature : now it takes prefix into account to determine if a path is valid or not.

slapos/recipe/cmmi/__init__.py

@@ -228,45 +228,57 @@ class Recipe(object):
             log.error('Command failed: %s: %s' % (e, cmd))
             raise zc.buildout.UserError('System error')
 
-    def clean(self, cleanup_list):
-      """
-      Basically, this method does not do much, appart from removing
-      what it is told to, as a list passed in args
-      """
-      import glob
-      def build_working_iterator(cleanup_list):
-        current_dir = os.getcwd()
-        for pattern in cleanup_list:
-          # if current pattern contains wildcards, we search for all matching
-          # files/directories
-          if pattern.find('*') > -1 or pattern.find('?') > -1 or pattern.find('[') > -1:
-            g = glob.glob(pattern)
-            if g:
-              for x in g:
-                yield x
+
+    def _return_if_exists_in_prefix(self, path, prefix):
+        relpath = os.path.relpath(path, prefix)
+        abspath = os.path.join(prefix, relpath)
+        if os.path.exists(abspath):
+            if relpath.startswith('..'):
+                raise IOError(prefix + " does not contain " + path)
             else:
-              raise IOError("Pattern " + pattern + " did not match anything")
-          elif os.path.exists(pattern):
-            if os.path.relpath(pattern).startswith('..'):
-              raise IOError(current_dir + " does not contain " + pattern)
+                return path
+        else:
+            raise IOError(path + " does not exist")
+
+    def _is_pattern(self, pattern):
+        if pattern.find('*') > -1 or pattern.find('?') > -1 or pattern.find('[') > -1:
+            return True
+        return False
+
+    def _build_iterator(self, cleanup_list, prefix):
+        import glob
+        # First, expand eventual wildcards
+        for entry in [os.path.join(prefix, x) for x in cleanup_list]:
+            if self._is_pattern(entry):
+                paths = glob.glob(entry)
+                if paths:
+                    for path in paths:
+                      yield self._return_if_exists_in_prefix(path, prefix)
+                else:
+                    raise IOError("Pattern " + entry + " did not match anything")
             else:
-              yield pattern
-          else:
-            raise IOError(pattern + " does not exist")
-      # As build_working_iterator is a generator and since we want to first
-      # check every file (raising exceptions when necessary) BEFORE removing
-      # anything from the file system, we enforce full lookup by build a tuple
-      # out of it
-      for x in tuple(build_working_iterator(cleanup_list)):
-        # This first condition is mandatory as we might have removed an entire
-        # tree during a previous iteration, possibly leading to a case where
-        # some items are still to be removed but do not exist anymore
-        if os.path.exists(x):
-          if os.path.isdir(x):
-            shutil.rmtree(x)
-          else:
-            os.remove(x)
-          logging.getLogger(self.name).info('%s has been successfully removed', x)
+                yield self._return_if_exists_in_prefix(entry, prefix)
+
+    def clean(self, cleanup_list):
+        """
+        Basically, this method does not do much, appart from removing
+        what it is told to, as a list passed in args
+        """
+        # As build_working_iterator is a generator and since we want to first
+        # check every file (raising exceptions when necessary) BEFORE removing
+        # anything from the file system, we enforce full lookup by build a tuple
+        # out of it
+        print self.options['prefix']
+        for x in tuple(self._build_iterator(cleanup_list, self.options['prefix'])):
+            # This first condition is mandatory as we might have removed an entire
+            # tree during a previous iteration, possibly leading to a case where
+            # some items are still to be removed but do not exist anymore
+            if os.path.exists(x):
+                if os.path.isdir(x):
+                    shutil.rmtree(x)
+                else:
+                    os.remove(x)
+                logging.getLogger(self.name).info('%s has been successfully removed', x)
 
 
 

slapos/recipe/cmmi/tests.py

@@ -7,6 +7,7 @@ import shutil
 import stat
 import tarfile
 import tempfile
+import glob
 from time import sleep
 import zc.buildout
 import zc.buildout.testing
@@ -317,7 +318,8 @@ class NonInformativeTests(unittest.TestCase):
                 'cleanup': 'foo\nbar'
             }
         )
-        os.chdir(self.dir)
+        os.mkdir(recipe.options.get('prefix'))
+        os.chdir(recipe.options.get('prefix'))
         tmp = open('foo', 'a')
         tmp = open('bar', 'a')
         self.assertTrue(os.path.exists('foo'))
@@ -326,7 +328,28 @@ class NonInformativeTests(unittest.TestCase):
         self.assertFalse(os.path.exists('foo'))
         self.assertFalse(os.path.exists('bar'))
 
+    def test_cleanup_succeeds_when_glob_is_valid(self):
+        recipe = self.make_recipe(
+            {},
+            'test',
+            {
+                'url': 'file://%s/testdata/package-0.0.0.tar.gz' % os.path.dirname(__file__),
+                'cleanup': 'foo/*'
+            }
+        )
+        os.mkdir(recipe.options.get('prefix'))
+        os.chdir(recipe.options.get('prefix'))
+        os.mkdir('foo')
+        tmp = open('foo/bar',  'a')
+        tmp = open('foo/barz', 'a')
+        self.assertTrue(os.path.exists('foo/bar'))
+        self.assertTrue(os.path.exists('foo/barz'))
+        recipe.install()
+        self.assertFalse(os.path.exists('foo/bar'))
+        self.assertFalse(os.path.exists('foo/barz'))
+
     def test_cleanup_fails_when_file_does_not_exist(self):
+        os.chdir(self.dir)
         recipe = self.make_recipe(
             {},
             'test',
@@ -335,12 +358,12 @@ class NonInformativeTests(unittest.TestCase):
                 'cleanup': 'foo'
             }
         )
-        os.chdir(self.dir)
-        self.assertFalse(os.path.exists('foo'))
+        self.assertFalse(os.path.exists(os.path.join(self.dir, 'foo')))
         with self.assertRaises(IOError):
           recipe.install()
 
     def test_cleanup_fails_when_file_is_out_of_reach(self):
+        os.chdir(self.dir)
         recipe = self.make_recipe(
             {},
             'test',
@@ -349,11 +372,23 @@ class NonInformativeTests(unittest.TestCase):
                 'cleanup': '/tmp/foo'
             }
         )
-        os.chdir(self.dir)
         open('/tmp/foo', 'a')
         self.assertTrue(os.path.exists('/tmp/foo'))
         with self.assertRaises(IOError):
-          recipe.install()
+            recipe.install()
+
+    def test_cleanup_fails_when_unmatched_glob(self):
+        os.chdir(self.dir)
+        pattern = '/tmp/foo/trololol*'
+        recipe = self.make_recipe({}, 'test',
+            {
+                'url': 'file://%s/testdata/package-0.0.0.tar.gz' % os.path.dirname(__file__),
+                'cleanup': pattern
+            }
+        )
+        self.assertEqual(glob.glob(pattern), [])
+        with self.assertRaises(IOError):
+            recipe.install()
 
 
 def test_suite():