Merge lp:~mikemc/ubuntuone-client/find-proxy-tunnel-fix-1083832 into lp:ubuntuone-client

[Mike McCracken]

- Use dirspec to find proxy tunnel exe, adding support for frozen osx. (LP: #1083832)

- Use dirspec to find proxy tunnel exe, adding support for frozen osx. (LP: #1083832)

Removed tests of old code that checks e.g. getting windows binary when frozen - dirspec tests cover those cases.

Tests pass on linux, and tests relevant to changes here pass on windows and darwin.

Tested IRL on darwin:
- start local squid, set system proxy settings to use it
- frozen app starts proxy-tunnel
- CONNECT to u1 servers shows up in squid access.log

Partially tested IRL on windows:
- starting control panel from source still correctly starts proxy-tunnel.

[dobey (dobey)]

+ print "IN START_PROCESS: tunnel_cmd is %r" % tunnel_cmd # MMCC DEBUG, 11/26 11:37:

review: Needs Fixing

[Mike McCracken (mikemc)]

Whoops, forgot to push the rev that removed those. Sorry about that.

[dobey (dobey)]

review: Approve

[Michał Karnicki (karni)]

Looks good :)

review: Approve

[Brian Curtin (brian.curtin)]

Cool.

review: Approve

=== modified file 'tests/syncdaemon/test_tunnel_runner.py'
--- tests/syncdaemon/test_tunnel_runner.py	2012-04-09 20:08:42 +0000
+++ tests/syncdaemon/test_tunnel_runner.py	2012-11-29 20:57:21 +0000
@@ -28,9 +28,6 @@
 # files in the program, then also delete it here.
 """Tests for the proxy tunnel runner."""
 
-import os
-import sys
-
 from twisted.internet import defer, error, reactor, task
 from twisted.trial.unittest import TestCase
 
@@ -63,7 +60,8 @@
     @defer.inlineCallbacks
     def test_executable_not_found(self):
         """The executable is not found anywhere."""
-        self.patch(tunnel_runner, "TUNNEL_EXECUTABLE", "this_does_not_exist")
+        self.patch(tunnel_runner, "get_tunnel_bin_cmd",
+                   lambda *args, **kwargs: ["this_does_not_exist"])
         tr = tunnel_runner.TunnelRunner(FAKE_HOST, FAKE_PORT)
         client = yield tr.get_client()
         self.assertEqual(client, reactor)
@@ -185,19 +183,3 @@
         client = yield self.tr.get_client()
         self.assertEqual(client.tunnel_port, FAKE_PORT)
         self.assertEqual(client.cookie, FAKE_COOKIE)
-
-    def test_frozen_path(self):
-        """Ensure the path is correct if sys is frozen."""
-        sys.frozen = "Yes"
-        self.addCleanup(delattr, sys, "frozen")
-        self.assertEqual(os.path.dirname(self.tr.get_process_path()),
-            os.path.dirname(sys.executable))
-
-    def test_start_process_win_devel(self):
-        """Test the windows devel case."""
-        fake_python = r"c:\python99\python.exe"
-        self.patch(sys, "platform", "win98")
-        self.patch(tunnel_runner.procutils, "which", lambda _: [fake_python])
-        tunnel_runner.TunnelRunner("fs-1.one.ubuntu.com", 443)
-        args, kwargs = self.spawned[1]
-        self.assertEqual(args[1], fake_python)
 
=== modified file 'ubuntuone/platform/constants.py'
--- ubuntuone/platform/constants.py	2012-04-09 20:07:05 +0000
+++ ubuntuone/platform/constants.py	2012-11-29 20:57:21 +0000
@@ -33,12 +33,7 @@
 
 import sys
 
-TUNNEL_EXECUTABLE = "ubuntuone-proxy-tunnel"
-
 if sys.platform == "win32":
     HASHQUEUE_DELAY = 3.0
-    if getattr(sys, "frozen", None) is not None:
-        # Only use the .exe suffix in windows+frozen
-        TUNNEL_EXECUTABLE += ".exe"
 else:
     HASHQUEUE_DELAY = 0.5
 
=== modified file 'ubuntuone/proxy/tunnel_client.py'
--- ubuntuone/proxy/tunnel_client.py	2012-06-01 04:11:49 +0000
+++ ubuntuone/proxy/tunnel_client.py	2012-11-29 20:57:21 +0000
@@ -42,7 +42,8 @@
 
 METHOD_LINE = "CONNECT %s:%d HTTP/1.0" + CRLF
 LOCALHOST = "127.0.0.1"
-logger = logging.getLogger("ubuntuone.Proxy.TunnelClient")
+
+logger = logging.getLogger("ubuntuone.SyncDaemon.TunnelClient")
 
 
 class TunnelClientProtocol(BaseTunnelProtocol):
@@ -166,6 +167,7 @@
     def finish_timeout(self):
         """Stop the timer from firing."""
         if self.timer and self.timer.active():
+            logger.debug("canceling timer before connection timeout")
             self.timer.cancel()
 
     def processExited(self, status):
@@ -173,6 +175,7 @@
         self.finish_timeout()
         logger.info("Tunnel process exit status %r.", status)
         if not self.client_d.called:
+            logger.debug("Tunnel process exited before TunnelClient created. Falling back to reactor")
             self.client_d.callback(reactor)
 
     def outReceived(self, data):
@@ -192,3 +195,6 @@
             logger.info("Tunnel process listening on port %r.", self.port)
             client = TunnelClient(LOCALHOST, self.port, self.cookie)
             self.client_d.callback(client)
+
+    def errReceived(self, data):
+        logger.debug("Got stderr from tunnel process: %r", data)
 
=== modified file 'ubuntuone/syncdaemon/tunnel_runner.py'
--- ubuntuone/syncdaemon/tunnel_runner.py	2012-04-09 20:08:42 +0000
+++ ubuntuone/syncdaemon/tunnel_runner.py	2012-11-29 20:57:21 +0000
@@ -29,14 +29,11 @@
 """Run the tunnel process and start a client, with a reactor as a fallback."""
 
 import logging
-import sys
-
-from os import path
 
 from twisted.internet import defer, reactor
-from twisted.python import procutils
 
-from ubuntuone.platform.constants import TUNNEL_EXECUTABLE
+from ubuntuone.clientdefs import LIBEXECDIR
+from ubuntuone.syncdaemon.utils import get_tunnel_bin_cmd
 
 logger = logging.getLogger("ubuntuone.SyncDaemon.TunnelRunner")
 
@@ -61,12 +58,10 @@
         """Start the tunnel process."""
         from ubuntuone.proxy.tunnel_client import TunnelProcessProtocol
         protocol = TunnelProcessProtocol(self.client_d)
-        process_path = self.get_process_path()
-        args = [process_path, host, str(port)]
-        if sys.platform.startswith("win") and not process_path.endswith("exe"):
-            python_exe = procutils.which("python.exe")
-            if python_exe:
-                args.insert(0, python_exe[0])
+        tunnel_cmd = get_tunnel_bin_cmd(extra_fallbacks=[LIBEXECDIR])
+
+        args = tunnel_cmd + [host, str(port)]
+
         self.process_transport = reactor.spawnProcess(protocol, args[0],
                                                       env=None, args=args)
         reactor.addSystemEventTrigger("before", "shutdown", self.stop)
@@ -77,23 +72,6 @@
         if self.process_transport.pid is not None:
             self.process_transport.signalProcess("KILL")
 
-    def get_process_path(self):
-        """Get the path to the tunnel process."""
-        # In a frozen setting, all binaries are in the same path
-        if getattr(sys, "frozen", None) is not None:
-            return path.join(path.dirname(
-                path.abspath(sys.executable)), TUNNEL_EXECUTABLE)
-
-        # This works when we are running from sources
-        filename = path.join(path.dirname(__file__), "..", "..", "bin",
-                             TUNNEL_EXECUTABLE)
-        if path.isfile(filename):
-            return filename
-
-        # Use the configured LIBEXECDIR
-        from ubuntuone.clientdefs import LIBEXECDIR
-        return path.join(LIBEXECDIR, TUNNEL_EXECUTABLE)
-
     def get_client(self):
         """A deferred with the reactor or a tunnel client."""
 
 
=== modified file 'ubuntuone/syncdaemon/utils.py'
--- ubuntuone/syncdaemon/utils.py	2012-10-15 22:05:57 +0000
+++ ubuntuone/syncdaemon/utils.py	2012-11-29 20:57:21 +0000
@@ -35,16 +35,20 @@
 
 from dirspec.utils import get_program_path
 
-DARWIN_APP_NAMES = {'ubuntuone-syncdaemon': 'UbuntuOne Syncdaemon.app'}
-
-
-def get_sd_bin_cmd():
-    """Get cmd + args to launch syncdaemon executable."""
+SYNCDAEMON_EXECUTABLE = 'ubuntuone-syncdaemon'
+TUNNEL_EXECUTABLE = 'ubuntuone-proxy-tunnel'
+
+DARWIN_APP_NAMES = {SYNCDAEMON_EXECUTABLE: 'UbuntuOne Syncdaemon.app',
+                    TUNNEL_EXECUTABLE: 'UbuntuOne Proxy Tunnel.app'}
+
+
+def _get_bin_cmd(exe_name, extra_fallbacks=[]):
+    """Get cmd+args to launch 'exe_name'."""
     syncdaemon_dir = os.path.dirname(__file__)
     ubuntuone_dir = os.path.dirname(syncdaemon_dir)
     tree_dir = os.path.dirname(ubuntuone_dir)
-    fallback_dirs = [os.path.join(tree_dir, 'bin')]
-    path = get_program_path('ubuntuone-syncdaemon',
+    fallback_dirs = [os.path.join(tree_dir, 'bin')] + extra_fallbacks
+    path = get_program_path(exe_name,
                             fallback_dirs=fallback_dirs,
                             app_names=DARWIN_APP_NAMES)
     cmd_args = [path]
@@ -58,3 +62,14 @@
             cmd_args.insert(0, procutils.which("python.exe")[0])
 
     return cmd_args
+
+
+def get_sd_bin_cmd():
+    """Get cmd + args to launch syncdaemon executable."""
+    return _get_bin_cmd(SYNCDAEMON_EXECUTABLE)
+
+
+def get_tunnel_bin_cmd(extra_fallbacks):
+    """Get cmd + args to launch proxy tunnel."""
+    return _get_bin_cmd(TUNNEL_EXECUTABLE,
+                        extra_fallbacks=extra_fallbacks)