Rework sugar.logger and make activities just

redirect their output to a log.

bin/sugar-activity

@@ -25,12 +25,13 @@ import pygtk
 pygtk.require('2.0')
 import gtk
 import dbus
+import dbus.service
 import dbus.glib
 
-from sugar import logger
 from sugar.activity import activityhandle
 from sugar.bundle.activitybundle import ActivityBundle
 from sugar import _sugarext
+from sugar import logger
 
 activity_instances = []
 
@@ -80,6 +81,8 @@ parser.add_option('-s', '--single-process', dest='single_process',
                   help='start all the instances in the same process')
 (options, args) = parser.parse_args()
 
+logger.start()
+
 if 'SUGAR_BUNDLE_PATH' not in os.environ:
     print 'SUGAR_BUNDLE_PATH is not defined in the environment.'
     sys.exit(1)
@@ -93,8 +96,6 @@ sys.path.append(bundle_path)
 
 bundle = ActivityBundle(bundle_path)
 
-logger.start(bundle.get_bundle_id())
-
 gettext.bindtextdomain(bundle.get_bundle_id(),
                        bundle.get_locale_path())
 gettext.textdomain(bundle.get_bundle_id())

services/shell/sugar-shell-service

@@ -22,9 +22,6 @@ import sys
 import os
 import logging
 
-from sugar import logger
-logger.start('shellservice')
-
 import gobject
 import dbus.glib
  

shell/sugar-shell

@@ -26,13 +26,10 @@ import gtk
 import gobject
 import gst
 
-from sugar import logger
 from sugar import env
+from sugar import logger
 from sugar.profile import get_profile
 
-logger.cleanup()
-logger.start('shell')
-
 sys.path.insert(0, env.get_shell_path())
 
 from view.Shell import Shell
@@ -97,6 +94,9 @@ def _shell_started_cb():
 def main():
     gobject.idle_add(_shell_started_cb)
 
+    logger.setup_logs_dir()
+    logger.start('shell')
+
     _save_session_info()
     _start_matchbox()
     _setup_translations()

sugar/activity/activityfactory.py

@@ -28,6 +28,7 @@ from sugar.presence import presenceservice
 from sugar.activity.activityhandle import ActivityHandle
 from sugar.activity import registry
 from sugar.datastore import datastore
+from sugar import logger
 from sugar import util
 
 import os
@@ -101,6 +102,11 @@ def get_command(activity, activity_id=None, object_id=None, uri=None):
 
     return command
 
+def open_log_file(activity, activity_id):
+    name = '%s-%s.log' % (activity.bundle_id, activity_id[:5])
+    path = os.path.join(logger.get_logs_dir(), name)
+    return open(path, 'w')
+
 class ActivityCreationHandler(gobject.GObject):
     """Sugar-side activity creation interface
     
@@ -176,12 +182,13 @@ class ActivityCreationHandler(gobject.GObject):
             activity = activity_registry.get_activity(self._service_name)
             if activity:
                 env = get_environment(activity)
-                command = get_command(activity,
+                log_file = open_log_file(activity, self._handle.activity_id)
-                                      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, shell=True,
-                                           cwd=activity.path)
+                                           cwd=activity.path, stdout=log_file,
+                                           stderr=log_file)
         else:
             system_bus = dbus.SystemBus()
             factory = system_bus.get_object(_RAINBOW_SERVICE_NAME,

sugar/logger.py

@@ -19,105 +19,44 @@
 import sys
 import os
 import logging
-import traceback
-from cStringIO import StringIO
 import time
 
-import dbus
+_MAX_BACKUP_DIRS = 3
-import dbus.service
-import dbus.glib
 
-from sugar import env
+def setup_logs_dir():
-
+    logs_dir = get_logs_dir()
-_log_writer = None
-
-STDOUT_LEVEL = 1000
-STDERR_LEVEL = 2000
-
-formatter = logging.Formatter('%(name)s: %(message)s')
-
-_LOGGER_SERVICE_NAME = "org.laptop.Logger"
-_LOGGER_OBJECT_PATH = "/org/laptop/Logger"
-_LOGGER_INTERFACE = "org.laptop.Logger"
-
-class LoggerManagerService(dbus.service.Object):
-    def __init__(self):
-        bus = dbus.SessionBus()
-        bus_name = dbus.service.BusName(_LOGGER_SERVICE_NAME, bus = bus)
-        dbus.service.Object.__init__(self, bus_name, _LOGGER_OBJECT_PATH)
-
-    @dbus.service.method(_LOGGER_INTERFACE)
-    def SetLevel(self, level):
-        set_level(level)
-
-class LogWriter:
-    def __init__(self, module_id):
-        self._module_id = module_id
-
-        logs_dir = _get_logs_dir()
-        log_path = os.path.join(logs_dir, module_id + '.log')
-        self._log_file = open(log_path, 'w')
-
-    def write_record(self, record):
-        self.write(record.levelno, formatter.format(record))
-
-    def write(self, level, msg):
-        if level == logging.ERROR:
-            level_txt = 'ERROR'
-        elif level == logging.WARNING:
-            level_txt = 'WARNING'
-        elif level == logging.DEBUG:
-            level_txt = 'DEBUG'
-        elif level == logging.INFO:
-            level_txt = 'INFO'
-        elif level == STDERR_LEVEL:
-            level_txt = 'STDERR'
-        elif level == STDOUT_LEVEL:
-            level_txt = 'STDOUT'            
-
-        if not len(msg) or msg[len(msg) - 1] != '\n':
-            msg += "\n"
-        fmt = "%.4f %s - %s" % (time.time(), level_txt, msg)
-        fmt = fmt.encode("utf8")
-        self._log_file.write(fmt)
-        self._log_file.flush()
-
-class Handler(logging.Handler):
-    def __init__(self, writer):
-        logging.Handler.__init__(self)
-
-        self._writer = writer
-
-    def emit(self, record):
-        self._writer.write_record(record)
-
-class StdoutCatcher:
-    def write(self, txt):
-        _log_writer.write(STDOUT_LEVEL, txt)
-        sys.__stdout__.write(txt)
-
-    def flush(self):
-        sys.__stderr__.flush()
-
-class StderrCatcher:
-    def write(self, txt):
-        _log_writer.write(STDERR_LEVEL, txt)
-        sys.__stderr__.write(txt)
-
-    def flush(self):
-        sys.__stderr__.flush()
-
-def __exception_handler(typ, exc, tb):
-    trace = StringIO()
-    traceback.print_exception(typ, exc, tb, None, trace)
-    print >> sys.stderr, trace.getvalue()
-
-    _log_writer.write(logging.ERROR, trace.getvalue())
-
-def _get_logs_dir():
-    logs_dir = os.path.join(env.get_profile_path(), 'logs')
     if not os.path.isdir(logs_dir):
         os.makedirs(logs_dir)
+
+    backup_logs = []
+    backup_dirs = []
+    for f in os.listdir(logs_dir):
+        path = os.path.join(logs_dir, f)
+        if os.path.isfile(path):
+            backup_logs.append(f)
+        elif os.path.isdir(path):
+            backup_dirs.append(path)    
+
+    if len(backup_dirs) > _MAX_BACKUP_DIRS:
+        backup_dirs.sort()
+        root = backup_dirs[0]
+        for f in os.listdir(root):
+            os.remove(os.path.join(root, f))
+        os.rmdir(root)
+
+    if len(backup_logs) > 0:
+        name = str(int(time.time()))
+        backup_dir = os.path.join(logs_dir, name)
+        os.mkdir(backup_dir)
+        for log in backup_logs:
+            source_path = os.path.join(logs_dir, log)
+            dest_path = os.path.join(backup_dir, log)
+            os.rename(source_path, dest_path)
+
+def get_logs_dir():
+    profile = os.environ.get('SUGAR_PROFILE', 'default')
+    logs_dir = os.path.join(os.path.expanduser('~'),
+                            '.sugar', profile, 'logs')
     return logs_dir
 
 def set_level(level):
@@ -125,91 +64,20 @@ def set_level(level):
                'warning' : logging.WARNING,
                'debug'   : logging.DEBUG,
                'info'    : logging.INFO }
-    root_logger = logging.getLogger('')
     if levels.has_key(level):
-        root_logger.setLevel(levels[level])
+        logging.getLogger('').setLevel(levels[level])
-
-def start(module_id):
-    log_writer = LogWriter(module_id)
-
-    root_logger = logging.getLogger('')
-    root_logger.setLevel(logging.ERROR)
-    root_logger.addHandler(Handler(log_writer))
 
+def start(log_filename=None, redirect_io=True):
     if os.environ.has_key('SUGAR_LOGGER_LEVEL'):
         set_level(os.environ['SUGAR_LOGGER_LEVEL'])
 
-    sys.stdout = StdoutCatcher()
+    if log_filename:
-    sys.stderr = StderrCatcher()
+        log_path = os.path.join(get_logs_dir(), log_filename + '.log')
-
+        log_file = open(log_path, 'w')
-    global _log_writer
-    _log_writer = log_writer
-    sys.excepthook = __exception_handler
-
-    service = LoggerManagerService()
-
-def cleanup():
-    logs_dir = _get_logs_dir()   
 
-    # File extension for backed up logfiles.
+        handler = logging.StreamHandler(log_file)
-    
+        logging.getLogger('').addHandler(handler)
-    file_suffix = int(time.time())
-    
-    # Absolute directory path where to store old logfiles.
-    # It will be created recursivly if it's not present.
-    
-    backup_dirpath = os.path.join(logs_dir, 'old')
-    
-    # How many versions shall be backed up of every logfile?
-    
-    num_backup_versions = 4
-    
-    # Make sure the backup location for old log files exists
-    
-    if not os.path.exists(backup_dirpath):
-        os.makedirs(backup_dirpath)
-    
-    # Iterate over every item in 'logs' directory
-    
-    for filename in os.listdir(logs_dir):
 
-        old_filepath = os.path.join(logs_dir, filename)
+        if redirect_io:
-        
+            os.dup2(log_file.fileno(), sys.stdout.fileno())
-        if os.path.isfile(old_filepath):
+            os.dup2(log_file.fileno(), sys.stderr.fileno())
-        
-            # Backup every file 
-            
-            new_filename = filename + '.' + str(file_suffix)
-            new_filepath = os.path.join(backup_dirpath, new_filename)  
-            os.rename(old_filepath, new_filepath)
-    
-    backup_map = {}
-    
-    # Temporarily map all backup logfiles
-    
-    for filename in os.listdir(backup_dirpath):
-    
-        # Remove the 'file_suffix' from the filename.
-        
-        end = filename.rfind(".")
-        key = filename[0:end].lower()        
-        key = key.replace(".", "_")
-        
-        if key not in backup_map:
-            backup_map[key] = []
-                        
-        backup_list = backup_map[key]
-        
-        backup_list.append( os.path.join(backup_dirpath, filename) )
-    
-    # Only keep 'num_backup_versions' versions of every logfile.
-    # Remove the others.
-    
-    for key in backup_map:
-        backup_list = backup_map[key]           
-        backup_list.sort()
-        backup_list.reverse()
-            
-        for i in range(num_backup_versions, len(backup_list)):
-            os.remove(backup_list[i])
-            

tests/graphics/iconcache.py

@@ -23,9 +23,6 @@ import gtk
 
 from sugar.graphics.icon import Icon
 from sugar.graphics.xocolor import XoColor
-from sugar import logger
-
-logger.start('iconcache')
 
 import common