Improve ActivityFactory utility functions and reduce code-path duplication.

  * Remove a race in open_log_file()

  * Make open_log_file return a path as well as a file-object so that
    Rainbow-launched activities can log to the same path that sugar-launched
    ones do.

  * Change the call to Rainbow so that it writes to the log-file provided by
    open_log_file()

  * Define SUGAR_BUNDLE_ID env variable and change the location of the check
    for /etc/olpc-security so that Rainbow doesn't have to search for or parse
    the activity bundle.

  * Remove import statements marked as unnecessary by pylint.

  * Fix up some whitespace issues.

1 files changed, 53 insertions, 45 deletions

diff --git a/lib/sugar/activity/activityfactory.py b/lib/sugar/activity/activityfactory.py
index 1789542..46b8689 100644
--- a/lib/sugar/activity/activityfactory.py
+++ b/lib/sugar/activity/activityfactory.py
@@ -21,12 +21,10 @@ import subprocess
 
 import dbus
 import gobject
-import gtk
 
 from sugar.presence import presenceservice
 from sugar.activity.activityhandle import ActivityHandle
 from sugar.activity import registry
-from sugar.datastore import datastore
 from sugar import util
 from sugar import env
 
@@ -93,6 +91,7 @@ def get_environment(activity):
         os.mkdir(tmp_dir)
 
     environ['SUGAR_BUNDLE_PATH'] = activity.path
+    environ['SUGAR_BUNDLE_ID']   = activity.bundle_id
     environ['SUGAR_ACTIVITY_ROOT'] = activity_root
     environ['PATH'] = bin_path + ':' + environ['PATH']
 
@@ -115,41 +114,47 @@ def get_command(activity, activity_id=None, object_id=None, uri=None):
 
     return command
 
-def open_log_file(activity, activity_id):
-    for i in range(1, 100):
+def open_log_file(activity):
+    i = 1
+    while True:
         path = env.get_logs_path('%s-%s.log' % (activity.bundle_id, i))
-        if not os.path.exists(path):
-            return open(path, 'w')
+        try:
+            fd = os.open(path, os.O_EXCL | os.O_CREAT \
+                             | os.O_SYNC | os.O_WRONLY, 0644)
+            f = os.fdopen(fd, 'w', 0)
+            return (path, f)
+        except:
+            i += 1
 
 class ActivityCreationHandler(gobject.GObject):
     """Sugar-side activity creation interface
-    
+
     This object uses a dbus method on the ActivityFactory
-    service to create the new activity.  It generates 
+    service to create the new activity.  It generates
     GObject events in response to the success/failure of
-    activity startup using callbacks to the service's 
+    activity startup using callbacks to the service's
     create call.
     """
 
     def __init__(self, service_name, handle):
         """Initialise the handler
-        
+
         service_name -- the service name of the bundle factory
-        activity_handle -- stores the values which are to 
+        activity_handle -- stores the values which are to
             be passed to the service to uniquely identify
-            the activity to be created and the sharing 
+            the activity to be created and the sharing
             service that may or may not be connected with it
-            
+
             sugar.activity.activityhandle.ActivityHandle instance
-        
-        calls the "create" method on the service for this 
-        particular activity type and registers the 
-        _reply_handler and _error_handler methods on that 
+
+        calls the "create" method on the service for this
+        particular activity type and registers the
+        _reply_handler and _error_handler methods on that
         call's results.
-        
-        The specific service which creates new instances of this 
+
+        The specific service which creates new instances of this
         particular type of activity is created during the activity
-        registration process in shell bundle registry which creates 
+        registration process in shell bundle registry which creates
         service definition files for each registered bundle type.
 
         If the file '/etc/olpc-security' exists, then activity launching
@@ -184,37 +189,40 @@ class ActivityCreationHandler(gobject.GObject):
 
     def _create_activity(self):
         if self._handle.activity_id is None:
-           self._handle.activity_id = create_activity_id()
+            self._handle.activity_id = create_activity_id()
 
         self._shell.NotifyLaunch(
                     self._service_name, self._handle.activity_id,
                     reply_handler=self._no_reply_handler,
                     error_handler=self._notify_launch_error_handler)
 
-        if not os.path.exists('/etc/olpc-security'):
-            activity_registry = registry.get_registry()
-            activity = activity_registry.get_activity(self._service_name)
-            if activity:
-                env = get_environment(activity)
-                log_file = open_log_file(activity, self._handle.activity_id)
-                command = get_command(activity, self._handle.activity_id,
-                                      self._handle.object_id,
-                                      self._handle.uri)
-                process = subprocess.Popen(command, env=env, cwd=activity.path,
+        activity_registry = registry.get_registry()
+        activity = activity_registry.get_activity(self._service_name)
+        if activity:
+            environ = get_environment(activity)
+            (log_path, log_file) = open_log_file(activity)
+            command = get_command(activity, self._handle.activity_id,
+                                  self._handle.object_id,
+                                  self._handle.uri)
+
+            if not os.path.exists('/etc/olpc-security'):
+                process = subprocess.Popen(command, env=environ, cwd=activity.path,
                                            stdout=log_file, stderr=log_file)
-        else:
-            system_bus = dbus.SystemBus()
-            factory = system_bus.get_object(_RAINBOW_SERVICE_NAME,
-                                            _RAINBOW_ACTIVITY_FACTORY_PATH)
-            stdio_paths = {'stdout': '/logs/stdout', 'stderr': '/logs/stderr'}
-            factory.CreateActivity(
-                    self._service_name,
-                    self._handle.get_dict(),
-                    stdio_paths,
-                    timeout=120 * DBUS_PYTHON_TIMEOUT_UNITS_PER_SECOND,
-                    reply_handler=self._create_reply_handler,
-                    error_handler=self._create_error_handler,
-                    dbus_interface=_RAINBOW_ACTIVITY_FACTORY_INTERFACE)
+            else:
+                log_file.close()
+                system_bus = dbus.SystemBus()
+                factory = system_bus.get_object(_RAINBOW_SERVICE_NAME,
+                                                _RAINBOW_ACTIVITY_FACTORY_PATH)
+                factory.CreateActivity(
+                        log_path,
+                        environ,
+                        command,
+                        environ['SUGAR_BUNDLE_PATH'],
+                        environ['SUGAR_BUNDLE_ID'],
+                        timeout=30 * DBUS_PYTHON_TIMEOUT_UNITS_PER_SECOND,
+                        reply_handler=self._create_reply_handler,
+                        error_handler=self._create_error_handler,
+                        dbus_interface=_RAINBOW_ACTIVITY_FACTORY_INTERFACE)
 
     def _no_reply_handler(self, *args):
         pass
@@ -233,7 +241,7 @@ class ActivityCreationHandler(gobject.GObject):
         logging.error("Activity activation request failed %s" % err)
 
     def _create_reply_handler(self, xid):
-        logging.debug("Activity created %s (%s)." % 
+        logging.debug("Activity created %s (%s)." %
             (self._handle.activity_id, self._service_name))
 
     def _create_error_handler(self, err):