More optimization to wrapper scripts

The workaround for shebang size limitation is removed because it's redundant
with what is done by zc.buildout.easy_install and slapos.recipe.cmmi. This also
fix the issue that a process has a bad name even when the workaround is useless.

slapos/recipe/librecipe/execute.py

@@ -28,9 +28,37 @@ def _wait_files_creation(file_list):
         if event.name in directory:
           directory[event.name] = event.mask & (flags.CREATE | flags.MOVED_TO)
 
-def generic_exec(args, extra_environ=None, wait_list=None):
+def generic_exec(args, extra_environ=None, wait_list=None,
+                 pidfile=None, reserve_cpu=False,
+                 #shebang_workaround=False, # XXX: still needed ?
+                 ):
+  args = list(args)
+
+  if pidfile:
+    import psutil
+    try:
+      with open(pidfile) as f:
+        pid = int(f.read())
+      running = psutil.Process(pid).cmdline()
+    except Exception:
+      pass
+    else:
+      # With chained shebangs, several paths may be inserted at the beginning.
+      n = len(args)
+      for i in xrange(1+len(running)-n):
+        if args == running[i:n+i]:
+          sys.exit("Already running with pid %s." % pid)
+    with open(pidfile, 'w') as f:
+      f.write(str(os.getpid()))
+
   args += sys.argv[1:]
 
+  if reserve_cpu:
+    # If the CGROUPS cpuset is available (and prepared by slap format),
+    # request an exclusive CPU core for this process.
+    with open(os.path.expanduser('~/.slapos-cpu-exclusive'), 'a') as f:
+      f.write('%s\n' % os.getpid())
+
   if wait_list:
     _wait_files_creation(wait_list)
 

slapos/recipe/librecipe/generic.py

@@ -33,7 +33,6 @@ import sys
 import inspect
 import re
 import shutil
-from textwrap import dedent
 import urllib
 import urlparse
 
@@ -139,57 +138,25 @@ class GenericBaseRecipe(object):
 
   def createWrapper(self, name, command, parameters, comments=(),
       parameters_extra=False, environment=None,
-      pidfile=None, reserve_cpu=False
   ):
     """
-    Creates a shell script for process replacement.
+    Creates a basic shell script for process replacement.
     Takes care of quoting.
-    Takes care of #! line limitation when the wrapped command is a script.
-    if pidfile parameter is specified, then it will make the wrapper a singleton,
-    accepting to run only if no other instance is running.
 
-    :param reserve_cpu: bool, try to reserve one core for the `command`
+    This must be kept minimal to avoid code duplication with generic_exec.
+    In particular, do not implement workaround for shebang size limitation here
+    (note that this can't be done correctly with a POSIX shell, because the
+    process can't be given a name).
     """
-
     lines = [ '#!/bin/sh' ]
 
     if comments:
       lines += '# ', '\n# '.join(comments), '\n'
 
-    lines.append('COMMAND=' + shlex.quote(command))
-
     for key in environment or ():
-      lines.append('export %s=%s' % (key, environment[key]))
-
-    if pidfile:
-      lines.append(dedent("""
-          # Check for other instances
-          pidfile=%s
-          if [ -s $pidfile ]; then
-            if pid=`pgrep -F $pidfile -f "$COMMAND" 2>/dev/null`; then
-              echo "Already running with pid $pid."
-              exit 1
-            fi
-          fi
-          echo $$ > $pidfile""" % shlex.quote(pidfile)))
-
-    if reserve_cpu:
-      # if the CGROUPS cpuset is available (and prepared by slap format)
-      # request an exclusive CPU core for this process
-      lines.append(dedent("""
-        # put own PID into waiting list for exclusive CPU-core access
-        echo $$ >> ~/.slapos-cpu-exclusive
-        """))
-
-    lines.append(dedent('''
-    # If the wrapped command uses a shebang, execute the referenced
-    # executable passing the script path as first argument.
-    # This is to workaround the limitation of 127 characters in #!
-    [ ! -f "$COMMAND" ] || {
-      [ "`head -c2`" != "#!" ] || read -r EXE ARG
-    } < "$COMMAND"
-
-    exec $EXE ${ARG:+"$ARG"} "$COMMAND"'''))
+      lines.append('export %s=%s' % (key, shlex.quote(environment[key])))
+
+    lines.append('exec ' + shlex.quote(command))
 
     parameters = map(shlex.quote, parameters)
     if parameters_extra:

slapos/recipe/notifier.py

@@ -80,30 +80,34 @@ class Notify(GenericBaseRecipe):
       # Just a touch
       open(log, 'w').close()
 
-    parameters = [
+    cmd = [notifier_binary,
             '-l', log,
             '--title', title,
             '--feed', feed_url,
             '--max-run', str(max_run),
             '--notification-url',
             ]
-    parameters.extend(notification_url.split(' '))
-    parameters.extend(['--executable', executable])
+    cmd += notification_url.split(' ')
+    cmd += '--executable', executable
     # For a more verbose mode, writing feed items for any action
     instance_root_name = instance_root_name or self.options.get('instance-root-name', None)
     log_url = log_url or self.options.get('log-url', None)
     status_item_directory = status_item_directory or self.options.get('status-item-directory', None)
     if instance_root_name and log_url and status_item_directory:
-      parameters.extend([
+      cmd += (
         '--instance-root-name', instance_root_name,
         '--log-url', log_url,
         '--status-item-directory', status_item_directory,
-      ])
+      )
+
+    if pidfile:
+      return self.createPythonScript(wrapper,
+        'slapos.recipe.librecipe.execute.generic_exec',
+        (cmd,), {'pidfile': pidfile})
 
     return self.createWrapper(name=wrapper,
-                              command=notifier_binary,
-                              parameters=parameters,
-                              pidfile=pidfile,
+                              command=cmd[0],
+                              parameters=cmd[1:],
                               parameters_extra=True,
                               comments=[
                                   '',

slapos/recipe/wrapper.py

@@ -26,7 +26,6 @@
 ##############################################################################
 
 import shlex
-import os
 
 from slapos.recipe.librecipe import GenericBaseRecipe
 
@@ -45,54 +44,36 @@ class Recipe(GenericBaseRecipe):
         command_line = shlex.split(self.options['command-line'])
         wrapper_path = self.options['wrapper-path']
         wait_files = self.options.get('wait-for-files')
-        environment = self.options.get('environment')
         parameters_extra = self.options.get('parameters-extra')
         pidfile = self.options.get('pidfile')
-        reserve_cpu = self.options.get('reserve-cpu', False)
 
-        if not wait_files and not environment:
-          # Create a simple wrapper as shell script
-          return [self.createWrapper(
-             name=wrapper_path,
-             command=command_line[0],
-             parameters=command_line[1:],
-             parameters_extra=parameters_extra,
-             pidfile=pidfile,
-             reserve_cpu=reserve_cpu
-          )]
-
-        # More complex needs: create a Python script as wrapper
-
-        extra_environ = {}
-        if environment:
-            for line in environment.splitlines():
+        environment = {}
+        for line in (self.options.get('environment') or '').splitlines():
           line = line.strip()
           if line:
             k, v = line.split('=')
-                    extra_environ[k.rstrip()] = v.lstrip()
+            environment[k.rstrip()] = v.lstrip()
 
-        args = [command_line, extra_environ]
+        kw = {}
         if wait_files:
-            args.append(wait_files.split())
+          kw['wait_list'] = wait_files.split()
+        if pidfile:
+          kw['pidfile'] = pidfile
+        if self.isTrueValue(self.options.get('reserve-cpu')):
+          kw['reserve_cpu'] = True
+
+        if kw:
+          # More complex needs: create a Python script as wrapper
+          args = [command_line]
+          if environment:
+            args.append(environment)
 
-        # We create a python script and a wrapper around the python
-        # script because the python script might have a too long #! line
-        if os.path.exists(os.path.join(self.buildout['buildout']['directory'], "bin")): 
-          base_script_path = os.path.join(
-            self.buildout['buildout']['directory'], "bin/" + wrapper_path.split("/")[-1])
-        else:
-          base_script_path = os.path.join(
-            self.buildout['buildout']['directory'], wrapper_path.split("/")[-1])
-        python_script = self.createPythonScript(
-            base_script_path +'.py',
+          return self.createPythonScript(wrapper_path,
             'slapos.recipe.librecipe.execute.generic_exec',
-            args)
-        return [python_script, self.createWrapper(
-             name=wrapper_path,
-             command=python_script,
-             parameters=[],
-             parameters_extra=parameters_extra,
-             pidfile=pidfile,
-             reserve_cpu=reserve_cpu
-        )]
+            args, kw)
 
+        return self.createWrapper(wrapper_path,
+          command_line[0],
+          command_line[1:],
+          parameters_extra=self.isTrueValue(parameters_extra),
+          environment=environment)