Minor polishing launching code

7 files changed, 160 insertions, 233 deletions

diff --git a/TODO b/TODO
index 3bafa41..7be14ac 100644
--- a/TODO
+++ b/TODO
@@ -14,6 +14,7 @@
 - changed pulls should take into account accept_length
 - process client configuration in more general manner than client stats sharing
 - i18n activity.info's strings
+- return 304 Not Modified from Router when it makes sense
 
 1.0
 ===
diff --git a/sugar_network/local/mounts.py b/sugar_network/local/mounts.py
index a526840..c3c2745 100644
--- a/sugar_network/local/mounts.py
+++ b/sugar_network/local/mounts.py
@@ -263,9 +263,9 @@ class _ProxyCommands(object):
         return result
 
     def _checkin(self, guid):
-        for phase, __ in checkin(self.mountpoint, guid, 'activity'):
+        for event in checkin(self.mountpoint, guid, 'activity'):
             # TODO Publish checkin progress
-            if phase == 'failure':
+            if event['state'] == 'failure':
                 self.publish({
                     'event': 'alert',
                     'mountpoint': self.mountpoint,
diff --git a/sugar_network/zerosugar/config.py b/sugar_network/zerosugar/config.py
deleted file mode 100644
index 99a49f4..0000000
--- a/sugar_network/zerosugar/config.py
+++ /dev/null
@@ -1,24 +0,0 @@
-# Copyright (C) 2011-2012 Aleksey Lim
-#
-# This program is free software: you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation, either version 3 of the License, or
-# (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program.  If not, see <http://www.gnu.org/licenses/>.
-
-from zeroinstall.injector import config as injector_config
-
-
-class Config(injector_config.Config):
-
-    clients = []
-
-
-config = Config()
diff --git a/sugar_network/zerosugar/feeds.py b/sugar_network/zerosugar/feeds.py
index a1542b6..0fb5558 100644
--- a/sugar_network/zerosugar/feeds.py
+++ b/sugar_network/zerosugar/feeds.py
@@ -21,9 +21,10 @@ from zeroinstall.injector import model
 
 import sweets_recipe
 from active_toolkit import util, enforce
-from sugar_network.zerosugar.config import config
 
 
+clients = []
+
 _logger = logging.getLogger('zerosugar.feeds')
 
 
@@ -32,7 +33,7 @@ def read(context):
 
     feed_content = None
     client = None
-    for client in config.clients:
+    for client in clients:
         try:
             blob = client.get(['context', context, 'feed'], cmd='get_blob')
             enforce(blob and 'path' in blob, 'No feed for %r context', context)
diff --git a/sugar_network/zerosugar/injector.py b/sugar_network/zerosugar/injector.py
index 2189ab8..4a1080b 100644
--- a/sugar_network/zerosugar/injector.py
+++ b/sugar_network/zerosugar/injector.py
@@ -22,14 +22,16 @@ import signal
 import shutil
 import logging
 import threading
-from os.path import join, exists, basename, isabs
+from os.path import join, exists, basename
 
 from zeroinstall.injector import model
+from zeroinstall.injector.config import Config
+from zeroinstall.injector.driver import Driver
 from zeroinstall.injector.requirements import Requirements
 
 from sweets_recipe import Spec
-from sugar_network.zerosugar import solver
-from sugar_network.zerosugar.config import config
+from sugar_network.zerosugar import feeds
+from sugar_network.zerosugar.solution import Solution
 from sugar_network import local
 from sugar_network.toolkit import sugar
 from active_toolkit import coroutine, util, enforce
@@ -39,80 +41,6 @@ _logger = logging.getLogger('zerosugar.injector')
 _pipe = None
 
 
-class Pipe(object):
-
-    def __init__(self, pid, fd):
-        self._pid = pid
-        self._file = os.fdopen(fd)
-        self._stat = {}
-
-    def fileno(self):
-        return None if self._file is None else self._file.fileno()
-
-    def read(self):
-        if self._file is None:
-            return None
-
-        event = self._file.readline()
-        if event:
-            event = json.loads(event)
-            phase = event.pop('phase')
-            if not self._process_inernals(phase, event):
-                return phase, event
-            else:
-                return None, None
-
-        return self._finalize()
-
-    def __iter__(self):
-        if self._file is None:
-            return
-
-        try:
-            while True:
-                coroutine.select([self._file.fileno()], [], [])
-                event = self._file.readline()
-                if not event:
-                    break
-
-                event = json.loads(event)
-                phase = event.pop('phase')
-                if not self._process_inernals(phase, event):
-                    yield phase, event
-                if phase == 'exec':
-                    break
-
-            fin = self._finalize()
-            if fin is not None:
-                yield fin
-        finally:
-            self._finalize()
-
-    def _process_inernals(self, phase, props):
-        if phase == 'stat':
-            self._stat.update(props)
-            return True
-        elif phase == 'failure':
-            props.update(self._stat)
-
-    def _finalize(self):
-        if self._file is None:
-            return
-
-        try:
-            __, status = os.waitpid(self._pid, 0)
-        except OSError:
-            return None
-        finally:
-            self._file.close()
-            self._file = None
-
-        failure = _decode_exit_failure(status)
-        if failure:
-            self._stat['error'] = failure
-            return 'failure', self._stat
-
-
 def launch(mountpoint, context, command='activity', args=None):
     return _fork(_launch, mountpoint, context, command, args)
 
@@ -121,44 +49,9 @@ def checkin(mountpoint, context, command='activity'):
     return _fork(_checkin, mountpoint, context, command)
 
 
-def _fork(callback, mountpoint, context, *args):
-    fd_r, fd_w = os.pipe()
-
-    pid = os.fork()
-    if pid:
-        os.close(fd_w)
-        return Pipe(pid, fd_r)
-
-    from sugar_network import IPCClient
-
-    os.close(fd_r)
-    global _pipe
-    _pipe = fd_w
-
-    def thread_func():
-        log_path = _setup_logging(context)
-        _progress('stat', log_path=log_path, mountpoint=mountpoint,
-                context=context)
-
-        config.clients = [IPCClient(mountpoint='~')]
-        if mountpoint != '~':
-            config.clients.append(IPCClient(mountpoint=mountpoint))
-
-        try:
-            callback(mountpoint, context, *args)
-        except Exception, error:
-            util.exception(_logger)
-            _progress('failure', error=str(error))
-
-    # Avoid a mess with current thread coroutines
-    thread = threading.Thread(target=thread_func)
-    thread.start()
-    thread.join()
-
-    os.close(fd_w)
-    sys.stdout.flush()
-    sys.stderr.flush()
-    os._exit(0)
+def _progress(**event):
+    os.write(_pipe, json.dumps(event))
+    os.write(_pipe, '\n')
 
 
 def _launch(mountpoint, context, command, args):
@@ -170,7 +63,7 @@ def _launch(mountpoint, context, command, args):
     args = cmd.path.split() + args
 
     _logger.info('Executing %s: %s', solution.interface, args)
-    _progress('exec')
+    _progress(state='exec')
 
     if command == 'activity':
         _activity_env(solution.top, os.environ)
@@ -194,18 +87,36 @@ def _checkin(mountpoint, context, command):
         raise
 
 
-def _progress(phase, **kwargs):
-    kwargs['phase'] = phase
-    os.write(_pipe, json.dumps(kwargs))
-    os.write(_pipe, '\n')
+def _solve(req):
+    driver = Driver(Config(), req)
+
+    driver.solver.solve(req.interface_uri,
+                driver.target_arch, command_name=req.command)
+
+    result = Solution(driver.solver.selections, req)
+    result.details = dict((k.uri, v)
+            for k, v in (driver.solver.details or {}).items())
+    result.ready = driver.solver.ready
+
+    if not result.ready:
+        # pylint: disable-msg=W0212
+        failure_reason = driver.solver._failure_reason
+        if not failure_reason:
+            missed_ifaces = [iface.uri for iface, impl in
+                    driver.solver.selections.items() if impl is None]
+            failure_reason = 'Cannot find requireed implementations ' \
+                    'for %s' % ', '.join(missed_ifaces)
+        result.failure_reason = model.SafeException(failure_reason)
+
+    return result
 
 
 def _make(context, command):
     requirement = Requirements(context)
     requirement.command = command
 
-    _progress('analyze', progress=-1)
-    solution = solver.solve(requirement)
+    _progress(state='analyze')
+    solution = _solve(requirement)
     enforce(solution.ready, solution.failure_reason)
 
     for sel, __, __ in solution.walk():
@@ -217,7 +128,7 @@ def _make(context, command):
                 sel.interface)
 
         # TODO Per download progress
-        _progress('download', progress=-1)
+        _progress(state='download')
 
         impl = sel.client.get(['implementation', sel.id, 'data'],
                 cmd='get_blob')
@@ -229,8 +140,7 @@ def _make(context, command):
             impl_path = join(impl_path, dl.extract)
         sel.local_path = impl_path
 
-    if not isabs(solution.top.id):
-        _progress('stat', implementation=solution.top.id)
+    _progress(state='ready', session={'implementation': solution.top.id})
 
     return solution
 
@@ -309,3 +219,94 @@ def _decode_exit_failure(status):
         signum = os.WTERMSIG(status)
         failure = 'Undefined status with signal %s' % signum
     return failure
+
+
+def _fork(callback, mountpoint, context, *args):
+    fd_r, fd_w = os.pipe()
+
+    pid = os.fork()
+    if pid:
+        os.close(fd_w)
+        return _Pipe(pid, fd_r)
+
+    from sugar_network import IPCClient
+
+    os.close(fd_r)
+    global _pipe
+    _pipe = fd_w
+
+    def thread_func():
+        _progress(state='boot',
+                session={
+                    'log_path': _setup_logging(context),
+                    'mountpoint': mountpoint,
+                    'context': context,
+                    })
+
+        feeds.clients.append(IPCClient(mountpoint='~'))
+        if mountpoint != '~':
+            feeds.clients.append(IPCClient(mountpoint=mountpoint))
+
+        try:
+            callback(mountpoint, context, *args)
+        except Exception, error:
+            util.exception(_logger)
+            _progress(state='failure', error=str(error))
+
+    # Avoid a mess with current thread coroutines
+    thread = threading.Thread(target=thread_func)
+    thread.start()
+    thread.join()
+
+    os.close(fd_w)
+    sys.stdout.flush()
+    sys.stderr.flush()
+    os._exit(0)
+
+
+class _Pipe(object):
+
+    def __init__(self, pid, fd):
+        self._pid = pid
+        self._file = os.fdopen(fd)
+        self._session = {}
+
+    def fileno(self):
+        return None if self._file is None else self._file.fileno()
+
+    def read(self):
+        if self._file is None:
+            return None
+
+        event = self._file.readline()
+        if not event:
+            status = 0
+            try:
+                __, status = os.waitpid(self._pid, 0)
+            except OSError:
+                pass
+            failure = _decode_exit_failure(status)
+            if failure:
+                event = {'state': 'failure', 'error': failure}
+                event.update(self._session)
+                return event
+            else:
+                self._file.close()
+                self._file = None
+                return None
+
+        event = json.loads(event)
+        if 'session' in event:
+            self._session.update(event.pop('session'))
+        event.update(self._session)
+        return event
+
+    def __iter__(self):
+        if self._file is None:
+            return
+        while True:
+            coroutine.select([self._file.fileno()], [], [])
+            event = self.read()
+            if event is None:
+                break
+            yield event
diff --git a/sugar_network/zerosugar/solver.py b/sugar_network/zerosugar/solver.py
deleted file mode 100644
index 7c2f541..0000000
--- a/sugar_network/zerosugar/solver.py
+++ /dev/null
@@ -1,50 +0,0 @@
-# Copyright (C) 2011-2012 Aleksey Lim
-#
-# This program is free software: you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation, either version 3 of the License, or
-# (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program.  If not, see <http://www.gnu.org/licenses/>.
-
-import logging
-
-from zeroinstall.injector import model
-from zeroinstall.injector.driver import Driver
-
-from sugar_network.zerosugar.config import config
-from sugar_network.zerosugar.solution import Solution
-
-
-_logger = logging.getLogger('zerosugar.solver')
-
-
-def solve(req, record_details=False):
-    driver = Driver(config=config, requirements=req)
-    driver.solver.record_details = record_details
-
-    driver.solver.solve(req.interface_uri,
-                driver.target_arch, command_name=req.command)
-
-    result = Solution(driver.solver.selections, req)
-    result.details = dict((k.uri, v)
-            for k, v in (driver.solver.details or {}).items())
-    result.ready = driver.solver.ready
-
-    if not result.ready:
-        # pylint: disable-msg=W0212
-        failure_reason = driver.solver._failure_reason
-        if not failure_reason:
-            missed_ifaces = [iface.uri for iface, impl in
-                    driver.solver.selections.items() if impl is None]
-            failure_reason = 'Cannot find requireed implementations ' \
-                    'for %s' % ', '.join(missed_ifaces)
-        result.failure_reason = model.SafeException(failure_reason)
-
-    return result
diff --git a/tests/units/injector.py b/tests/units/injector.py
index ffc39be..43da2c2 100755
--- a/tests/units/injector.py
+++ b/tests/units/injector.py
@@ -39,14 +39,11 @@ class InjectorTest(tests.Test):
                 )
 
         pipe = checkin('/', context)
+        log_path = tests.tmpdir +  '/.sugar/default/logs/%s.log' % context
         self.assertEqual([
-            ('analyze', {'progress': -1}),
-            ('failure', {
-                'log_path': tests.tmpdir +  '/.sugar/default/logs/%s.log' % context,
-                'error': "Interface '%s' has no usable implementations" % context,
-                'mountpoint': '/',
-                'context': context,
-                }),
+            {'state': 'boot', 'mountpoint': '/', 'context': context, 'log_path': log_path},
+            {'state': 'analyze', 'mountpoint': '/', 'context': context, 'log_path': log_path},
+            {'state': 'failure', 'error': "Interface '%s' has no usable implementations" % context, 'mountpoint': '/', 'context': context, 'log_path': log_path},
             ],
             [i for i in pipe])
 
@@ -80,15 +77,12 @@ class InjectorTest(tests.Test):
         os.unlink('cache/context/%s/%s/feed.meta' % (context[:2], context))
 
         pipe = checkin('/', context)
+        log_path = tests.tmpdir +  '/.sugar/default/logs/%s_1.log' % context
         self.assertEqual([
-            ('analyze', {'progress': -1}),
-            ('download', {'progress': -1}),
-            ('failure', {
-                'log_path': tests.tmpdir +  '/.sugar/default/logs/%s_1.log' % context,
-                'error': 'Cannot download implementation',
-                'mountpoint': '/',
-                'context': context,
-                }),
+            {'state': 'boot', 'mountpoint': '/', 'context': context, 'log_path': log_path},
+            {'state': 'analyze', 'mountpoint': '/', 'context': context, 'log_path': log_path},
+            {'state': 'download', 'mountpoint': '/', 'context': context, 'log_path': log_path},
+            {'state': 'failure', 'error': 'Cannot download implementation', 'mountpoint': '/', 'context': context, 'log_path': log_path},
             ],
             [i for i in pipe])
         os.unlink('cache/implementation/%s/%s/data.meta' % (impl[:2], impl))
@@ -100,9 +94,12 @@ class InjectorTest(tests.Test):
         bundle.close()
 
         pipe = checkin('/', context)
+        log_path = tests.tmpdir +  '/.sugar/default/logs/%s_2.log' % context
         self.assertEqual([
-            ('analyze', {'progress': -1}),
-            ('download', {'progress': -1}),
+            {'state': 'boot', 'mountpoint': '/', 'context': context, 'log_path': log_path},
+            {'state': 'analyze', 'mountpoint': '/', 'context': context, 'log_path': log_path},
+            {'state': 'download', 'mountpoint': '/', 'context': context, 'log_path': log_path},
+            {'state': 'ready', 'implementation': impl, 'mountpoint': '/', 'context': context, 'log_path': log_path},
             ],
             [i for i in pipe])
 
@@ -163,17 +160,15 @@ class InjectorTest(tests.Test):
         bundle.close()
 
         pipe = launch('/', context)
+
+        log_path = tests.tmpdir +  '/.sugar/default/logs/%s.log' % context
         self.assertEqual([
-            ('analyze', {'progress': -1}),
-            ('download', {'progress': -1}),
-            ('exec', {}),
-            ('failure', {
-                'implementation': impl,
-                'log_path': tests.tmpdir +  '/.sugar/default/logs/%s.log' % context,
-                'error': 'Exited with status 1',
-                'mountpoint': '/',
-                'context': context,
-                }),
+            {'state': 'boot', 'mountpoint': '/', 'context': context, 'log_path': log_path},
+            {'state': 'analyze', 'mountpoint': '/', 'context': context, 'log_path': log_path},
+            {'state': 'download', 'mountpoint': '/', 'context': context, 'log_path': log_path},
+            {'state': 'ready', 'implementation': impl, 'mountpoint': '/', 'context': context, 'log_path': log_path},
+            {'state': 'exec', 'implementation': impl, 'mountpoint': '/', 'context': context, 'log_path': log_path},
+            {'state': 'failure', 'implementation': impl, 'error': 'Exited with status 1', 'mountpoint': '/', 'context': context, 'log_path': log_path},
             ],
             [i for i in pipe])
 
@@ -235,10 +230,13 @@ class InjectorTest(tests.Test):
         bundle.close()
 
         pipe = launch('/', context)
+        log_path = tests.tmpdir +  '/.sugar/default/logs/%s_1.log' % context
         self.assertEqual([
-            ('analyze', {'progress': -1}),
-            ('download', {'progress': -1}),
-            ('exec', {}),
+            {'state': 'boot', 'mountpoint': '/', 'context': context, 'log_path': log_path},
+            {'state': 'analyze', 'mountpoint': '/', 'context': context, 'log_path': log_path},
+            {'state': 'download', 'mountpoint': '/', 'context': context, 'log_path': log_path},
+            {'state': 'ready', 'implementation': impl_2, 'mountpoint': '/', 'context': context, 'log_path': log_path},
+            {'state': 'exec', 'implementation': impl_2, 'mountpoint': '/', 'context': context, 'log_path': log_path},
             ],
             [i for i in pipe])