Merge lp:~doxxx/bzr/bug939605-2.5 into lp:bzr/2.5

[Gordon Tyler]

Locate applications using the windows registry in addition to the PATH envvar

Backport of fix for bug 939605 to bzr 2.5 series.

[Martin Packman (gz)]

Changes look good.

+ # Internt Explorer is always registered in the App Path
+ self.assertTrue(osutils.find_executable_on_path('iexplore') is not None)

I wonder a little if one of those people who purge IE from their system will ever run our test suite. Not worrying about that seems correct, the test looks fine (though 'is not None' is perhaps not useful).

See earlier mp for initial reviews:

<https://code.launchpad.net/~doxxx/bzr/bug939605/+merge/94488>

review: Approve

[Martin Packman (gz)]

sent to pqm by email

[Gordon Tyler (doxxx)]

As I understand it, if you purge IE from Windows, it breaks in mysterious ways. That may have changed in more recent versions but it certainly was true a few years ago.

=== modified file 'bzrlib/mergetools.py'
--- bzrlib/mergetools.py	2011-12-19 13:23:58 +0000
+++ bzrlib/mergetools.py	2012-02-28 05:06:17 +0000
@@ -48,13 +48,13 @@
     cmd_list = cmdline.split(command_line)
     exe = cmd_list[0]
     if sys.platform == 'win32':
-        if os.path.isabs(exe):
-            base, ext = os.path.splitext(exe)
-            path_ext = [unicode(s.lower())
-                        for s in os.getenv('PATHEXT', '').split(os.pathsep)]
-            return os.path.exists(exe) and ext in path_ext
-        else:
-            return osutils.find_executable_on_path(exe) is not None
+        exe = _get_executable_path(exe)
+        if exe is None:
+            return False
+        base, ext = os.path.splitext(exe)
+        path_ext = [unicode(s.lower())
+                    for s in os.getenv('PATHEXT', '').split(os.pathsep)]
+        return os.path.exists(exe) and ext in path_ext
     else:
         return (os.access(exe, os.X_OK)
                 or osutils.find_executable_on_path(exe) is not None)
@@ -69,6 +69,9 @@
     if invoker is None:
         invoker = subprocess_invoker
     cmd_list = cmdline.split(command_line)
+    exe = _get_executable_path(cmd_list[0])
+    if exe is not None:
+        cmd_list[0] = exe
     args, tmp_file = _subst_filename(cmd_list, filename)
     def cleanup(retcode):
         if tmp_file is not None:
@@ -79,6 +82,12 @@
     return invoker(args[0], args[1:], cleanup)
 
 
+def _get_executable_path(exe):
+    if os.path.isabs(exe):
+        return exe
+    return osutils.find_executable_on_path(exe)
+
+
 def _subst_filename(args, filename):
     subst_names = {
         'base': filename + u'.BASE',

=== modified file 'bzrlib/osutils.py'
--- bzrlib/osutils.py	2012-02-01 12:31:53 +0000
+++ bzrlib/osutils.py	2012-02-28 05:06:17 +0000
@@ -2473,10 +2473,6 @@
     :param name: The base name of the executable.
     :return: The path to the executable found or None.
     """
-    path = os.environ.get('PATH')
-    if path is None:
-        return None
-    path = path.split(os.pathsep)
     if sys.platform == 'win32':
         exts = os.environ.get('PATHEXT', '').split(os.pathsep)
         exts = [ext.lower() for ext in exts]
@@ -2488,11 +2484,18 @@
             exts = [ext]
     else:
         exts = ['']
-    for ext in exts:
-        for d in path:
-            f = os.path.join(d, name) + ext
-            if os.access(f, os.X_OK):
-                return f
+    path = os.environ.get('PATH')
+    if path is not None:
+        path = path.split(os.pathsep)
+        for ext in exts:
+            for d in path:
+                f = os.path.join(d, name) + ext
+                if os.access(f, os.X_OK):
+                    return f
+    if sys.platform == 'win32':
+        app_path = win32utils.get_app_path(name)
+        if app_path != name:
+            return app_path
     return None
 
 

=== modified file 'bzrlib/tests/test_mergetools.py'
--- bzrlib/tests/test_mergetools.py	2011-06-30 21:00:38 +0000
+++ bzrlib/tests/test_mergetools.py	2012-02-28 05:06:17 +0000
@@ -81,11 +81,7 @@
         self.assertTrue(mergetools.check_availability(sys.executable))
 
     def test_exe_on_path(self):
-        if sys.platform == 'win32':
-            exe = 'cmd.exe'
-        else:
-            exe = 'sh'
-        self.assertTrue(mergetools.check_availability(exe))
+        self.assertTrue(mergetools.check_availability('python'))
 
     def test_nonexistent(self):
         self.assertFalse(mergetools.check_availability('DOES NOT EXIST'))
@@ -112,6 +108,19 @@
             ('test.txt.OTHER', 'other stuff'),
         ))
         
+    def test_invoke_expands_exe_path(self):
+        self.overrideEnv('PATH', os.path.dirname(sys.executable))
+        def dummy_invoker(exe, args, cleanup):
+            self._exe = exe
+            self._args = args
+            cleanup(0)
+            return 0
+        command = '%s {result}' % os.path.basename(sys.executable)
+        retcode = mergetools.invoke(command, 'test.txt', dummy_invoker)
+        self.assertEqual(0, retcode)
+        self.assertEqual(sys.executable, self._exe)
+        self.assertEqual(['test.txt'], self._args)
+        
     def test_success(self):
         def dummy_invoker(exe, args, cleanup):
             self._exe = exe

=== modified file 'bzrlib/tests/test_osutils.py'
--- bzrlib/tests/test_osutils.py	2012-02-01 12:32:26 +0000
+++ bzrlib/tests/test_osutils.py	2012-02-28 05:06:17 +0000
@@ -2225,6 +2225,14 @@
         self.assertTrue(
             osutils.find_executable_on_path('THIS SHOULD NOT EXIST') is None)
         self.assertTrue(osutils.find_executable_on_path('file.txt') is None)
+        
+    def test_windows_app_path(self):
+        if sys.platform != 'win32':
+            raise tests.TestSkipped('test requires win32')
+        # Override PATH env var so that exe can only be found on App Path
+        self.overrideEnv('PATH', '')
+        # Internt Explorer is always registered in the App Path
+        self.assertTrue(osutils.find_executable_on_path('iexplore') is not None)
 
     def test_other(self):
         if sys.platform == 'win32':

=== modified file 'doc/en/release-notes/bzr-2.5.txt'
--- doc/en/release-notes/bzr-2.5.txt	2012-02-26 19:21:01 +0000
+++ doc/en/release-notes/bzr-2.5.txt	2012-02-28 05:06:17 +0000
@@ -32,6 +32,9 @@
 .. Fixes for situations where bzr would previously crash or give incorrect
    or undesirable results.
 
+* Fixed merge tool availability checking and invocation to search the
+  Windows App Path registry in addition to the PATH. (Gordon Tyler, #939605)
+
 Documentation
 *************