Clean up part location if it exists before installation or if installation fails

This makes install() idempotent as it should be.
And by default, the user expects recipes to do their best
to undo any partial change on error.

rubygems.py

@@ -6,20 +6,23 @@ import glob
 import logging
 import os
 import re
-import shutil
 import stat
 import subprocess
 import zc.buildout
 
 import six.moves.urllib as urllib
 
-from slapos.recipe.downloadunpacked import Recipe as Download
 from zc.buildout.easy_install import allow_picked_versions
+from slapos.recipe import rmtree
+from slapos.recipe.downloadunpacked import Recipe as Download
 
 strip = lambda x:x.strip()  # noqa
 
 is_true = ('false', 'true').index
 
+RUBYGEMS_URL_BASE = 'https://rubygems.org/rubygems/rubygems'
+VERSION_ZIP_RE = '-([0-9.]+).zip'
+
 
 class Recipe(object):
     """zc.buildout recipe for compiling and installing software"""
@@ -42,7 +45,6 @@ class Recipe(object):
             raise zc.buildout.UserError(
                 "Configuration error, 'gems' option is missing")
 
-        self.version = options.get('version')
         self.url = options.get('url')
         # Allow to define specific ruby executable. If not, take just 'ruby'
         self.ruby_executable = options.get('ruby-executable', 'ruby')
@@ -172,36 +174,26 @@ class Recipe(object):
             gem_dict['version'] = parsed_gem[1].strip()
         return gem_dict
 
-    def _get_latest_rubygems(self):
-        if self.url:
-            version = self.version
-            if not version:
-                version = (
-                    re.search(r'rubygems-([0-9.]+).zip$', self.url).group(1)
-                )
-            return (self.url, version)
-
-        if self.version:
-            return ('https://rubygems.org/rubygems/'
-                    'rubygems-%s.zip' % self.version, self.version)
+    def _get_rubygems_url_and_version(self):
+        version = self.options.get('version')
+        if version:
+            return '%s-%s.zip' % (RUBYGEMS_URL_BASE, version), version
 
-        f = urllib.request.urlopen('https://rubygems.org/pages/download')
-        s = f.read().decode('utf-8')
-        f.close()
-        r = re.search(r'https://rubygems.org/rubygems/'
-                      r'rubygems-([0-9.]+).zip', s)
-        if r:
-            url = r.group(0)
-            version = r.group(1)
-            return (url, version)
+        url = self.url
+        if url:
+            r = re.search(VERSION_ZIP_RE + '$', url)
         else:
-            raise zc.buildout.UserError(
-                'Can\'t find latest rubygems version.')
+            f = urllib.request.urlopen('https://rubygems.org/pages/download')
+            try:
+                r = re.search(RUBYGEMS_URL_BASE + VERSION_ZIP_RE,
+                              f.read().decode('utf-8'))
+            finally:
+                f.close()
+        if r:
+            return url or r.group(0), r.group(1)
+        raise zc.buildout.UserError("Can't find rubygems version.")
 
-    def _install_rubygems(self):
-        url, version = self._get_latest_rubygems()
-        if int(version.split(".")[0]) < 2:
-            raise zc.buildout.UserError("Rubygems version must be >= 2.0.0")
+    def _install_rubygems(self, url, version):
         srcdir = os.path.join(self.buildout['buildout']['parts-directory'],
                               'rubygems-' + version)
         options = {
@@ -210,20 +202,8 @@ class Recipe(object):
         }
         recipe = Download(self.buildout, self.name, options)
         recipe.install()
-        self.version = version
 
         current_dir = os.getcwd()
-        try:
-            os.mkdir(self.options['location'])
-        except OSError as e:
-            if e.errno == errno.EEXIST:
-                pass
-            else:
-                raise zc.buildout.UserError(
-                    'IO error while creating %s directory.'
-                    % self.options['location']
-                )
-
         os.chdir(srcdir)
 
         env = self._get_env()
@@ -241,7 +221,7 @@ class Recipe(object):
         try:
             self.run(cmd, env)
         finally:
-            shutil.rmtree(srcdir)
+            rmtree(srcdir)
             os.chdir(current_dir)
 
     def _install_executable(self, path):
@@ -288,21 +268,9 @@ class Recipe(object):
 
         self.run(cmd, self._get_env())
 
-    def get_gem_executable(self, bindir):
-        gem_executable = os.path.join(bindir, 'gem')
-        gem_executable = glob.glob(gem_executable + '*')
-
-        if gem_executable:
-            if not self.version:
-                self.version = self.run([
-                    gem_executable[0],
-                    '--version',
-                ])
-            return gem_executable[0]
-
-    def get_dependency_list(self, gem_dict, gem_executable):
+    def get_dependency_list(self, gem_dict, gem_executable, version):
         gem_search_pattern = '^' + gem_dict['gemname'].replace('.',r'\.') + '$'
-        if self.version[0] < '3':
+        if version[0] < '3':
             gem_search_pattern = '/' + gem_search_pattern + '/'
 
         cmd = [
@@ -330,41 +298,53 @@ class Recipe(object):
         } for match in self.gem_regex.findall(cmd_result)]
 
     def install(self):
-        parts = [self.options['location']]
+        location = self.options['location']
 
         bindir = os.path.join(self.options['location'], 'bin')
-        gem_executable = self.get_gem_executable(bindir)
-
-        if not gem_executable:
-            self._install_rubygems()
-            gem_executable = self.get_gem_executable(bindir)
-
-        gem_dict_list = list(map(self.get_gem_dict, self.gems))
-        for gem_dict in gem_dict_list:
-            if self.deployment:
-                if 'version' not in gem_dict:
-                    raise zc.buildout.UserError(
-                        'Configuration error, '
-                        'version for gem %s is missing' % gem_dict['gemname']
-                    )
-
-                for dep_dict in self.get_dependency_list(gem_dict, 
-                                                         gem_executable):
-                    match = [gem_d for gem_d in gem_dict_list
-                              if dep_dict['gemname'] == gem_d['gemname']]
-                    if not match:
+        if os.path.lexists(location):
+            rmtree(location)
+        try:
+            os.makedirs(location)
+            parts = [location]
+
+            url, version = self._get_rubygems_url_and_version()
+            if int(version.split(".")[0]) < 2:
+                raise zc.buildout.UserError("Rubygems version must be >= 2.0.0")
+
+            self._install_rubygems(url, version)
+            gem_executable = glob.glob(os.path.join(bindir, 'gem*'))[0]
+
+            gem_dict_list = list(map(self.get_gem_dict, self.gems))
+            for gem_dict in gem_dict_list:
+                if self.deployment:
+                    if 'version' not in gem_dict:
                         raise zc.buildout.UserError(
                             'Configuration error, '
-                            'version for dependency %s is missing'
-                            % dep_dict['gemname']
+                            'version for gem %s is missing' % gem_dict['gemname']
                         )
 
-                    for constraint in dep_dict['constraint_list']:
-                        self._check_dependency_constraint(
-                            constraint, match[0], gem_dict['gemname'])
-
-            self.log.info('installing ruby gem "%s"', gem_dict['gemname'])
-            self._install_gem(gem_dict, gem_executable, bindir) 
+                    for dep_dict in self.get_dependency_list(gem_dict,
+                                                             gem_executable,
+                                                             version):
+                        match = [gem_d for gem_d in gem_dict_list
+                                  if dep_dict['gemname'] == gem_d['gemname']]
+                        if not match:
+                            raise zc.buildout.UserError(
+                                'Configuration error, '
+                                'version for dependency %s is missing'
+                                % dep_dict['gemname']
+                            )
+
+                        for constraint in dep_dict['constraint_list']:
+                            self._check_dependency_constraint(
+                                constraint, match[0], gem_dict['gemname'])
+
+                self.log.info('installing ruby gem "%s"', gem_dict['gemname'])
+                self._install_gem(gem_dict, gem_executable, bindir)
+        except:
+            if os.path.lexists(location):
+                rmtree(location)
+            raise
 
         for executable in os.listdir(bindir):
             installed_path = self._install_executable(

tests/test_rubygems.py

@@ -21,6 +21,9 @@ RUBYGEMS_DEFAULT_VERSION = '2.0.0'
 
 
 def touch(path):
+    parent = path.parent
+    if not parent.exists():
+      os.makedirs(str(parent))
     with path.open('w') as f:
         f.write('')
 
@@ -41,6 +44,20 @@ class fixture(object):
             self.tear_down()
         return wrapper
 
+    def install_rubygems(self, buildout, name, options):
+        tempdir, rubygemsdir = os.path.split(options['destination'])
+        version = rubygemsdir.split('-')[-1]
+        dirs = (
+            buildout['buildout']['bin-directory'],
+            os.path.join(tempdir, 'ruby-' + version),
+            options['destination'],
+        )
+        for directory in dirs:
+            os.makedirs(directory)
+        touch(self.tempdir.joinpath('rubygems', 'bin', 'gem'))
+
+        return type(str(), (), {'install': lambda _: None})()
+
     def patch(self, modules):
         self.patchers = {}
         self.patches = {}
@@ -48,11 +65,6 @@ class fixture(object):
             self.patchers[name] = mock.patch(module)
             self.patches[name] = self.patchers[name].start()
 
-    def makedirs(self, dirs):
-        self.tempdir = pathlib.Path(tempfile.mkdtemp())
-        for directory in dirs:
-            os.makedirs(str(self.tempdir / directory))
-
     def set_up(self):
         name = 'rubygems'
         version = self.options.get('return', {}).get('version', self.version)
@@ -67,14 +79,9 @@ class fixture(object):
                    % RUBYGEMS_DEFAULT_VERSION
                   ).encode('utf-8'))
         )
+        self.patches['download'].side_effect = self.install_rubygems
 
-        self.makedirs((
-            'bin',
-            'ruby-' + version,
-            'rubygems-' + version,
-            'rubygems/bin',
-        ))
-
+        self.tempdir = pathlib.Path(tempfile.mkdtemp())
         buildout = {'buildout': dict({
             'parts-directory': str(self.tempdir),
             'bin-directory': str(self.tempdir / 'bin'),
@@ -133,8 +140,8 @@ class RubyGemsDefaultTestCase(RubyGemsTestCase):
 
         expected_install_arg_list_list = [
             [
-                'ruby', None, 'install', '--no-document',
-                '--bindir=%s/rubygems/bin' % path,
+                'ruby', str(path.joinpath(name, 'bin', 'gem')), 'install',
+                '--no-document', '--bindir=%s/rubygems/bin' % path,
                 'sass', '--',
             ],
         ]
@@ -215,18 +222,7 @@ class RubyGemsDefaultTestCase(RubyGemsTestCase):
         patches['urlopen'].return_value = BytesIO(b'')
         recipe = rubygems.Recipe(buildout, name, options)
         self.assertRaisesRegexp(zc.buildout.UserError,
-            'Can\'t find latest rubygems version.',
-            recipe.install
-        )
-
-    @fixture({'recipe': {'gems': 'sass'}})
-    @mock.patch('rubygems.os.mkdir')
-    def test_mkdir_error(self, path, patches, buildout, name, options, version, mkdir):
-        mkdir.side_effect = OSError(errno.EIO)
-        recipe = rubygems.Recipe(buildout, name, options)
-        self.assertRaisesRegexp(
-            zc.buildout.UserError,
-            'IO error while creating %s/rubygems directory.' % path,
+            "Can't find rubygems version.",
             recipe.install
         )
 
@@ -293,12 +289,13 @@ class RubyGemsDeploymentTestCase(RubyGemsTestCase):
             if version[0] < '3':
                 gem_search_pattern = '/' + gem_search_pattern + '/'
 
+            gem_executable = str(path.joinpath(name, 'bin', 'gem'))
             expected_install_arg_list_list.extend([
                 [
-                    'ruby', None, 'dependency', '-rv', gem_dict['version'],
-                    gem_search_pattern,
+                    'ruby', gem_executable, 'dependency',
+                    '-rv', gem_dict['version'], gem_search_pattern,
                 ], [
-                    'ruby', None, 'install', '--no-document',
+                    'ruby', gem_executable, 'install', '--no-document',
                     '--bindir=%s/rubygems/bin' % path,
                     '--ignore-dependencies', gem_dict['gemname'],
                     '--version=' + gem_dict['version'], '--',
@@ -309,13 +306,9 @@ class RubyGemsDeploymentTestCase(RubyGemsTestCase):
             path, patches, expected_install_arg_list_list)
 
     @deployment_fixture({'recipe': {'gems': 'hashie==0.3.1'}})
-    @mock.patch('rubygems.Recipe.get_gem_executable')
     def test_already_installed_rubygems(
-            self, path, patches, buildout, name, options, version, get_gem_exe):
+            self, path, patches, buildout, name, options, version):
         touch(path / 'rubygems/bin/gem')
-        def mocked_get_gem_exe(_):
-            self.version = RUBYGEMS_DEFAULT_VERSION
-        get_gem_exe.side_effect = mocked_get_gem_exe
         self.deployment_install_test(
             buildout, name, path, patches, options, version)