Fix shared=true, other bugs, and inconsistencies between recipes; much cleanup

slapos/recipe/__init__.py

@@ -5,59 +5,177 @@ except ImportError:
     from pkgutil import extend_path
     __path__ = extend_path(__path__, __name__)
 
-import errno, logging, os, shutil
-import zc.buildout
-
-logger = logging.getLogger(__name__)
+import errno, json, logging, os, shutil, stat
+from hashlib import md5
+from zc.buildout import UserError
+from zc.buildout.rmtree import rmtree as buildout_rmtree
 
 def generatePassword(length=8):
   from random import SystemRandom
   from string import ascii_lowercase
   return ''.join(SystemRandom().sample(ascii_lowercase, length))
 
+def is_true(value, default=False):
+  return default if value is None else ('false', 'true').index(value)
+
+def make_read_only(path):
+  if not os.path.islink(path):
+    os.chmod(path, os.stat(path).st_mode & 0o555)
+
+def make_read_only_recursively(path):
+  make_read_only(path)
+  for root, dir_list, file_list in os.walk(path):
+    for dir_ in dir_list:
+      make_read_only(os.path.join(root, dir_))
+    for file_ in file_list:
+      make_read_only(os.path.join(root, file_))
+
 def rmtree(path):
   try:
-    os.remove(path)
+    buildout_rmtree(path)
   except OSError as e:
-    if e.errno != errno.EISDIR:
+    if e.errno == errno.ENOENT:
+      return
+    if e.errno != errno.ENOTDIR:
       raise
-    shutil.rmtree(path)
+    os.remove(path)
+
 
-class EnvironMixin:
+class EnvironMixin(object):
 
-  def __init__(self, allow_none=True):
-    environment = self.options.get('environment', '').strip()
+  def __init__(self, allow_none=True, compat=False):
+    environment = self.options.get('environment')
     if environment:
-      from os import environ
       if '=' in environment:
         self._environ = env = {}
+        if compat: # for slapos.recipe.cmmi
+          environment_section = self.options.get('environment-section')
+          if environment_section:
+            env.update(self.buildout[environment_section])
+          compat = set(env)
+        else:
+          compat = ()
         for line in environment.splitlines():
           line = line.strip()
           if line:
             try:
               k, v = line.split('=', 1)
             except ValueError:
-              raise zc.buildout.UserError('Line %r in environment is incorrect' %
-                line)
-            k = k.strip()
+              raise UserError('Line %r in environment is incorrect' % line)
+            k = k.rstrip()
             if k in env:
-              raise zc.buildout.UserError('Key %r is repeated' % k)
-            env[k] = v.strip() % environ
+             if k in compat:
+              compat.remove(k)
+             else:
+              raise UserError('Key %r is repeated' % k)
+            env[k] = v.lstrip()
       else:
-        self._environ = dict((k, v.strip() % environ)
-          for k, v in self.buildout[environment].items())
+        self._environ = self.buildout[environment]
     else:
       self._environ = None if allow_none else {}
 
-  @property
-  def environ(self):
-    if self._environ is not None:
-      from os import environ
-      env = self._environ.copy()
-      for k, v in env.items():
-        logger.info(
-          'Environment %r set to %r' if k in environ else
-          'Environment %r added with %r', k, v)
-      for kw in environ.items():
-        env.setdefault(*kw)
-      return env
+  def __getattr__(self, attr):
+    if attr == 'logger':
+      value = logging.getLogger(self.name)
+    elif attr == 'environ':
+      env = self._environ
+      del self._environ
+      if env is None:
+        value = None
+      else:
+        from os import environ
+        value = environ.copy()
+        for k in sorted(env):
+          value[k] = v = env[k] % environ
+          self.logger.info('[ENV] %s = %s', k, v)
+    else:
+      return self.__getattribute__(attr)
+    setattr(self, attr, value)
+    return value
+
+
+class Shared(object):
+
+  keep_on_error = False
+  mkdir_location = True
+  signature = None
+
+  def __init__(self, buildout, name, options):
+    self.maybe_shared = shared = is_true(options.get('shared'))
+    if shared:
+      # Trigger computation of part signature for shared signature.
+      # From now on, we should not pull new dependencies.
+      # Ignore if buildout is too old.
+      options.get('__buildout_signature__')
+      shared = buildout['buildout'].get('shared-part-list')
+      if shared:
+        profile_base_location = options.get('_profile_base_location_')
+        signature = json.dumps({
+          k: (v.replace(profile_base_location, '${:_profile_base_location_}')
+              if profile_base_location else v)
+          for k, v in options.items()
+          if k != '_profile_base_location_'
+        }, indent=0, sort_keys=True)
+        if not isinstance(signature, bytes): # BBB: Python 3
+          signature = signature.encode()
+        digest = md5(signature).hexdigest()
+        location = None
+        for shared in shared.splitlines():
+          shared = shared.strip().rstrip('/')
+          if shared:
+            location = os.path.join(os.path.join(shared, name), digest)
+            if os.path.exists(location):
+              break
+        if location:
+          self.logger = logging.getLogger(name)
+          self.logger.info('shared at %s', location)
+          self.location = location
+          self.signature = signature
+          return
+    self.location = os.path.join(buildout['buildout']['parts-directory'], name)
+
+  def assertNotShared(self, reason):
+    if self.maybe_shared:
+      raise UserError("When shared=true, " + reason)
+
+  def install(self, install):
+    signature = self.signature
+    location = self.location
+    if signature is not None:
+      path = os.path.join(location, '.buildout-shared.json')
+      if os.path.exists(path):
+        self.logger.info('shared part is already installed')
+        return ()
+    rmtree(location)
+    try:
+      if self.mkdir_location:
+        os.makedirs(location)
+      else:
+        parent = os.path.dirname(location)
+        if not os.path.isdir(parent):
+          os.makedirs(parent)
+      install()
+      try:
+        s = os.stat(location)
+      except OSError as e:
+        if e.errno != errno.ENOENT:
+          raise
+        raise UserError('%r was not created' % location)
+      if self.maybe_shared and not stat.S_ISDIR(s.st_mode):
+        raise UserError('%r is not a directory' % location)
+      if signature is None:
+        return [location]
+      tmp = path + '.tmp'
+      with open(tmp, 'wb') as f:
+        f.write(signature)
+      # XXX: The following symlink is for backward compatibility with old
+      #      'slapos node prune' (slapos.core).
+      os.symlink('.buildout-shared.json', os.path.join(location,
+        '.buildout-shared.signature'))
+      os.rename(tmp, path)
+    except:
+      if not self.keep_on_error:
+        rmtree(location)
+      raise
+    make_read_only_recursively(location)
+    return ()

slapos/recipe/build/__init__.py

@@ -36,7 +36,7 @@ import subprocess
 import sys
 import tempfile
 import zc.buildout
-from slapos.recipe import rmtree, EnvironMixin
+from .. import is_true, rmtree, EnvironMixin, Shared
 
 ARCH_MAP = {
     'i386': 'x86',
@@ -90,9 +90,7 @@ def guessPlatform():
   return ARCH_MAP[uname()[-2]]
 
 GLOBALS = (lambda *x: {x.__name__: x for x in x})(
-  call, guessPlatform, guessworkdir)
-
-TRUE_LIST = ('y', 'on', 'yes', 'true', '1')
+  call, guessPlatform, guessworkdir, is_true)
 
 class Script(EnvironMixin):
   """Free script building system"""
@@ -154,10 +152,9 @@ class Script(EnvironMixin):
       raise zc.buildout.UserError('Promise not met, found issues:\n  %s\n' %
           '\n  '.join(promise_problem_list))
 
-  def download(self, url, md5sum=None):
-    download = zc.buildout.download.Download(self.buildout['buildout'],
-        hash_name=True, cache=self.buildout['buildout'].get('download-cache'))
-    path, is_temp = download(url, md5sum=md5sum)
+  def download(self, *args, **kw):
+    path, is_temp = zc.buildout.download.Download(self.buildout['buildout'],
+      hash_name=True)(*args, **kw)
     if is_temp:
       self.cleanup_list.append(path)
     return path
@@ -227,7 +224,6 @@ class Script(EnvironMixin):
     self.options = options
     self.buildout = buildout
     self.name = name
-    self.logger = logging.getLogger('SlapOS build of %s' % self.name)
     missing = True
     keys = 'init', 'install', 'update'
     for option in keys:
@@ -238,17 +234,29 @@ class Script(EnvironMixin):
     if missing:
       raise zc.buildout.UserError(
         'at least one of the following option is required: ' + ', '.join(keys))
-    if self.options.get('keep-on-error', '').strip().lower() in TRUE_LIST:
+    if is_true(self.options.get('keep-on-error')):
       self.logger.debug('Keeping directories in case of errors')
       self.keep_on_error = True
     else:
       self.keep_on_error = False
-    if self._install and 'location' not in options:
-      options['location'] = os.path.join(
-        buildout['buildout']['parts-directory'], self.name)
     EnvironMixin.__init__(self, False)
     if self._init:
       self._exec(self._init)
+    shared = Shared(buildout, name, options)
+    if self._update:
+      shared.assertNotShared("option 'update' can't be set")
+    if self._install:
+      location = options.get('location')
+      if location:
+        shared.assertNotShared("option 'location' can't be set")
+        shared.location = location
+      else:
+        options['location'] = shared.location
+      shared.keep_on_error = True
+      shared.mkdir_location = False
+      self._shared = shared
+    else:
+      shared.assertNotShared("option 'install' must be set")
 
   def _exec(self, script):
     options = self.options
@@ -268,13 +276,13 @@ class Script(EnvironMixin):
     exec(code, g)
 
   def install(self):
-    if not self._install:
-      self.update()
-      return ""
+    if self._install:
+      return self._shared.install(self.__install)
+    self.update()
+    return ()
+
+  def __install(self):
     location = self.options['location']
-    if os.path.lexists(location):
-      self.logger.warning('Removing already existing path %r', location)
-      rmtree(location)
     self.cleanup_list = []
     try:
       self._exec(self._install)
@@ -290,9 +298,6 @@ class Script(EnvironMixin):
           else:
             self.logger.debug('Removing %r', path)
             rmtree(path)
-    if not os.path.exists(location):
-      raise zc.buildout.UserError('%r was not created' % location)
-    return location
 
   def update(self):
     if self._update:

slapos/recipe/build/tests.py

@@ -11,9 +11,8 @@ from zc.buildout.testing import buildoutTearDown
 from contextlib import contextmanager
 from functools import wraps
 from subprocess import check_call, check_output, CalledProcessError, STDOUT
-from slapos.recipe.gitclone import GIT_CLONE_ERROR_MESSAGE, \
-    GIT_CLONE_CACHE_ERROR_MESSAGE
-from slapos.recipe.downloadunpacked import make_read_only_recursively
+from ..gitclone import GIT_CLONE_ERROR_MESSAGE, GIT_CLONE_CACHE_ERROR_MESSAGE
+from .. import make_read_only_recursively
 
 
 optionflags = (doctest.ELLIPSIS | doctest.NORMALIZE_WHITESPACE)
@@ -563,6 +562,26 @@ class MakeReadOnlyTests(unittest.TestCase):
     make_read_only_recursively(self.tmp_dir)
     self.assertRaises(IOError, open, os.path.join(self.tmp_dir, 'folder', 'symlink'), 'w')
 
+MD5SUM = []
+
+def md5sum(m):
+  x = m.group(0)
+  try:
+      i = MD5SUM.index(x)
+  except ValueError:
+      i = len(MD5SUM)
+      MD5SUM.append(x)
+  return '<MD5SUM:%s>' % i
+
+renormalizing_patters = [
+  zc.buildout.testing.normalize_path,
+  zc.buildout.testing.not_found,
+  (re.compile(
+    '.*CryptographyDeprecationWarning: Python 2 is no longer supported by the Python core team. '
+    'Support for it is now deprecated in cryptography, and will be removed in the next release.\n.*'
+    ), ''),
+  (re.compile('[0-9a-f]{32}'), md5sum),
+]
 
 def test_suite():
   suite = unittest.TestSuite((
@@ -573,12 +592,9 @@ def test_suite():
           tearDown=zc.buildout.testing.buildoutTearDown,
           optionflags=optionflags,
           checker=renormalizing.RENormalizing([
-              zc.buildout.testing.normalize_path,
               (re.compile(r'http://localhost:\d+'), 'http://test.server'),
-              # Clean up the variable hashed filenames to avoid spurious
-              # test failures
-              (re.compile(r'[a-f0-9]{32}'), ''),
-              ]),
+              ] + renormalizing_patters),
+          globs={'MD5SUM': MD5SUM},
           ),
       unittest.makeSuite(GitCloneNonInformativeTests),
       unittest.makeSuite(MakeReadOnlyTests),

slapos/recipe/download.py

@@ -26,91 +26,57 @@
 ##############################################################################
 import errno
 import os
-import shutil
-import zc.buildout
-import logging
-from hashlib import md5
-from .downloadunpacked import make_read_only_recursively, Signature
+from zc.buildout import download
+from . import Shared
 
 class Recipe(object):
-  _parts = None
+
+  _shared = None
 
   def __init__(self, buildout, name, options):
-    buildout_section = buildout['buildout']
-    self._downloader = zc.buildout.download.Download(buildout_section,
-      hash_name=True)
+    self._buildout = buildout['buildout']
     self._url = options['url']
-    self._md5sum = options.get('md5sum')
+    self._md5sum = options.get('md5sum') or None
     self._name = name
     mode = options.get('mode')
-    log = logging.getLogger(name)
-    self._shared = shared = ((options.get('shared', '').lower() == 'true') and
-                             buildout['buildout'].get('shared-parts', None))
     if mode is not None:
       mode = int(mode, 8)
     self._mode = mode
-    if 'filename' in options and 'destination' in options:
-      raise zc.buildout.UserError('Parameters filename and destination are '
-        'exclusive.')
 
-    destination = options.get('destination', None)
-    if destination is None:
-      if shared:
-        shared_part = buildout['buildout'].get('shared-parts', None)
-        shared = os.path.join(shared_part.strip().rstrip('/'), name)
-        if not os.path.exists(shared):
-          os.makedirs(shared)
-        self._signature = Signature('.slapos.recipe.build.signature')
-        profile_base_location = options.get('_profile_base_location_', '')
-        for k, v in sorted(options.items()):
-          if profile_base_location:
-            v = v.replace(profile_base_location, '${:_profile_base_location_}')
-          self._signature.update(k, v)
-        shared = os.path.join(shared, self._signature.hexdigest())
-        self._parts = parts = shared
-        log.info('shared directory  %s set for %s', shared, name)
-      else:
-        self._parts = parts = os.path.join(buildout_section['parts-directory'],
-          name)
+    shared = Shared(buildout, name, options)
+    if not self._md5sum:
+      shared.assertNotShared("option 'md5sum' must be set")
 
-      destination = os.path.join(parts, options.get('filename', name))
+    destination = options.get('destination')
+    if destination:
+      shared.assertNotShared("option 'destination' can't be set")
+    else:
+      self._shared = shared
+      destination = os.path.join(shared.location,
+                                 options.get('filename') or name)
       # Compatibility with other recipes: expose location
-      options['location'] = parts
+      options['location'] = shared.location
     options['target'] = self._destination = destination
 
   def install(self):
+    shared = self._shared
+    if shared:
+      return shared.install(self._download)
     destination = self._destination
-    result = [destination]
-    parts = self._parts
-    log = logging.getLogger(self._name)
-    if self._shared:
-      log.info('Checking whether package is installed at shared path: %s', destination)
-      if self._signature.test(self._parts):
-        log.info('This shared package has been installed by other package')
-        return []
-
-    if parts is not None and not os.path.isdir(parts):
-      os.mkdir(parts)
-      result.append(parts)
-    path, is_temp = self._downloader(self._url, md5sum=self._md5sum)
-    with open(path, 'rb') as fsrc:
-      if is_temp:
-        os.remove(path)
-      try:
-        os.remove(destination)
-      except OSError as e:
-        if e.errno != errno.ENOENT:
-          raise
-      with open(destination, 'wb') as fdst:
-        if self._mode is not None:
-          os.fchmod(fdst.fileno(), self._mode)
-        shutil.copyfileobj(fsrc, fdst)
+    try:
+      os.remove(destination)
+    except OSError as e:
+      if e.errno != errno.ENOENT:
+        raise
+    self._download()
+    return [destination]
 
-    if self._shared:
-      self._signature.save(parts)
-      make_read_only_recursively(self._parts)
-    return result
+  def _download(self):
+    download.Download(self._buildout, hash_name=True)(
+      self._url, self._md5sum, self._destination)
+    if self._mode is not None:
+      os.chmod(self._destination, self._mode)
 
   def update(self):
     if not self._md5sum:
-      self.install()
+      self._download()

slapos/recipe/vm.py

@@ -31,16 +31,14 @@ from io import BytesIO
 from collections import defaultdict
 from contextlib import contextmanager
 from os.path import join
-from slapos.recipe import EnvironMixin, generatePassword, logger, rmtree
 from zc.buildout import UserError
+from . import EnvironMixin, generatePassword, is_true, rmtree
 
 ARCH = os.uname()[4]
 
 @contextmanager
 def building_directory(directory):
-  if os.path.lexists(directory):
-    logger.warning('Removing already existing path %r', directory)
-    rmtree(directory)
+  rmtree(directory)
   os.makedirs(directory)
   try:
     yield
@@ -48,8 +46,6 @@ def building_directory(directory):
     shutil.rmtree(directory)
     raise
 
-is_true = ('false', 'true').index
-
 class Popen(subprocess.Popen):
 
   def stop(self):
@@ -99,6 +95,7 @@ class BaseRecipe(EnvironMixin):
 
   def __init__(self, buildout, name, options, allow_none=True):
     self.buildout = buildout
+    self.name = name
     self.options = options
     try:
       options['location'] = options['location'].strip()
@@ -255,7 +252,7 @@ class InstallDebianRecipe(BaseRecipe):
             raise NotImplementedError
         p[k] = v.strip()
 
-    vm_run = is_true(options.get('vm.run', 'true'))
+    vm_run = is_true(options.get('vm.run'), True)
     packages = ['ssh', 'sudo'] if vm_run else []
     packages += options.get('packages', '').split()
     if packages: