Merge lp:~michael.nelson/charm-helpers/configure-sources-single-source-null-key into lp:charm-helpers

[Michael Nelson]

fetch.configure_sources can take a single source and a null key.

This branch fixes two issues I came across while working on a charm:

1) fetch.configure_sources already accepted either a list of sources with list of keys (where individual keys could be None), but not a single source with a None key.

2) fetch.add_source() didn't pass sources starting with 'deb ' through, and apt-add-repository gives a different result depending on whether the source starts with 'deb ' in some situations (see example in the test doc string).

[James Page (james-page)]

review: Approve

=== modified file 'charmhelpers/fetch/__init__.py'
--- charmhelpers/fetch/__init__.py	2013-08-21 11:45:37 +0000
+++ charmhelpers/fetch/__init__.py	2013-10-11 09:24:08 +0000
@@ -80,8 +80,9 @@
 
 
 def add_source(source, key=None):
-    if ((source.startswith('ppa:') or
-         source.startswith('http:'))):
+    if (source.startswith('ppa:') or
+        source.startswith('http:') or
+        source.startswith('deb ')):
         subprocess.check_call(['add-apt-repository', '--yes', source])
     elif source.startswith('cloud:'):
         apt_install(filter_installed_packages(['ubuntu-cloud-keyring']),
@@ -118,8 +119,11 @@
     Note that 'null' (a.k.a. None) should not be quoted.
     """
     sources = safe_load(config(sources_var))
-    keys = safe_load(config(keys_var))
-    if isinstance(sources, basestring) and isinstance(keys, basestring):
+    keys = config(keys_var)
+    if keys is not None:
+        keys = safe_load(keys)
+    if isinstance(sources, basestring) and (
+            keys is None or isinstance(keys, basestring)):
         add_source(sources, keys)
     else:
         if not len(sources) == len(keys):

=== modified file 'tests/fetch/test_fetch.py'
--- tests/fetch/test_fetch.py	2013-08-21 15:45:53 +0000
+++ tests/fetch/test_fetch.py	2013-10-11 09:24:08 +0000
@@ -92,6 +92,28 @@
                                        '--yes',
                                        source])
 
+    @patch('subprocess.check_call')
+    def test_add_source_deb(self, check_call):
+        """add-apt-repository behaves differently when using the deb prefix.
+
+        $ add-apt-repository --yes "http://special.example.com/ubuntu precise-special main"
+        $ grep special /etc/apt/sources.list
+        deb http://special.example.com/ubuntu precise precise-special main
+        deb-src http://special.example.com/ubuntu precise precise-special main
+
+        $ add-apt-repository --yes "deb http://special.example.com/ubuntu precise-special main"
+        $ grep special /etc/apt/sources.list
+        deb http://special.example.com/ubuntu precise precise-special main
+        deb-src http://special.example.com/ubuntu precise precise-special main
+        deb http://special.example.com/ubuntu precise-special main
+        deb-src http://special.example.com/ubuntu precise-special main
+        """
+        source = "deb http://archive.ubuntu.com/ubuntu raring-backports main"
+        fetch.add_source(source=source)
+        check_call.assert_called_with(['add-apt-repository',
+                                       '--yes',
+                                       source])
+
     @patch.object(fetch, 'filter_installed_packages')
     @patch.object(fetch, 'apt_install')
     def test_add_source_cloud(self, apt_install, filter_pkg):
@@ -134,6 +156,13 @@
 
     @patch.object(fetch, 'config')
     @patch.object(fetch, 'add_source')
+    def test_configure_sources_single_source_no_key(self, add_source, config):
+        config.side_effect = ['source', None]
+        fetch.configure_sources()
+        add_source.assert_called_with('source', None)
+
+    @patch.object(fetch, 'config')
+    @patch.object(fetch, 'add_source')
     def test_configure_sources_multiple_sources(self, add_source, config):
         sources = ["sourcea", "sourceb"]
         keys = ["keya", None]