Merge lp:~ev/apt-clone/python3 into lp:apt-clone

[Evan]

Port to Python3.

[lp:~ev/apt-clone/python3]

109. By Evan on 2012-06-11

* Finish port to Python 3:
  - Handle unicode changes.
  - Do not leak file descriptors.
  - Test against both Python2.7 and Python3.
  - Do not use the deprecated failUnless.
  - Use the io module instead of StringIO.
  - Fix a failing test caused by acpi-support being in main.

110. By Evan on 2012-06-11

Actually use python3 :)

[Colin Watson (cjwatson)]

> === modified file 'apt_clone.py'
> --- apt_clone.py 2012-01-27 17:14:05 +0000
> +++ apt_clone.py 2012-06-11 15:50:47 +0000
> @@ -182,7 +184,8 @@
> tarinfo = tarfile.TarInfo("./var/lib/apt-clone/installed.pkgs")
> tarinfo.size = len(s)
> tarinfo.mtime = time.time()
> - tar.addfile(tarinfo, StringIO(s))
> + s = s.encode('utf-8')
> + tar.addfile(tarinfo, BytesIO(s))

tarinfo.size will be wrong if s isn't entirely ASCII. I think you need
to encode earlier.

> - #print tar.getnames()
> + print(tar.getnames())

Did you mean to uncomment this?

> === modified file 'debian/changelog'
> --- debian/changelog 2012-01-27 16:54:20 +0000
> +++ debian/changelog 2012-06-11 15:50:47 +0000
> @@ -8,6 +8,16 @@
>
> -- Michael Vogt <email address hidden> Fri, 27 Jan 2012 16:29:17 +0100
>
> +apt-clone (0.2.2ubuntu1) UNRELEASED; urgency=low
> +
> + * Port to Python 3:
> + - Use Python 3-style print functions.
> + - Use "raise Exception(value)" syntax rather than the old-style "raise
> + Exception, value".
> + - Use dict.items() rather than dict.iteritems().
> +
> + -- Colin Watson <email address hidden> Mon, 11 Jun 2012 09:12:14 +0100
> +
> apt-clone (0.2.2) unstable; urgency=low
>
> * fix extraction of no-longer downloadable debs, thanks
>

This might need updating :-)

The rest looks good to me.

 review needs-fixing

review: Needs Fixing

[lp:~ev/apt-clone/python3]

111. By Evan on 2012-06-12

Encode earlier so tarinfo.size is accurate. Thanks Colin Watson!

112. By Evan on 2012-06-12

Re-comment debugging print statement.

113. By Evan on 2012-06-12

Fix changelog.

[Evan (ev)]

I've addressed the issues Colin raises as r111, r112, and r113.

[Colin Watson (cjwatson)]

Thanks, looks good to me now. I think this is Michael's upstream branch so he may want to flip the version back to 0.2.3 for upload to Debian.

review: Approve

=== modified file 'apt-clone'
--- apt-clone	2012-01-27 23:47:38 +0000
+++ apt-clone	2012-06-12 09:23:17 +0000
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/python3
 # Copyright (C) 2011 Canonical
 #
 # Authors:
@@ -17,6 +17,8 @@
 # this program; if not, write to the Free Software Foundation, Inc.,
 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
 
+from __future__ import print_function
+
 import argparse
 from apt_clone import AptClone
 
@@ -79,19 +81,19 @@
     clone = AptClone()
     if args.command == "info":
         info = clone.info(args.source)
-        print info
+        print(info)
     if args.command == "clone":
         clone.save_state(args.source, args.destination, 
                          args.with_dpkg_repack, args.with_dpkg_status)
-        print "not installable: %s" % ", ".join(clone.not_downloadable)
-        print "version mismatch: %s" % ", ".join(clone.version_mismatch)
+        print("not installable: %s" % ", ".join(clone.not_downloadable))
+        print("version mismatch: %s" % ", ".join(clone.version_mismatch))
         if not args.with_dpkg_repack:
-            print "\nNote that you can use --with-dpkg-repack to include "\
-                  "those packages in the clone file."
+            print("\nNote that you can use --with-dpkg-repack to include "
+                  "those packges in the clone file.")
     elif args.command == "restore":
         if args.simulate:
             miss = clone.simulate_restore_state(args.source)
-            print "missing: %s" % ",".join(sorted(list(miss)))
+            print("missing: %s" % ",".join(sorted(list(miss))))
         else:
             clone.restore_state(args.source, args.destination)
     elif args.command == "restore-new-distro":
@@ -111,7 +113,7 @@
         if args.simulate:
             miss = clone.simulate_restore_state(
                 args.source, args.new_distro_codename)
-            print "missing: %s" % ",".join(sorted(list(miss)))
+            print("missing: %s" % ",".join(sorted(list(miss))))
         else:
             clone.restore_state(
                 args.source, args.destination, args.new_distro_codename, protect_installed)

=== modified file 'apt_clone.py'
--- apt_clone.py	2012-01-27 17:14:05 +0000
+++ apt_clone.py	2012-06-12 09:23:17 +0000
@@ -16,6 +16,8 @@
 # this program; if not, write to the Free Software Foundation, Inc.,
 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
 
+from __future__ import print_function
+
 import apt
 from apt.cache import FetchFailedException
 import apt_pkg
@@ -29,7 +31,7 @@
 import tempfile
 import time
 
-from StringIO import StringIO
+from io import BytesIO
 
 if "APT_CLONE_DEBUG_RESOLVER" in os.environ:
     apt_pkg.config.set("Debug::pkgProblemResolver", "1")
@@ -158,9 +160,9 @@
                        'arch'       : apt_pkg.config.find("APT::Architecture")
                      }
         # save it
-        f = tempfile.NamedTemporaryFile()
+        f = tempfile.NamedTemporaryFile(mode='w')
         info = "\n".join(["%s: %s" % (key, value) 
-                          for (key, value) in host_info.iteritems()])
+                          for (key, value) in host_info.items()])
         f.write(info+"\n")
         f.flush()
         tar.add(f.name, arcname="./var/lib/apt-clone/uname")
@@ -180,9 +182,10 @@
                     self.version_mismatch.add(pkg.name)
         # store the installed.pkgs
         tarinfo = tarfile.TarInfo("./var/lib/apt-clone/installed.pkgs")
+        s = s.encode('utf-8')
         tarinfo.size = len(s)
         tarinfo.mtime = time.time()
-        tar.addfile(tarinfo, StringIO(s))
+        tar.addfile(tarinfo, BytesIO(s))
 
     def _write_state_dpkg_status(self, tar):
         # store dpkg-status, this is not strictly needed as installed.pkgs
@@ -232,11 +235,11 @@
             self.commands.repack_deb(pkgname, tdir)
         tar.add(tdir, arcname="./var/lib/apt-clone/debs")
         shutil.rmtree(tdir)
-        #print tdir
+        #print(tdir)
 
     # detect prefix
     def _detect_tarprefix(self, tar):
-        #print tar.getnames()
+        #print(tar.getnames())
         if tar.getnames()[-1].startswith("./"):
             self.TARPREFIX = "./"
         else:
@@ -313,7 +316,7 @@
         self._detect_tarprefix(tar)
 
         if not os.path.exists(targetdir):
-            print "Dir '%s' does not exist, need to bootstrap first" % targetdir
+            print("Dir '%s' does not exist, need to bootstrap first" % targetdir)
             distro = self._get_info_distro(statefile)
             self.commands.debootstrap(targetdir, distro)
 
@@ -401,7 +404,7 @@
         # the actiongroup will help libapt to speed up the following loop
         with cache.actiongroup():
             for line in f.readlines():
-                line = line.strip()
+                line = line.strip().decode('utf-8')
                 if line.startswith("#") or line == "":
                     continue
                 (name, version, auto) = line.split()
@@ -416,7 +419,7 @@
                             if cache.broken_count > 0:
                                 resolver.resolve()
                                 if not cache[name].marked_install:
-                                    raise SystemError, "pkg %s not marked upgrade" % name
+                                    raise SystemError("pkg %s not marked upgrade" % name)
                         else:
                             # normal mode, this assume the system is consistent
                             cache[name].mark_install(from_user=from_user)
@@ -519,9 +522,10 @@
         owned = set()
         dpkg_basedir = os.path.dirname(apt_pkg.config.get("Dir::State::status"))
         for f in glob.glob(os.path.join(dpkg_basedir, "info", "*.list")):
-            for line in open(f):
-                if line.startswith("/etc/"):
-                    owned.add(line.strip())
+            with open(f) as fp:
+                for line in fp:
+                    if line.startswith("/etc/"):
+                        owned.add(line.strip())
         # now go over etc
         unowned = set()
         for dirpath, dirnames, filenames in os.walk(etcdir):
@@ -535,37 +539,39 @@
         dpkg_status = sourcedir+apt_pkg.config.find("Dir::State::status")
         modified = set()
         # iterate dpkg-status file
-        tag = apt_pkg.TagFile(open(dpkg_status))
-        for entry in tag:
-            if "conffiles" in entry:
-                for line in entry["conffiles"].split("\n"):
-                    obsolete = None
-                    if len(line.split()) == 3:
-                        name, md5sum, obsolete = line.split()
-                    else:
-                        name, md5sum = line.split()
-                    # update
-                    path = sourcedir+name
-                    md5sum = md5sum.strip()
-                    # ignore oboslete conffiles
-                    if obsolete == "obsolete":
-                        continue
-                    # user removed conffile
-                    if not os.path.exists(path):
-                        logging.debug("conffile %s removed" % path)
-                        modified.add(path)
-                        continue
-                    # check content
-                    md5 = hashlib.md5()
-                    md5.update(open(path).read())
-                    if md5.hexdigest() != md5sum:
-                        logging.debug("conffile %s (%s != %s)" % (
-                                path, md5.hexdigest(), md5sum))
-                        modified.add(path)
+        with open(dpkg_status) as fp:
+            tag = apt_pkg.TagFile(fp)
+            for entry in tag:
+                if "conffiles" in entry:
+                    for line in entry["conffiles"].split("\n"):
+                        obsolete = None
+                        if len(line.split()) == 3:
+                            name, md5sum, obsolete = line.split()
+                        else:
+                            name, md5sum = line.split()
+                        # update
+                        path = sourcedir+name
+                        md5sum = md5sum.strip()
+                        # ignore oboslete conffiles
+                        if obsolete == "obsolete":
+                            continue
+                        # user removed conffile
+                        if not os.path.exists(path):
+                            logging.debug("conffile %s removed" % path)
+                            modified.add(path)
+                            continue
+                        # check content
+                        md5 = hashlib.md5()
+                        with open(path, 'rb') as fp:
+                            md5.update(fp.read())
+                        if md5.hexdigest() != md5sum:
+                            logging.debug("conffile %s (%s != %s)" % (
+                                    path, md5.hexdigest(), md5sum))
+                            modified.add(path)
         return modified
 
     def _dump_debconf_database(self, sourcedir):
-        print "not implemented yet"
+        print("not implemented yet")
         # debconf-copydb configdb newdb --config=Name:newdb --config=Driver:File --config=Filename:/tmp/lala.db
         # 
         # debconf-copydb newdb configdb --config=Name:newdb --config=Driver:File --config=Filename:/tmp/lala.db

=== modified file 'debian/changelog'
--- debian/changelog	2012-01-27 16:54:20 +0000
+++ debian/changelog	2012-06-12 09:23:17 +0000
@@ -1,11 +1,28 @@
-apt-clone (0.2.3) UNRELEASED; urgency=low
+apt-clone (0.2.2ubuntu1) UNRELEASED; urgency=low
 
+  [ Michael Vogt ]
   * apt_clone.py:
     - fix restoring of the clone with later apt versions (thanks
       to Colin Watson)
     - bind mount /proc, /sys on restore to chroot (thanks to
       Colin Watson)
 
+  [ Colin Watson ]
+  * Port to Python 3:
+    - Use Python 3-style print functions.
+    - Use "raise Exception(value)" syntax rather than the old-style "raise
+      Exception, value".
+    - Use dict.items() rather than dict.iteritems().
+
+  [ Evan Dandrea ]
+  * Finish port to Python 3:
+    - Handle unicode changes.
+    - Do not leak file descriptors.
+    - Test against both Python2.7 and Python3.
+    - Do not use the deprecated failUnless.
+    - Use the io module instead of StringIO.
+    - Fix a failing test caused by acpi-support being in main.
+
  -- Michael Vogt <[e-mail]>  Fri, 27 Jan 2012 16:29:17 +0100
 
 apt-clone (0.2.2) unstable; urgency=low

=== modified file 'tests/Makefile'
--- tests/Makefile	2011-04-08 14:36:22 +0000
+++ tests/Makefile	2012-06-12 09:23:17 +0000
@@ -4,7 +4,9 @@
 test:
 	pyflakes ../apt-clone ../apt_clone.py
 	set -e; for f in *.py; do \
-		PYTHONPATH=.. python $$f; \
+		for ver in python3 python2; do \
+			PYTHONPATH=.. $$ver $$f; \
+		done; \
 	done; \
 	# cruft from the tests
 	rm -f ./data/mock-system/var/cache/apt/*.bin

=== modified file 'tests/test_clone.py'
--- tests/test_clone.py	2012-01-02 18:08:31 +0000
+++ tests/test_clone.py	2012-06-12 09:23:17 +0000
@@ -1,5 +1,7 @@
 #!/usr/bin/python
 
+from __future__ import print_function
+
 import apt
 import apt_pkg
 import mock
@@ -10,7 +12,7 @@
 import tempfile
 import unittest
 
-from StringIO import StringIO
+from io import StringIO
 
 sys.path.insert(0, "..")
 import apt_clone
@@ -31,7 +33,8 @@
         os.makedirs(os.path.join(self.tempdir, "var/lib/dpkg/"))
         # ensure we are the right arch
         os.makedirs(os.path.join(self.tempdir, "etc/apt"))
-        open(os.path.join(self.tempdir, "etc/apt/apt.conf"), "w").write('''
+        with open(os.path.join(self.tempdir, "etc/apt/apt.conf"), "w") as fp:
+            fp.write('''
 #clear Dpkg::Post-Invoke;
 #clear Dpkg::Pre-Invoke;
 #clear APT::Update;
@@ -59,7 +62,7 @@
         tarname = os.path.join(targetdir, clone.CLONE_FILENAME)
         self.assertTrue(os.path.exists(tarname))
         tar = tarfile.open(tarname)
-        #print tar.getmembers()
+        #print(tar.getmembers())
         # verify members in tar
         members = [m.name for m in tar.getmembers()]
         self.assertTrue("./etc/apt/sources.list" in members)
@@ -108,11 +111,14 @@
         # create target dir
         targetdir = self.tempdir
         # status file from maverick (to simulate running on a maverick live-cd)
-        s=open("./data/dpkg-status/dpkg-status-ubuntu-maverick").read()
+        with open("./data/dpkg-status/dpkg-status-ubuntu-maverick") as fp:
+            s = fp.read()
         s = s.replace(
             "Architecture: i386",
             "Architecture: %s" % apt_pkg.config.find("Apt::Architecture"))
-        open(os.path.join(targetdir, "var/lib/dpkg", "status"), "w").write(s)
+        path = os.path.join(targetdir, "var/lib/dpkg", "status")
+        with open(path, "w") as fp:
+            fp.write(s)
         # test upgrade clone from lucid system to maverick
         clone = AptClone(cache_cls=MockAptCache)
         clone.restore_state(
@@ -121,14 +127,16 @@
             "maverick")
         sources_list = os.path.join(targetdir, "etc","apt","sources.list")
         self.assertTrue(os.path.exists(sources_list))
-        self.assertTrue("maverick" in open(sources_list).read())
-        self.assertFalse("lucid" in open(sources_list).read())
+        with open(sources_list) as fp:
+            self.assertTrue("maverick" in fp.read())
+        with open(sources_list) as fp:
+            self.assertFalse("lucid" in fp.read())
         
     def test_restore_state_simulate(self):
         clone = AptClone()
         missing = clone.simulate_restore_state("./data/apt-state.tar.gz")
         # missing, because clone does not have universe enabled
-        self.assertEqual(list(missing), ["accerciser", "acpi-support"])
+        self.assertEqual(list(missing), ["accerciser"])
 
     def test_restore_state_simulate_with_new_release(self):
         #apt_pkg.config.set("Debug::PkgProblemResolver", "1")
@@ -139,7 +147,7 @@
         missing = clone.simulate_restore_state(
             "./data/apt-state-ubuntu-lucid.tar.gz", "maverick") 
         # FIXME: check that the stuff in missing is ok
-        print missing
+        print(missing)
 
     def test_modified_conffiles(self):
         clone = AptClone()
@@ -162,13 +170,13 @@
             "Dir::state::status", 
             "/var/lib/dpkg/status")
         unowned = clone._find_unowned_in_etc()
-        #print unowned
+        #print(unowned)
         self.assertNotEqual(unowned, set())
         # negative test, is created by the installer
         self.assertTrue("/etc/apt/sources.list" in unowned)
         # postivie test, belongs to base-files
         self.assertFalse("/etc/issue" in unowned)
-        print "\n".join(sorted(unowned))
+        print("\n".join(sorted(unowned)))
 
 if __name__ == "__main__":
     unittest.main()

=== modified file 'tests/test_clone_upgrade.py'
--- tests/test_clone_upgrade.py	2011-04-07 13:02:27 +0000
+++ tests/test_clone_upgrade.py	2012-06-12 09:23:17 +0000
@@ -9,7 +9,7 @@
 import tempfile
 import unittest
 
-from StringIO import StringIO
+from io import StringIO
 
 sys.path.insert(0, "..")
 import apt_clone
@@ -60,7 +60,8 @@
         sources_list = os.path.join(tmpdir, "etc", "apt", "sources.list")
         if not os.path.exists(os.path.dirname(sources_list)):
             os.makedirs(os.path.dirname(sources_list))
-        open(os.path.join(sources_list), "w").write("""
+        with open(os.path.join(sources_list), "w") as fp:
+            fp.write("""
 deb http://archive.ubuntu.com/ubuntu %s main restricted universe multiverse
 """ % from_dist)
         cache = apt.Cache(rootdir=tmpdir)
@@ -74,20 +75,18 @@
             dpkg_status = os.path.join(tmpdir, "var", "lib", "dpkg", "status")
             if not os.path.exists(os.path.dirname(dpkg_status)):
                 os.makedirs(os.path.dirname(dpkg_status))
-            dpkg = open(dpkg_status, "w")
-            installed = open(installed_pkgs, "w")
-            for pkg in cache:
-                if pkg.marked_install:
-                    s = str(pkg.candidate.record)
-                    s = s.replace("Package: %s\n" % pkg.name,
-                                  "Package: %s\n%s\n" % (
-                            pkg.name, "Status: install ok installed"))
-                    dpkg.write("%s\n" % s)
-                    installed.write("%s %s %s\n" % (pkg.name,
-                                                    pkg.candidate.version,
-                                                    int(pkg.is_auto_installed)))
-            dpkg.close()
-            installed.close()
+            with open(dpkg_status, "w") as dpkg:
+                with open(installed_pkgs, "w") as installed:
+                    for pkg in cache:
+                        if pkg.marked_install:
+                            s = str(pkg.candidate.record)
+                            s = s.replace("Package: %s\n" % pkg.name,
+                                          "Package: %s\n%s\n" % (
+                                    pkg.name, "Status: install ok installed"))
+                            dpkg.write("%s\n" % s)
+                            installed.write("%s %s %s\n" % (pkg.name,
+                                                            pkg.candidate.version,
+                                                            int(pkg.is_auto_installed)))
         return tmpdir
 
 

=== modified file 'tests/test_in_chroot.py'
--- tests/test_in_chroot.py	2012-01-27 17:14:05 +0000
+++ tests/test_in_chroot.py	2012-06-12 09:23:17 +0000
@@ -1,5 +1,7 @@
 #!/usr/bin/python
 
+from __future__ import print_function
+
 import apt
 import logging
 import os
@@ -25,7 +27,7 @@
 
     def test_real(self):
         if os.getuid() != 0:
-            print "Skipping because uid != 0"
+            print("Skipping because uid != 0")
             return
         # do it
         target = "./test-chroot"
@@ -34,8 +36,8 @@
             subprocess.call(["debootstrap", "--arch=i386",
                              "maverick", target])
         # force i386
-        open(os.path.join(target, "etc/apt/apt.conf"), "w").write(
-            'APT::Architecture "i386";')
+        with open(os.path.join(target, "etc/apt/apt.conf"), "w") as fp:
+            fp.write('APT::Architecture "i386";')
 
         # restore
         clone = AptClone()

=== modified file 'tests/test_merge_sources.py'
--- tests/test_merge_sources.py	2011-04-06 14:06:31 +0000
+++ tests/test_merge_sources.py	2012-06-12 09:23:17 +0000
@@ -26,8 +26,8 @@
                 if line != '\n' and not line.startswith('#'):
                     tally[line] += 1
             # There should not be any duplicate source lines.
-            for line, count in tally.iteritems():
-                self.failUnless(count == 1, '"%s" occurred %d times.'
+            for line, count in tally.items():
+                self.assertTrue(count == 1, '"%s" occurred %d times.'
                                 % (line, count))
 
             # Check for extras, others...
@@ -43,7 +43,7 @@
                 for line in fp:
                     if line == match:
                         found = True
-                self.failUnless(found,
+                self.assertTrue(found,
                         '%s repository not present or disabled.' % pocket)
 
 if __name__ == "__main__":