Merge lp:~jml/pkgme-devportal/multiple-deps into lp:pkgme-devportal

[Jonathan Lange]

Get all the dependencies at once.

Kill get_dependencies and replace with get_multiple_dependencies, that
returns all of the dependencies.

[James Westby (james-w)]

review: Approve

=== modified file 'NEWS'
--- NEWS	2012-10-23 13:56:33 +0000
+++ NEWS	2012-10-26 14:45:28 +0000
@@ -2,6 +2,22 @@
 NEWS for pkgme-devportal
 ========================
 
+NEXT
+====
+
+Changes
+-------
+
+  * ``get_dependencies`` is no more.  Use ``get_multiple_dependencies``
+    instead.  (Jonathan Lange).
+
+Improvements
+------------
+
+ * Added ``get_multiple_dependencies``, which gets dependencies for multiple
+   libraries in a single operation.
+
+
 0.4.10 (2012-10-23)
 ===================
 

=== modified file 'devportalbinary/aptfile.py'
--- devportalbinary/aptfile.py	2012-10-11 13:57:59 +0000
+++ devportalbinary/aptfile.py	2012-10-26 14:45:28 +0000
@@ -207,13 +207,18 @@
                 for package in mapping[library]:
                     yield library, package, arch
 
-    def get_dependencies(self, lib, arch="i386"):
-        # do lazy downloading for now, we could also make this part
-        # of bin/fetch-symbols I guess(?)
+    def get_multiple_dependencies(self, library_names, arch):
+        """Get the binary packages that provide libraries.
+
+        :return: (deps, missing), where ``deps`` is a dict mapping library
+            names to sets of packages that provide them, and ``missing`` is a
+            set of library names for which no dependencies could be found.
+        """
         self._download_and_prepare_contents_file_if_needed(
             self.DISTROSERIES, arch)
         lib_to_pkgs = self._get_lib_to_pkgs_mapping(self.DISTROSERIES, arch)
-        return lib_to_pkgs.get(lib)
+        deps = ((lib, lib_to_pkgs.get(lib)) for lib in library_names)
+        return dict((lib, dep) for (lib, dep) in deps if dep)
 
     def close(self):
         pass

=== modified file 'devportalbinary/binary.py'
--- devportalbinary/binary.py	2012-10-11 10:15:33 +0000
+++ devportalbinary/binary.py	2012-10-26 14:45:28 +0000
@@ -80,6 +80,11 @@
 class UnknownDependency(PkgmeError):
     """Raised when we do not know which dependency to add."""
 
+    def __init__(self, libraries):
+        super(UnknownDependency, self).__init__(
+            "Can't find dependency for %s." %
+            ', '.join('"%s"' % lib for lib in libraries))
+
 
 class UnsupportedArchitecture(PkgmeError):
     """Raised when we find a unsupported architecture."""
@@ -276,29 +281,30 @@
 
 
 def get_packages_for_libraries(library_names, arch):
+    """Get packages that provide libraries.
+
+    :param library_names: A set of library names.
+    :param arch: A single architecture to get packages for.
+    :return: A set of package names that are dependencies that provide
+        the libraries.
+    """
     conf = load_configuration()
     lib_overrides = conf.libraries_overrides
+    needs_overrides = set(lib_overrides.keys()) & library_names
+    library_names -= needs_overrides
+    # ensure that overrides always "trump" the existing set if they
+    # are found
+    # XXX: we don't consider arch in overrides, do we need to?
+    # -- james_w
+    deps = set(lib_overrides[lib] for lib in needs_overrides)
     with closing(get_dependency_database()) as db:
-        deps = set()
-        for lib in library_names:
-            # ensure that overrides always "trump" the existing set if they
-            # are found
-            # XXX: we don't consider arch in overrides, do we need to?
-            # -- james_w
-            if lib in lib_overrides:
-                new_deps = set([lib_overrides[lib]])
-                trace.log(
-                    "found dependencies '%s' for lib '%s' via override" % (
-                        new_deps, lib))
-            else:
-                new_deps = db.get_dependencies(lib, arch)
-                trace.log(
-                    "found dependencies '%s' for lib '%s'" % (new_deps, lib))
-
-            if not new_deps:
-                raise UnknownDependency('Can\'t find dependency for "%s".' % lib)
-            deps |= new_deps
-        return deps
+        found = db.get_multiple_dependencies(library_names, arch)
+    missing = library_names - set(found.keys())
+    if missing:
+        raise UnknownDependency(missing)
+    for lib_deps in found.values():
+        deps |= lib_deps
+    return deps
 
 
 def find_bundled_libraries(path, libraries):
@@ -318,10 +324,7 @@
     return libraries_to_path
 
 
-def guess_embedded_libs_search_paths(
-    path,
-    library_finder=needed_libraries_from_objdump,
-    libraries_to_deps=get_packages_for_libraries):
+def guess_embedded_libs_search_paths(path):
     binaries = iter_binaries(path)
     libraries, arch = get_shared_library_dependencies(binaries)
     libraries_to_paths = find_bundled_libraries(path, libraries)
@@ -353,17 +356,14 @@
     return extra_targets
 
 
-def guess_dependencies(
-    path,
-    library_finder=needed_libraries_from_objdump,
-    libraries_to_deps=get_packages_for_libraries):
+def guess_dependencies(path):
     binaries = iter_binaries(path)
     libraries, arch = get_shared_library_dependencies(binaries)
     # find/filter out the embedded libs
     libraries_to_paths = find_bundled_libraries(path, libraries)
     libraries = set(libraries) - set(libraries_to_paths.keys())
     # go over the rest
-    deps = libraries_to_deps(libraries, arch)
+    deps = get_packages_for_libraries(libraries, arch)
     return deps, arch
 
 

=== modified file 'devportalbinary/database.py'
--- devportalbinary/database.py	2012-10-23 12:23:27 +0000
+++ devportalbinary/database.py	2012-10-26 14:45:28 +0000
@@ -19,6 +19,7 @@
     )
 from launchpadlib.launchpad import Launchpad
 from pkgme.run_script import run_subprocess
+from storm.expr import And, Column, Select, Table
 from storm.locals import create_database, Store
 from storm.uri import URI as StormURI
 
@@ -442,12 +443,30 @@
     def from_options(cls, options):
         return cls(cls.get_store_from_config(options))
 
-    def get_dependencies(self, library_name, arch='i386'):
-        """Get the binary packages that provide 'library_name'."""
-        result = self._store.execute(
-            "SELECT dependency FROM libdep WHERE library = ? AND architecture = ?",
-                  (unicode(library_name), unicode(arch)))
-        return set([dependency for [dependency] in result])
+    def _get_query(self, library_names, arch):
+        return Select(
+            [Column('library'), Column('dependency')],
+            And(Column('architecture') == arch,
+                Column('library').is_in(map(unicode, library_names))),
+            Table('libdep'))
+
+    def get_multiple_dependencies(self, library_names, arch):
+        """Get the binary packages that provide libraries.
+
+        :return: (deps, missing), where ``deps`` is a dict mapping library
+            names to sets of packages that provide them, and ``missing`` is a
+            set of library names for which no dependencies could be found.
+        """
+        arch = unicode(arch)
+        result = self._store.execute(self._get_query(library_names, arch))
+        found = {}
+        for row in result:
+            [lib, dependency] = row
+            if lib in found:
+                found[lib].add(dependency)
+            else:
+                found[lib] = set([dependency])
+        return found
 
     def insert_new_library(self, package_name, library_name,
                            dependency, arch):
@@ -499,9 +518,15 @@
     def from_options(cls, options):
         return cls(Client(options.database_base_url))
 
-    def get_dependencies(self, library_name, arch='i386'):
-        deps = self._client.get_binaries_for_libraries([library_name], [arch])
-        return set(deps[arch].get(library_name, []))
+    def get_multiple_dependencies(self, library_names, arch):
+        """Get the binary packages that provide libraries.
+
+        :return: (deps, missing), where ``deps`` is a dict mapping library
+            names to sets of packages that provide them, and ``missing`` is a
+            set of library names for which no dependencies could be found.
+        """
+        deps = self._client.get_binaries_for_libraries(library_names, [arch])
+        return dict((lib, set(dep)) for (lib, dep) in deps[arch].items())
 
     def close(self):
         pass

=== modified file 'devportalbinary/tests/test_aptfile.py'
--- devportalbinary/tests/test_aptfile.py	2012-09-17 15:31:20 +0000
+++ devportalbinary/tests/test_aptfile.py	2012-10-26 14:45:28 +0000
@@ -27,7 +27,8 @@
     def test_get_dependencies(self):
         """ Test that data from the fixture dependencies file works """
         self.assertEqual(
-            self.db.get_dependencies("libz.so.1"), set(["zlib1g"]))
+            self.db.get_multiple_dependencies(["libz.so.1"], 'i386'),
+            ({'libz.so.1': 'zlib1g'}, set()))
 
     @patch("urllib.urlretrieve")
     def test_lazy_downloading(self, mock_urlretrieve):
@@ -43,8 +44,8 @@
         db = AptFilePackageDatabase(tempdir.path)
         mock_urlretrieve.side_effect = _put_fixture_contents_file_in_place
         self.assertEqual(
-            db.get_dependencies("libfoo.so.2", arch="i386"),
-            set(["pkgfoo", "pkgbar"]))
+            db.get_multiple_dependencies(["libfoo.so.2"], arch="i386"),
+            ({'libfoo.so.2': set(["pkgfoo", "pkgbar"])}, set()))
         self.assertEqual(len(db._get_lib_to_pkgs_mapping("oneiric", "i386")), 1)
 
     def test_close(self):

=== modified file 'devportalbinary/tests/test_binary.py'
--- devportalbinary/tests/test_binary.py	2012-09-27 21:50:03 +0000
+++ devportalbinary/tests/test_binary.py	2012-10-26 14:45:28 +0000
@@ -378,7 +378,7 @@
         with closing(PackageDatabase(self.db_fixture.conn)) as db:
             db.update_package('foo', {'i386': {'libasound.so.2': 'libfoo'}})
         self.assertEqual(
-            get_packages_for_libraries(["libasound.so.2"], "i386"),
+            get_packages_for_libraries(set(["libasound.so.2"]), "i386"),
             set(["libasound2"]))
 
     def test_architecture(self):

=== modified file 'devportalbinary/tests/test_database.py'
--- devportalbinary/tests/test_database.py	2012-10-23 12:23:27 +0000
+++ devportalbinary/tests/test_database.py	2012-10-26 14:45:28 +0000
@@ -86,33 +86,33 @@
         db = self.get_package_db()
         db.insert_new_library('foo-src', 'libfoo.so.0', 'foo', 'i386')
         db.insert_new_library('foo-src', 'libfoo.so.0', 'bar', 'i386')
-        deps = db.get_dependencies('libfoo.so.0')
-        self.assertEqual(deps, set(['foo', 'bar']))
+        deps = db.get_multiple_dependencies(['libfoo.so.0'], 'i386')
+        self.assertEqual(deps, {'libfoo.so.0': set(['foo', 'bar'])})
 
     def test_get_dependencies(self):
         db = self.get_package_db()
         db.insert_new_library('foo-src', 'libfoo.so.0', 'foo', 'i386')
-        deps = db.get_dependencies('libfoo.so.0')
-        self.assertEqual(deps, set(['foo']))
+        deps = db.get_multiple_dependencies(['libfoo.so.0'], 'i386')
+        self.assertEqual(deps, {'libfoo.so.0': set(['foo'])})
 
     def test_respects_architecture(self):
         db = self.get_package_db()
         db.insert_new_library('foo-src', 'libfoo.so.0', 'foo', 'i386')
         db.insert_new_library('foo-src', 'libfoo.so.0', 'foo-amd64', 'amd64')
-        deps = db.get_dependencies('libfoo.so.0', arch='amd64')
-        self.assertEqual(deps, set(['foo-amd64']))
+        deps = db.get_multiple_dependencies(['libfoo.so.0'], arch='amd64')
+        self.assertEqual(deps, {'libfoo.so.0': set(['foo-amd64'])})
 
     def test_unknown_library(self):
         db = self.get_package_db()
-        deps = db.get_dependencies('libfoo.so.0')
-        self.assertEqual(deps, set())
+        deps = db.get_multiple_dependencies(['libfoo.so.0'], 'i386')
+        self.assertEqual(deps, {})
 
     def test_update_package(self):
         db = self.get_package_db()
         db.update_package(
             'foo', {'i386': {'libfoo.so.1': 'foo-bin'}})
-        deps = db.get_dependencies('libfoo.so.1')
-        self.assertEqual(deps, set(['foo-bin']))
+        deps = db.get_multiple_dependencies(['libfoo.so.1'], 'i386')
+        self.assertEqual(deps, {'libfoo.so.1': set(['foo-bin'])})
 
     def test_update_existing_package_no_libraries(self):
         db = self.get_package_db()
@@ -120,8 +120,8 @@
         # Run again, this time with no symbols, representing that a newer
         # version of the package no longer exports any libraries.
         db.update_package('foo', {'i386': {}})
-        deps = db.get_dependencies('libfoo.so.1')
-        self.assertEqual(deps, set())
+        deps = db.get_multiple_dependencies(['libfoo.so.1'], 'i386')
+        self.assertEqual(deps, {})
 
     def test_update_package_two_architectures(self):
         # If two architectures are updated separately then they
@@ -129,8 +129,8 @@
         db = self.get_package_db()
         db.update_package('foo', {'i386': {'libfoo.so.1': 'foo-bin'}})
         db.update_package('foo', {'amd64': {'libfoo.so.1': 'foo-bin-amd64'}})
-        deps = db.get_dependencies('libfoo.so.1', arch='i386')
-        self.assertEqual(deps, set(['foo-bin']))
+        deps = db.get_multiple_dependencies(['libfoo.so.1'], arch='i386')
+        self.assertEqual(deps, {'libfoo.so.1': set(['foo-bin'])})
 
     def test_close(self):
         # Test that we can close the package db.
@@ -459,8 +459,10 @@
         client = Client(double.base_url)
         wrapper = LibdepServiceClient(client)
         self.assertEqual(
-            set(['libfoo-bin']), wrapper.get_dependencies('libfoo', 'i386'))
-        self.assertEqual(set(), wrapper.get_dependencies('libbar', 'i386'))
+            {'libfoo': set(['libfoo-bin'])},
+            wrapper.get_multiple_dependencies(['libfoo'], 'i386'))
+        self.assertEqual(
+            {}, wrapper.get_multiple_dependencies(['libbar'], 'i386'))
 
     def test_has_close_method(self):
         client = Client('http://localhost/')