Merge ~dbungert/curtin:command-apt-libapt-v4 into curtin:master

[Dan Bungert]

commands/apt: use python-apt for sources.list

Start using python{,3}-apt for sources.list handling.
generate_sources_list now operates in a pipeline-like model, where each
stage performs one specific transformation on the list of entries.

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:3e77e3070d5915f7a79b16518aae9cf5fb2087fb
https://jenkins.ubuntu.com/server/job/curtin-ci/175/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/175/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/175/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/175/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/175/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/175//rebuild

review: Needs Fixing (continuous-integration)

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:7d32d97412fad5e8cc50f21d7f64a51abd828e8e
https://jenkins.ubuntu.com/server/job/curtin-ci/176/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/176/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/176/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/176/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/176/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/176//rebuild

review: Needs Fixing (continuous-integration)

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:17251758df3653effc7236ddd07958f11e419936
https://jenkins.ubuntu.com/server/job/curtin-ci/177/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/177/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/177/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/177/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/177/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/177//rebuild

review: Needs Fixing (continuous-integration)

0cf5057... by Dan Bungert on 2021-09-10

tox: +sitepackages for py27-pylint

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:0cf505757aeb92c3a4e86cc31a70084bb03822b9
https://jenkins.ubuntu.com/server/job/curtin-ci/178/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/178/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/178/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/178/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/178/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/178//rebuild

review: Approve (continuous-integration)

[Michael Hudson-Doyle (mwhudson)]

Very nice. Just one suggestion, feel free to act on or ignore that and then merge, no further review required.

review: Approve

7193cf4... by Dan Bungert on 2021-09-20

commands/apt: uri update simplification

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:7193cf40c5f65bd52d615eb19199d2e468f13030
https://jenkins.ubuntu.com/server/job/curtin-ci/181/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/181/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/181/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/181/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/181/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/181//rebuild

review: Approve (continuous-integration)

diff --git a/curtin/commands/apt_config.py b/curtin/commands/apt_config.py
index e7d84c0..ef4a34c 100644
--- a/curtin/commands/apt_config.py
+++ b/curtin/commands/apt_config.py
@@ -11,6 +11,8 @@ import os
 import re
 import sys
 
+from aptsources.sourceslist import SourceEntry
+
 from curtin.log import LOG
 from curtin import (config, distro, gpg, paths, util)
 
@@ -211,22 +213,38 @@ def rename_apt_lists(new_mirrors, target=None):
                 LOG.warn("Failed to rename apt list:", exc_info=True)
 
 
-def mirror_to_placeholder(tmpl, mirror, placeholder):
-    """ mirror_to_placeholder
-        replace the specified mirror in a template with a placeholder string
-        Checks for existance of the expected mirror and warns if not found
-    """
-    if mirror not in tmpl:
-        if mirror.endswith("/") and mirror[:-1] in tmpl:
-            LOG.debug("mirror_to_placeholder: '%s' did not exist in tmpl, "
-                      "did without a trailing /.  Accomodating.", mirror)
-            mirror = mirror[:-1]
-        else:
-            LOG.warn("Expected mirror '%s' not found in: %s", mirror, tmpl)
-    return tmpl.replace(mirror, placeholder)
+def update_default_mirrors(entries, mirrors, target):
+    """replace existing default repos with the configured mirror"""
+
+    defaults = get_default_mirrors(distro.get_architecture(target))
+    mirrors_replacement = {
+        defaults['PRIMARY']: mirrors["MIRROR"],
+        defaults['SECURITY']: mirrors["SECURITY"],
+    }
+
+    # allow original file URIs without the trailing slash to match mirror
+    # specifications that have it
+    noslash = {}
+    for key in mirrors_replacement.keys():
+        if key[-1] == '/':
+            noslash[key[:-1]] = mirrors_replacement[key]
+
+    mirrors_replacement.update(noslash)
+
+    for entry in entries:
+        entry.uri = mirrors_replacement.get(entry.uri, entry.uri)
+    return entries
+
 
+def update_mirrors(entries, mirrors):
+    """perform template replacement of mirror placeholders with configured
+       values"""
+    for entry in entries:
+        entry.uri = util.render_string(entry.uri, mirrors)
+    return entries
 
-def map_known_suites(suite):
+
+def map_known_suites(suite, release):
     """there are a few default names which will be auto-extended.
        This comes at the inability to use those names literally as suites,
        but on the other hand increases readability of the cfg quite a lot"""
@@ -236,10 +254,10 @@ def map_known_suites(suite):
                'proposed': '$RELEASE-proposed',
                'release': '$RELEASE'}
     try:
-        retsuite = mapping[suite]
+        template_suite = mapping[suite]
     except KeyError:
-        retsuite = suite
-    return retsuite
+        template_suite = suite
+    return util.render_string(template_suite, {'RELEASE': release})
 
 
 def disable_suites(disabled, src, release):
@@ -248,36 +266,30 @@ def disable_suites(disabled, src, release):
     if not disabled:
         return src
 
-    retsrc = src
+    suites_to_disable = []
     for suite in disabled:
-        suite = map_known_suites(suite)
-        releasesuite = util.render_string(suite, {'RELEASE': release})
-        LOG.debug("Disabling suite %s as %s", suite, releasesuite)
+        release_suite = map_known_suites(suite, release)
+        LOG.debug("Disabling suite %s as %s", suite, release_suite)
+        suites_to_disable.append(release_suite)
+
+    output = []
+    for entry in src:
+        if entry.dist in suites_to_disable and not entry.disabled:
+            # handle commenting ourselves - it handles lines with
+            # options better
+            entry = SourceEntry('# ' + str(entry))
+        output.append(entry)
+    return output
 
-        newsrc = ""
-        for line in retsrc.splitlines(True):
-            if line.startswith("#"):
-                newsrc += line
-                continue
 
-            # sources.list allow options in cols[1] which can have spaces
-            # so the actual suite can be [2] or later. example:
-            # deb [ arch=amd64,armel k=v ] http://example.com/debian
-            cols = line.split()
-            if len(cols) > 1:
-                pcol = 2
-                if cols[1].startswith("["):
-                    for col in cols[1:]:
-                        pcol += 1
-                        if col.endswith("]"):
-                            break
+def update_dist(entries, release):
+    for entry in entries:
+        entry.dist = util.render_string(entry.dist, {'RELEASE': release})
+    return entries
 
-                if cols[pcol] == releasesuite:
-                    line = '# suite disabled by curtin: %s' % line
-            newsrc += line
-        retsrc = newsrc
 
-    return retsrc
+def entries_to_str(entries):
+    return ''.join([str(entry) + '\n' for entry in entries])
 
 
 def generate_sources_list(cfg, release, mirrors, target=None):
@@ -285,34 +297,31 @@ def generate_sources_list(cfg, release, mirrors, target=None):
         create a source.list file based on a custom or default template
         by replacing mirrors and release in the template
     """
-    default_mirrors = get_default_mirrors(distro.get_architecture(target))
     aptsrc = "/etc/apt/sources.list"
-    params = {'RELEASE': release}
-    for k in mirrors:
-        params[k] = mirrors[k]
 
     tmpl = cfg.get('sources_list', None)
+    from_file = False
     if tmpl is None:
         LOG.info("No custom template provided, fall back to modify"
                  "mirrors in %s on the target system", aptsrc)
         tmpl = util.load_file(paths.target_path(target, aptsrc))
-        # Strategy if no custom template was provided:
-        # - Only replacing mirrors
-        # - no reason to replace "release" as it is from target anyway
-        # - The less we depend upon, the more stable this is against changes
-        # - warn if expected original content wasn't found
-        tmpl = mirror_to_placeholder(tmpl, default_mirrors['PRIMARY'],
-                                     "$MIRROR")
-        tmpl = mirror_to_placeholder(tmpl, default_mirrors['SECURITY'],
-                                     "$SECURITY")
+        from_file = True
+
+    entries = [SourceEntry(line) for line in tmpl.splitlines(True)]
+    if from_file:
+        # when loading from an existing file, we also replace default
+        # URIs with configured mirrors
+        entries = update_default_mirrors(entries, mirrors, target)
+
+    entries = update_mirrors(entries, mirrors)
+    entries = update_dist(entries, release)
+    entries = disable_suites(cfg.get('disable_suites'), entries, release)
+    output = entries_to_str(entries)
 
     orig = paths.target_path(target, aptsrc)
     if os.path.exists(orig):
         os.rename(orig, orig + ".curtin.old")
-
-    rendered = util.render_string(tmpl, params)
-    disabled = disable_suites(cfg.get('disable_suites'), rendered, release)
-    util.write_file(paths.target_path(target, aptsrc), disabled, mode=0o644)
+    util.write_file(paths.target_path(target, aptsrc), output, mode=0o644)
 
 
 def apply_preserve_sources_list(target):
diff --git a/tests/unittests/test_apt_source.py b/tests/unittests/test_apt_source.py
index 6556399..01e70ad 100644
--- a/tests/unittests/test_apt_source.py
+++ b/tests/unittests/test_apt_source.py
@@ -12,6 +12,8 @@ import socket
 import mock
 from mock import call
 
+from aptsources.sourceslist import SourceEntry
+
 from curtin import distro
 from curtin import gpg
 from curtin import util
@@ -63,6 +65,18 @@ class PseudoChrootableTarget(util.ChrootableTarget):
 ChrootableTargetStr = "curtin.commands.apt_config.util.ChrootableTarget"
 
 
+def entryify(data):
+    return [SourceEntry(line) for line in data.splitlines()]
+
+
+def lineify(entries):
+    out = apt_config.entries_to_str(entries)
+    # the tests are written without the trailing newline,
+    # but we don't want to remove multiple of them
+    out = out[:-1] if len(out) > 0 and out[-1] == '\n' else out
+    return out
+
+
 class TestAptSourceConfig(CiTestCase):
     """ TestAptSourceConfig
     Main Class to test apt configs
@@ -77,6 +91,7 @@ class TestAptSourceConfig(CiTestCase):
         self.matcher = re.compile(ADD_APT_REPO_MATCH).search
         self.add_patch('curtin.util.subp', 'm_subp')
         self.m_subp.return_value = ('s390x', '')
+        self.target = self.tmp_dir()
 
     @staticmethod
     def _add_apt_sources(*args, **kwargs):
@@ -758,6 +773,7 @@ class TestAptSourceConfig(CiTestCase):
     def test_disable_suites(self):
         """test_disable_suites - disable_suites with many configurations"""
         release = "xenial"
+
         orig = """deb http://ubuntu.com//ubuntu xenial main
 deb http://ubuntu.com//ubuntu xenial-updates main
 deb http://ubuntu.com//ubuntu xenial-security main
@@ -771,39 +787,38 @@ deb http://ubuntu.com//ubuntu xenial-updates main
 deb http://ubuntu.com//ubuntu xenial-security main
 deb-src http://ubuntu.com//ubuntu universe multiverse
 deb http://ubuntu.com/ubuntu/ xenial-proposed main"""
-        result = apt_config.disable_suites(disabled, orig, release)
-        self.assertEqual(expect, result)
+        result = apt_config.disable_suites(disabled, entryify(orig), release)
+        self.assertEqual(expect, lineify(result))
 
         # single disable release suite
         disabled = ["$RELEASE"]
-        expect = """\
-# suite disabled by curtin: deb http://ubuntu.com//ubuntu xenial main
+        expect = """# deb http://ubuntu.com//ubuntu xenial main
 deb http://ubuntu.com//ubuntu xenial-updates main
 deb http://ubuntu.com//ubuntu xenial-security main
 deb-src http://ubuntu.com//ubuntu universe multiverse
 deb http://ubuntu.com/ubuntu/ xenial-proposed main"""
-        result = apt_config.disable_suites(disabled, orig, release)
-        self.assertEqual(expect, result)
+        result = apt_config.disable_suites(disabled, entryify(orig), release)
+        self.assertEqual(expect, lineify(result))
 
         # single disable other suite
         disabled = ["$RELEASE-updates"]
         expect = """deb http://ubuntu.com//ubuntu xenial main
-# suite disabled by curtin: deb http://ubuntu.com//ubuntu xenial-updates main
+# deb http://ubuntu.com//ubuntu xenial-updates main
 deb http://ubuntu.com//ubuntu xenial-security main
 deb-src http://ubuntu.com//ubuntu universe multiverse
 deb http://ubuntu.com/ubuntu/ xenial-proposed main"""
-        result = apt_config.disable_suites(disabled, orig, release)
-        self.assertEqual(expect, result)
+        result = apt_config.disable_suites(disabled, entryify(orig), release)
+        self.assertEqual(expect, lineify(result))
 
         # multi disable
         disabled = ["$RELEASE-updates", "$RELEASE-security"]
         expect = """deb http://ubuntu.com//ubuntu xenial main
-# suite disabled by curtin: deb http://ubuntu.com//ubuntu xenial-updates main
-# suite disabled by curtin: deb http://ubuntu.com//ubuntu xenial-security main
+# deb http://ubuntu.com//ubuntu xenial-updates main
+# deb http://ubuntu.com//ubuntu xenial-security main
 deb-src http://ubuntu.com//ubuntu universe multiverse
 deb http://ubuntu.com/ubuntu/ xenial-proposed main"""
-        result = apt_config.disable_suites(disabled, orig, release)
-        self.assertEqual(expect, result)
+        result = apt_config.disable_suites(disabled, entryify(orig), release)
+        self.assertEqual(expect, lineify(result))
 
         # multi line disable (same suite multiple times in input)
         disabled = ["$RELEASE-updates", "$RELEASE-security"]
@@ -815,14 +830,14 @@ deb http://UBUNTU.com//ubuntu xenial-updates main
 deb http://UBUNTU.COM//ubuntu xenial-updates main
 deb http://ubuntu.com/ubuntu/ xenial-proposed main"""
         expect = """deb http://ubuntu.com//ubuntu xenial main
-# suite disabled by curtin: deb http://ubuntu.com//ubuntu xenial-updates main
-# suite disabled by curtin: deb http://ubuntu.com//ubuntu xenial-security main
+# deb http://ubuntu.com//ubuntu xenial-updates main
+# deb http://ubuntu.com//ubuntu xenial-security main
 deb-src http://ubuntu.com//ubuntu universe multiverse
-# suite disabled by curtin: deb http://UBUNTU.com//ubuntu xenial-updates main
-# suite disabled by curtin: deb http://UBUNTU.COM//ubuntu xenial-updates main
+# deb http://UBUNTU.com//ubuntu xenial-updates main
+# deb http://UBUNTU.COM//ubuntu xenial-updates main
 deb http://ubuntu.com/ubuntu/ xenial-proposed main"""
-        result = apt_config.disable_suites(disabled, orig, release)
-        self.assertEqual(expect, result)
+        result = apt_config.disable_suites(disabled, entryify(orig), release)
+        self.assertEqual(expect, lineify(result))
 
         # comment in input
         disabled = ["$RELEASE-updates", "$RELEASE-security"]
@@ -835,15 +850,15 @@ deb-src http://ubuntu.com//ubuntu universe multiverse
 deb http://UBUNTU.COM//ubuntu xenial-updates main
 deb http://ubuntu.com/ubuntu/ xenial-proposed main"""
         expect = """deb http://ubuntu.com//ubuntu xenial main
-# suite disabled by curtin: deb http://ubuntu.com//ubuntu xenial-updates main
-# suite disabled by curtin: deb http://ubuntu.com//ubuntu xenial-security main
+# deb http://ubuntu.com//ubuntu xenial-updates main
+# deb http://ubuntu.com//ubuntu xenial-security main
 deb-src http://ubuntu.com//ubuntu universe multiverse
 #foo
-#deb http://UBUNTU.com//ubuntu xenial-updates main
-# suite disabled by curtin: deb http://UBUNTU.COM//ubuntu xenial-updates main
+# deb http://UBUNTU.com//ubuntu xenial-updates main
+# deb http://UBUNTU.COM//ubuntu xenial-updates main
 deb http://ubuntu.com/ubuntu/ xenial-proposed main"""
-        result = apt_config.disable_suites(disabled, orig, release)
-        self.assertEqual(expect, result)
+        result = apt_config.disable_suites(disabled, entryify(orig), release)
+        self.assertEqual(expect, lineify(result))
 
         # single disable custom suite
         disabled = ["foobar"]
@@ -854,9 +869,9 @@ deb http://ubuntu.com/ubuntu/ foobar main"""
         expect = """deb http://ubuntu.com//ubuntu xenial main
 deb http://ubuntu.com//ubuntu xenial-updates main
 deb http://ubuntu.com//ubuntu xenial-security main
-# suite disabled by curtin: deb http://ubuntu.com/ubuntu/ foobar main"""
-        result = apt_config.disable_suites(disabled, orig, release)
-        self.assertEqual(expect, result)
+# deb http://ubuntu.com/ubuntu/ foobar main"""
+        result = apt_config.disable_suites(disabled, entryify(orig), release)
+        self.assertEqual(expect, lineify(result))
 
         # single disable non existing suite
         disabled = ["foobar"]
@@ -868,8 +883,8 @@ deb http://ubuntu.com/ubuntu/ notfoobar main"""
 deb http://ubuntu.com//ubuntu xenial-updates main
 deb http://ubuntu.com//ubuntu xenial-security main
 deb http://ubuntu.com/ubuntu/ notfoobar main"""
-        result = apt_config.disable_suites(disabled, orig, release)
-        self.assertEqual(expect, result)
+        result = apt_config.disable_suites(disabled, entryify(orig), release)
+        self.assertEqual(expect, lineify(result))
 
         # single disable suite with option
         disabled = ["$RELEASE-updates"]
@@ -879,12 +894,12 @@ deb http://ubuntu.com//ubuntu xenial-security main
 deb-src http://ubuntu.com//ubuntu universe multiverse
 deb http://ubuntu.com/ubuntu/ xenial-proposed main"""
         expect = """deb http://ubuntu.com//ubuntu xenial main
-# suite disabled by curtin: deb [a=b] http://ubu.com//ubu xenial-updates main
+# deb [a=b] http://ubu.com//ubu xenial-updates main
 deb http://ubuntu.com//ubuntu xenial-security main
 deb-src http://ubuntu.com//ubuntu universe multiverse
 deb http://ubuntu.com/ubuntu/ xenial-proposed main"""
-        result = apt_config.disable_suites(disabled, orig, release)
-        self.assertEqual(expect, result)
+        result = apt_config.disable_suites(disabled, entryify(orig), release)
+        self.assertEqual(expect, lineify(result))
 
         # single disable suite with more options and auto $RELEASE expansion
         disabled = ["updates"]
@@ -894,13 +909,12 @@ deb http://ubuntu.com//ubuntu xenial-security main
 deb-src http://ubuntu.com//ubuntu universe multiverse
 deb http://ubuntu.com/ubuntu/ xenial-proposed main"""
         expect = """deb http://ubuntu.com//ubuntu xenial main
-# suite disabled by curtin: deb [a=b c=d] \
-http://ubu.com//ubu xenial-updates main
+# deb [a=b c=d] http://ubu.com//ubu xenial-updates main
 deb http://ubuntu.com//ubuntu xenial-security main
 deb-src http://ubuntu.com//ubuntu universe multiverse
 deb http://ubuntu.com/ubuntu/ xenial-proposed main"""
-        result = apt_config.disable_suites(disabled, orig, release)
-        self.assertEqual(expect, result)
+        result = apt_config.disable_suites(disabled, entryify(orig), release)
+        self.assertEqual(expect, lineify(result))
 
         # single disable suite while options at others
         disabled = ["$RELEASE-security"]
@@ -911,25 +925,74 @@ deb-src http://ubuntu.com//ubuntu universe multiverse
 deb http://ubuntu.com/ubuntu/ xenial-proposed main"""
         expect = """deb http://ubuntu.com//ubuntu xenial main
 deb [arch=foo] http://ubuntu.com//ubuntu xenial-updates main
-# suite disabled by curtin: deb http://ubuntu.com//ubuntu xenial-security main
+# deb http://ubuntu.com//ubuntu xenial-security main
 deb-src http://ubuntu.com//ubuntu universe multiverse
 deb http://ubuntu.com/ubuntu/ xenial-proposed main"""
-        result = apt_config.disable_suites(disabled, orig, release)
-        self.assertEqual(expect, result)
+        result = apt_config.disable_suites(disabled, entryify(orig), release)
+        self.assertEqual(expect, lineify(result))
 
     def test_disable_suites_blank_lines(self):
         """test_disable_suites_blank_lines - ensure blank lines allowed"""
-        lines = ["deb %(repo)s %(rel)s main universe",
-                 "",
-                 "deb %(repo)s %(rel)s-updates main universe",
-                 "   # random comment",
-                 "#comment here",
-                 ""]
         rel = "trusty"
-        repo = 'http://example.com/mirrors/ubuntu'
-        orig = "\n".join(lines) % {'repo': repo, 'rel': rel}
-        self.assertEqual(
-            orig, apt_config.disable_suites(["proposed"], orig, rel))
+
+        orig = """
+deb http://example.com/mirrors/ubuntu trusty main universe
+
+deb http://example.com/mirrors/ubuntu trusty-updates main universe
+
+deb http://example.com/mirrors/ubuntu trusty-proposed main universe
+
+#comment here"""
+        expect = """
+deb http://example.com/mirrors/ubuntu trusty main universe
+
+deb http://example.com/mirrors/ubuntu trusty-updates main universe
+
+# deb http://example.com/mirrors/ubuntu trusty-proposed main universe
+
+#comment here"""
+        disabled = ["proposed"]
+        result = apt_config.disable_suites(disabled, entryify(orig), rel)
+        self.assertEqual(expect, lineify(result))
+
+    @mock.patch("curtin.util.write_file")
+    @mock.patch("curtin.distro.get_architecture")
+    def test_generate_with_options(self, get_arch, write_file):
+        get_arch.return_value = "amd64"
+        orig = """deb http://ubuntu.com//ubuntu $RELEASE main
+# stuff things
+
+deb http://ubuntu.com//ubuntu $RELEASE-updates main
+deb http://ubuntu.com//ubuntu $RELEASE-security main
+deb-src http://ubuntu.com//ubuntu $RELEASE universe multiverse
+# deb http://ubuntu.com/ubuntu/ $RELEASE-proposed main
+deb [a=b] http://ubuntu.com/ubuntu/ $RELEASE-backports main
+"""
+        expect = """deb http://ubuntu.com//ubuntu xenial main
+# stuff things
+
+deb http://ubuntu.com//ubuntu xenial-updates main
+deb http://ubuntu.com//ubuntu xenial-security main
+deb-src http://ubuntu.com//ubuntu xenial universe multiverse
+# deb http://ubuntu.com/ubuntu/ xenial-proposed main
+# deb [a=b] http://ubuntu.com/ubuntu/ $RELEASE-backports main
+"""
+        # $RELEASE in backports doesn't get expanded because the line is
+        # considered invalid because of the options.  So when the line
+        # gets commented out, it comments out the original line, not
+        # what we've modifed it to.
+        rel = 'xenial'
+        mirrors = {'PRIMARY': 'http://ubuntu.com/ubuntu/'}
+
+        cfg = {
+            'preserve_sources_list': False,
+            'sources_list': orig,
+            'disable_suites': ['backports'],
+        }
+
+        apt_config.generate_sources_list(cfg, rel, mirrors, self.target)
+        filepath = os.path.join(self.target, 'etc/apt/sources.list')
+        write_file.assert_called_with(filepath, expect, mode=0o644)
 
 
 class TestDebconfSelections(CiTestCase):
diff --git a/tests/vmtests/__init__.py b/tests/vmtests/__init__.py
index 0b19d8f..fd6c246 100644
--- a/tests/vmtests/__init__.py
+++ b/tests/vmtests/__init__.py
@@ -1631,6 +1631,9 @@ class VMBaseClass(TestCase):
     def check_file_regex(self, filename, regex):
         self.assertRegex(self.load_collect_file(filename), regex)
 
+    def not_file_regex(self, filename, regex):
+        self.assertNotRegex(self.load_collect_file(filename), regex)
+
     # To get rid of deprecation warning in python 3.
     def assertRegex(self, s, r):
         try:
@@ -1640,6 +1643,14 @@ class VMBaseClass(TestCase):
             # Python 2.
             self.assertRegexpMatches(s, r)
 
+    def assertNotRegex(self, s, r):
+        try:
+            # Python 3.
+            super(VMBaseClass, self).assertNotRegex(s, r)
+        except AttributeError:
+            # Python 2.
+            self.assertNotRegexpMatches(s, r)
+
     def get_blkid_data(self, blkid_file):
         data = self.load_collect_file(blkid_file)
         ret = {}
diff --git a/tests/vmtests/test_apt_source.py b/tests/vmtests/test_apt_source.py
index a090ffa..9d00dc8 100644
--- a/tests/vmtests/test_apt_source.py
+++ b/tests/vmtests/test_apt_source.py
@@ -164,9 +164,8 @@ class TestAptSrcDisablePockets(TestAptSrcAbs):
                               r"deb.*us.archive.ubuntu.com")
         self.check_file_regex("sources.list",
                               r"deb.*security.ubuntu.com")
-        # updates disabled
-        self.check_file_regex("sources.list",
-                              r"# suite disabled by curtin:.*-updates")
+        # updates disabled and not present
+        self.not_file_regex("sources.list", r"# .*-updates")
 
 
 class TestAptSrcModifyArches(TestAptSrcModify):
diff --git a/tox.ini b/tox.ini
index f27dfbc..d9437c5 100644
--- a/tox.ini
+++ b/tox.ini
@@ -71,6 +71,7 @@ commands = {envpython} -m pylint --errors-only {posargs:curtin tests/vmtests}
 [testenv:py27-pylint]
 # set basepython because tox 1.6 (trusty) does not support generated environments
 basepython = python2.7
+sitepackages = true
 deps = {[testenv]deps}
    {[testenv:py27]deps}
     pylint==1.8.1