Merge lp:~mvo/pkgme-devportal/add-configuration-for-library-overrides into lp:pkgme-devportal

[Michael Vogt]

Allow configuring overrides for the lib->package mapping.

This branch provides a way to override library dependencies. The background for this is bug #1025186. Often a library is provided by multiple alternative versions that can not be co-installed. Common cases are libgl1-mesa-{glx,swx11}. To catch them a simple override mechanism is provided.

[Jonathan Lange (jml)]

 * Really like splitting 'configuration' into a different module. Thanks.
 * ``write_config`` should probably be a Fixture that just sets the config. Not necessary to change in this branch, but if you're feeling
 * I would have thought that the config should be '<library>: <recommended package>'. I don't know if I'm right or you're right or even if the patch should be changed at this point in the absence of data. Probably OK to merge.
 * There really ought to be an example configuration that shows this
 * We're probably going to want a web control for this. Pinging LOSAs every time we want to add something is going to suck.

review: Approve

[James Westby (james-w)]

Hi,

I had the same instinct as jml with the '<library>: <recommended package>' config (making it a set of recommended packages to match the API though). It seems overly broad to do the search, and the dict would likely be quicker
if there were a lot of overrides.

I'm also unsure that a config is the right thing for this, becuase aren't the overrides going
to be the same for everyone everywhere? I guess not, and it's hard to know what will happen, so
let's go with this.

I'm for this landing quickly, but I think because changing the config would be tricky because
of backwards compatibility we should change it to do the dict method rather than the search
method before landing. What do you think?

Thanks,

James

[Michael Vogt (mvo)]

Thanks to both of you, I like the idea(s) and will change the config format. I'm unsure about the config too TBH, but it seems like guessing on some heuristics is not better so until we find a better solution having a override is probably ok(ish).

I will not address the "add-webinterface" for now as this should be a seperate branch :)

[Michael Vogt (mvo)]

I addressed the above points one (except for the webinterface). I hope this is in line with what you have in mind, I'm happy to refactor further if needed :)

review: Needs Resubmitting

[James Westby (james-w)]

review: Approve

[Canonical CA Tarmac (ca-tarmac)]

The attempt to merge lp:~mvo/pkgme-devportal/add-configuration-for-library-overrides into lp:pkgme-devportal failed. Below is the output from the failed tests.

Downloading/unpacking mock (from -r test-dependencies.txt (line 1))
  Running setup.py egg_info for package mock
    warning: no files found matching '*.png' under directory 'docs'
    warning: no files found matching '*.css' under directory 'docs'
    warning: no files found matching '*.html' under directory 'docs'
    warning: no files found matching '*.js' under directory 'docs'
Installing collected packages: mock
  Running setup.py install for mock
    warning: no files found matching '*.png' under directory 'docs'
    warning: no files found matching '*.css' under directory 'docs'
    warning: no files found matching '*.html' under directory 'docs'
    warning: no files found matching '*.js' under directory 'docs'
Successfully installed mock
Cleaning up...
Tests running...
======================================================================
ERROR: tests.test_binary.BinaryBackendTests.test_config_glue_lib_overrides
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/mnt/tarmac-tmp/tmp4oMKa_/devportalbinary/tests/test_binary.py", line 305, in test_config_glue_lib_overrides
    self.useFixture(ConfigFileFixture())
  File "/mnt/tarmac-tmp/tmp4oMKa_/virtualenv/lib/python2.6/site-packages/testtools-0.9.15-py2.6.egg/testtools/testcase.py", line 579, in useFixture
    fixture.setUp()
  File "devportalbinary/testing.py", line 126, in setUp
    with open(CONF_FILE, 'w') as f:
IOError: [Errno 2] No such file or directory: '~/.config/pkgme-binary/conf'
======================================================================
ERROR: discover.ModuleImportFailure.testing
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/mnt/tarmac-tmp/tmp4oMKa_/virtualenv/lib/python2.6/site-packages/discover.py", line 64, in testFailure
    raise exception
ImportError: Failed to import test module: testing
Traceback (most recent call last):
  File "/mnt/tarmac-tmp/tmp4oMKa_/virtualenv/lib/python2.6/site-packages/discover.py", line 288, in _find_tests
    module = self._get_module_from_name(name)
  File "/mnt/tarmac-tmp/tmp4oMKa_/virtualenv/lib/python2.6/site-packages/discover.py", line 266, in _get_module_from_name
    __import__(name)
  File "/mnt/tarmac-tmp/tmp4oMKa_/devportalbinary/testing.py", line 30, in <module>
    from .configuration import CONF_FILE
ValueError: Attempted relative import in non-package

Ran 115 tests in 0.384s
FAILED (failures=2)

Branched 118 revision(s).
/mnt/tarmac-tmp/tmp4oMKa_/virtualenv/lib/python2.6/site-packages/distribute-0.6.10-py2.6.egg/setuptools/command/bdist_egg.py:422: UnicodeWarning: Unicode equal comparison failed to convert both arguments to Unicode - interpreting them as being unequal
  symbols = dict.fromkeys(iter_symbols(code))
simplejson/_speedups.c: In function 'encoder_listencode_obj':
simplejson/_speedups.c:2269: warning: comparison of distinct pointer types lacks a cast
simplejson/_speedups.c:2269: warning: passing argument 2 of 'PyTyp...

[lp:~mvo/pkgme-devportal/add-configuration-for-library-overrides]

58. By Michael Vogt on 2012-07-16

devportalbinary/testing.py: do not use relative import (tarmac is on py2.6 apparently)

59. By Michael Vogt on 2012-07-16

devportalbinary/testing.py: add os.path.expanduser() to fix tarmac failure

[Michael Vogt (mvo)]

Setting back to approved to let tarmac try again.

=== modified file 'devportalbinary/binary.py'
--- devportalbinary/binary.py	2012-03-28 16:34:58 +0000
+++ devportalbinary/binary.py	2012-07-16 18:51:18 +0000
@@ -40,11 +40,11 @@
 from pkgme.errors import PkgmeError
 from pkgme.run_script import run_subprocess
 
+from devportalbinary.configuration import load_configuration
 from devportalbinary.database import PackageDatabase
 from devportalbinary.metadata import MetadataBackend
 
 
-
 # XXX: No idea about how icons will be there. Ignoring for now.
 
 
@@ -160,10 +160,18 @@
 
 
 def get_packages_for_libraries(library_names, arch):
+    conf = load_configuration()
+    lib_overrides = conf.options.lib_overrides_overrides
     db = PackageDatabase.create()
     deps = set()
     for lib in library_names:
-        new_deps = db.get_dependencies(lib)
+        # ensure that overrides always "trump" the existing set if they
+        # are found
+        if lib in lib_overrides:
+            new_deps = set([lib_overrides[lib]])
+        else:
+            new_deps = db.get_dependencies(lib)
+
         if not new_deps:
             raise UnknownDependency('Can\'t find dependency for "%s".' % lib)
         deps |= new_deps

=== added file 'devportalbinary/configuration.py'
--- devportalbinary/configuration.py	1970-01-01 00:00:00 +0000
+++ devportalbinary/configuration.py	2012-07-16 18:51:18 +0000
@@ -0,0 +1,69 @@
+import os
+
+try:
+    from configglue import glue
+except ImportError:
+    from configglue.pyschema import glue
+
+try:
+    from configglue.schema import (
+        Schema,
+        Section,
+        DictOption,
+        StringOption,
+    )
+except ImportError:
+    from configglue.pyschema.schema import (
+        Schema,
+        ConfigSection as Section,
+        DictConfigOption as DictOption,
+        StringConfigOption as StringOption,
+        )
+
+
+# XXX: 'pkgme-binary' is the historic name of this package.  Change this
+# to look first in ~/.config/pkgme-devportal/conf and then fall back to
+# this one.  Once production systems are updated to the new config, remove
+# the fallback.
+CONF_FILE = '~/.config/pkgme-binary/conf'
+
+
+class DevportalSchema(Schema):
+
+    # database
+    database = Section()
+    database.db_type = StringOption(default='sqlite',
+            help=('The database to use, one of "sqlite", or "postgres"'))
+    database.host = StringOption(
+            help='The database host (for postgres)')
+    database.port = StringOption(
+            help='The database port (for postgres)')
+    database.username = StringOption(
+            help='The database username (for postgres)')
+    database.password = StringOption(
+            help='The database password (for postgres)')
+    database.db_name = StringOption(
+            help='The database name (for postgres)')
+    database.path = StringOption(
+            help='The path to the database file (for sqlite)')
+    
+    scan_mode = StringOption(
+        help='To scan binary or source packages.')
+
+    # overrides
+    lib_overrides = Section()
+    overrides = { 'libasound.so.2': 'libasound2',
+                  'libgl.so.1': 'libgl1-mesa-glx',
+                }
+    lib_overrides.overrides = DictOption(
+        default=overrides,
+        help='mapping of library name to pkgname to force picking selected '
+             'dependencies')
+
+
+def load_configuration():
+    config_location = os.path.expanduser(CONF_FILE)
+    config_files = []
+    if os.path.exists(config_location):
+        config_files.append(config_location)
+    return glue.configglue(DevportalSchema, config_files)

=== modified file 'devportalbinary/database.py'
--- devportalbinary/database.py	2012-04-26 12:10:00 +0000
+++ devportalbinary/database.py	2012-07-16 18:51:18 +0000
@@ -8,23 +8,6 @@
 import tempfile
 
 from bzrlib import urlutils
-try:
-    from configglue import glue
-except ImportError:
-    from configglue.pyschema import glue
-
-try:
-    from configglue.schema import (
-        Schema,
-        Section,
-        StringOption,
-    )
-except ImportError:
-    from configglue.pyschema.schema import (
-        Schema,
-        ConfigSection as Section,
-        StringConfigOption as StringOption,
-        )
 from fixtures import (
     Fixture,
     TempDir,
@@ -37,6 +20,7 @@
 from pkgme.run_script import run_subprocess
 from storm.locals import create_database, Store
 
+from .configuration import load_configuration
 from .utils import download_file
 
 
@@ -274,47 +258,11 @@
         yield library, dependency
 
 
-class PackageDBSchema(Schema):
-
-    database = Section()
-    database.db_type = StringOption(default='sqlite',
-            help=('The database to use, one of "sqlite", or "postgres"'))
-    database.host = StringOption(
-            help='The database host (for postgres)')
-    database.port = StringOption(
-            help='The database port (for postgres)')
-    database.username = StringOption(
-            help='The database username (for postgres)')
-    database.password = StringOption(
-            help='The database password (for postgres)')
-    database.db_name = StringOption(
-            help='The database name (for postgres)')
-    database.path = StringOption(
-            help='The path to the database file (for sqlite)')
-
-    scan_mode = StringOption(
-        help='To scan binary or source packages.')
-
-
-def load_configuration():
-    config_location = os.path.expanduser(PackageDatabase.CONF_FILE)
-    config_files = []
-    if os.path.exists(config_location):
-        config_files.append(config_location)
-    return glue.configglue(PackageDBSchema, config_files)
-
-
 class PackageDatabase(object):
 
     SQLITE = 'sqlite'
     POSTGRES = 'postgres'
 
-    # XXX: 'pkgme-binary' is the historic name of this package.  Change this
-    # to look first in ~/.config/pkgme-devportal/conf and then fall back to
-    # this one.  Once production systems are updated to the new config, remove
-    # the fallback.
-    CONF_FILE = '~/.config/pkgme-binary/conf'
-
     def __init__(self, store):
         self._store = store
 

=== modified file 'devportalbinary/testing.py'
--- devportalbinary/testing.py	2012-07-05 07:11:28 +0000
+++ devportalbinary/testing.py	2012-07-16 18:51:18 +0000
@@ -27,6 +27,8 @@
 from devportalbinary.binary import MetadataBackend
 from devportalbinary.database import PackageDatabase
 
+from devportalbinary.configuration import CONF_FILE
+
 class IsChildPath(Matcher):
 
     def __init__(self, parent_path):
@@ -86,7 +88,7 @@
         self.useFixture(TempHomeDirFixture())
         tempdir = self.useFixture(TempDir())
         db_path = os.path.join(tempdir.path, 'test-db')
-        config_file_path = os.path.expanduser(PackageDatabase.CONF_FILE)
+        config_file_path = os.path.expanduser(CONF_FILE)
         os.makedirs(os.path.dirname(config_file_path))
         with open(config_file_path, 'w') as f:
             f.write('[database]\ndb_type=sqlite\npath=%s\n' % db_path)
@@ -107,6 +109,28 @@
         self.useFixture(EnvironmentVariableFixture("HOME", self.path))
 
 
+class ConfigFileFixture(TempHomeDirFixture):
+    """Create a lib_overrides config file.
+       
+    This also provides a example for the [lib_overrides] section syntax
+    """
+    
+    # for the test, needs to be in sync with the config file below
+    TEST_LIBS = { 'libasound.so.2' : 'libasound2',
+                  'libgl.so.1' : 'libgl1-mesa-glx',
+                  }
+
+    def setUp(self):
+        super(ConfigFileFixture, self).setUp()
+        os.makedirs(os.path.dirname(os.path.expanduser(CONF_FILE)))
+        with open(os.path.expanduser(CONF_FILE), 'w') as f:
+            f.write("""
+[lib_overrides]
+libasound.so.2 = libasound2
+libgl1.so.1 = libglx-mesa-dri
+""")
+
+
 class MetadataFixture(Fixture):
     """Create a metadata file to use.
 
@@ -188,3 +212,13 @@
         if path is None:
             path = self.useFixture(TempdirFixture()).path
         return self.BACKEND(path)
+
+
+def write_config(conf_file, conf_key, **kwargs):
+    config_path = os.path.expanduser(conf_file)
+    if not os.path.isdir(os.path.dirname(config_path)):
+        os.makedirs(os.path.dirname(config_path))
+    with open(config_path, 'w') as f:
+        f.write("[%s]\n" % conf_key)
+        for key, value in kwargs.items():
+            f.write("%s=%s\n" % (key, value))

=== modified file 'devportalbinary/tests/test_binary.py'
--- devportalbinary/tests/test_binary.py	2012-03-16 14:08:44 +0000
+++ devportalbinary/tests/test_binary.py	2012-07-16 18:51:18 +0000
@@ -1,18 +1,22 @@
 # Copyright 2011-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
+import json
 import os
 import shutil
 
 from testtools import TestCase
 from testtools.matchers import StartsWith
 
-from pkgme.testing import TempdirFixture
+from pkgme.testing import (
+    TempdirFixture,
+    )
 
 from devportalbinary.binary import (
     BinaryBackend,
     get_file_type,
     get_file_types,
+    get_packages_for_libraries,
     get_shared_library_dependencies,
     guess_dependencies,
     guess_executable,
@@ -22,16 +26,23 @@
     NoBinariesFound,
     UnknownDependency,
     )
+from devportalbinary.configuration import (
+    CONF_FILE,
+    load_configuration,
+    )
 from devportalbinary.metadata import (
     MetadataBackend,
     )
 from devportalbinary.testing import (
     BackendTests,
     BinaryFileFixture,
+    ConfigFileFixture,
     DatabaseFixture,
     get_test_data_dir_path,
     get_test_data_file_path,
     MetadataFixture,
+    TempHomeDirFixture,
+    write_config,
     )
 
 
@@ -288,3 +299,23 @@
         self.assertEqual(
             '/opt/%s/the-best' % (package_name,),
             backend.get_executable(package_name))
+    
+    def test_config_glue_lib_overrides(self):
+        # XXX: this needs to be in sync with testing.py:ConfigFileFixture
+        self.useFixture(ConfigFileFixture())
+        conf = load_configuration()
+        self.assertEqual(
+            conf.options.lib_overrides_overrides, ConfigFileFixture.TEST_LIBS)
+
+    def test_get_lib_overrides_for_packages_for_libraries(self):
+        """Test that the configuration file overrides the found
+           library dependencies 
+        """
+        db = self.useFixture(DatabaseFixture()).db
+        db.update_source_package('foo', [
+                [('libasound.so.2', 'libfoo')],
+                [('libasound.so.2', 'libbar')],
+                ])
+        self.assertEqual(
+            get_packages_for_libraries(["libasound.so.2"], "i386"),
+            set(["libasound2"]))

=== modified file 'devportalbinary/tests/test_database.py'
--- devportalbinary/tests/test_database.py	2012-01-20 16:17:16 +0000
+++ devportalbinary/tests/test_database.py	2012-07-16 18:51:18 +0000
@@ -17,7 +17,11 @@
     load_configuration,
     PackageDatabase,
     )
-from devportalbinary.testing import TempHomeDirFixture
+from devportalbinary.configuration import CONF_FILE
+from devportalbinary.testing import (
+    TempHomeDirFixture,
+    write_config,
+    )
 
 
 class ResultsIn(Matcher):
@@ -92,12 +96,7 @@
         self.assertEqual(deps, set())
 
     def write_database_config(self, **kwargs):
-        config_path = os.path.expanduser(PackageDatabase.CONF_FILE)
-        os.makedirs(os.path.dirname(config_path))
-        with open(config_path, 'w') as f:
-            f.write("[database]\n")
-            for key, value in kwargs.items():
-                f.write("%s=%s\n" % (key, value))
+        write_config(CONF_FILE, "database", **kwargs)
 
     def test_get_db_info_from_config_sqlite(self):
         other_tempdir = self.useFixture(TempDir())