Merge lp:~brian-murray/apport/bug-1336062 into lp:~apport-hackers/apport/trunk

[Brian Murray]

This is pretty well documented in the attached bug report. I've chosen to test this by pretending that '/bin/true' is provided by mypackage. If we are using the cache from the sandbox then the test will pass, if not the test will fail with the following message.

 $ PYTHON=python3 test/run backend_apt_dpkg.test_get_file_package_uninstalled
Testing local source tree.
Running pep8...
Running pyflakes...
--- Testing backend_apt_dpkg.test_get_file_package_uninstalled ---
test_get_file_package_uninstalled (__main__.T)
get_file_package() on uninstalled packages. ... FAIL

======================================================================
FAIL: test_get_file_package_uninstalled (__main__.T)
get_file_package() on uninstalled packages.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test/test_backend_apt_dpkg.py", line 230, in test_get_file_package_uninstalled
    self.assertEqual(impl.get_file_package('/bin/true', True, cache_dir), 'mypackage')
AssertionError: 'coreutils' != 'mypackage'
- coreutils
+ mypackage

----------------------------------------------------------------------
Ran 1 test in 4.107s

FAILED (failures=1)

[Martin Pitt (pitti)]

Thanks Brian, nice catch! I un-indented the moved code block again (as "return" doesn't need an else branch) to make the diff easier to read.

review: Approve

=== modified file 'backends/packaging-apt-dpkg.py'
--- backends/packaging-apt-dpkg.py	2014-07-02 06:31:13 +0000
+++ backends/packaging-apt-dpkg.py	2014-07-10 21:01:17 +0000
@@ -336,38 +336,38 @@
         Also, release and arch can be set to a foreign release/architecture
         instead of the one from the current system.
         '''
-        # check if the file is a diversion
-        dpkg = subprocess.Popen(['dpkg-divert', '--list', file],
-                                stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-        out = dpkg.communicate()[0].decode('UTF-8')
-        if dpkg.returncode == 0 and out:
-            pkg = out.split()[-1]
-            if pkg != 'hardening-wrapper':
-                return pkg
-
-        fname = os.path.splitext(os.path.basename(file))[0].lower()
-
-        all_lists = []
-        likely_lists = []
-        for f in glob.glob('/var/lib/dpkg/info/*.list'):
-            p = os.path.splitext(os.path.basename(f))[0].lower().split(':')[0]
-            if p in fname or fname in p:
-                likely_lists.append(f)
-            else:
-                all_lists.append(f)
-
-        # first check the likely packages
-        match = self.__fgrep_files(file, likely_lists)
-        if not match:
-            match = self.__fgrep_files(file, all_lists)
-
-        if match:
-            return os.path.splitext(os.path.basename(match))[0].split(':')[0]
-
         if uninstalled:
             return self._search_contents(file, map_cachedir, release, arch)
         else:
-            return None
+            # check if the file is a diversion
+            dpkg = subprocess.Popen(['dpkg-divert', '--list', file],
+                                    stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+            out = dpkg.communicate()[0].decode('UTF-8')
+            if dpkg.returncode == 0 and out:
+                pkg = out.split()[-1]
+                if pkg != 'hardening-wrapper':
+                    return pkg
+
+            fname = os.path.splitext(os.path.basename(file))[0].lower()
+
+            all_lists = []
+            likely_lists = []
+            for f in glob.glob('/var/lib/dpkg/info/*.list'):
+                p = os.path.splitext(os.path.basename(f))[0].lower().split(':')[0]
+                if p in fname or fname in p:
+                    likely_lists.append(f)
+                else:
+                    all_lists.append(f)
+
+            # first check the likely packages
+            match = self.__fgrep_files(file, likely_lists)
+            if not match:
+                match = self.__fgrep_files(file, all_lists)
+
+            if match:
+                return os.path.splitext(os.path.basename(match))[0].split(':')[0]
+
+        return None
 
     @classmethod
     def get_system_architecture(klass):

=== modified file 'test/test_backend_apt_dpkg.py'
--- test/test_backend_apt_dpkg.py	2014-04-30 14:27:45 +0000
+++ test/test_backend_apt_dpkg.py	2014-07-10 21:01:17 +0000
@@ -187,6 +187,7 @@
 usr/bin/frobnicate                                      foo/frob
 usr/bin/frob                                            foo/frob-utils
 bo/gu/s                                                 na/mypackage
+bin/true                                                na/mypackage
 ''')
 
             # test Contents.gz for -updates pocket
@@ -226,6 +227,7 @@
 
             # valid cache, should not need to access the mirror
             impl.set_mirror('file:///foo/nonexisting')
+            self.assertEqual(impl.get_file_package('/bin/true', True, cache_dir), 'mypackage')
             self.assertEqual(impl.get_file_package('/bo/gu/s', True, cache_dir), 'mypackage')
             self.assertEqual(impl.get_file_package('/lib/libnew.so.5', True, cache_dir), 'libnew5')