Merge lp:~xnox/apt-btrfs-snapshot/py3 into lp:apt-btrfs-snapshot

[Dimitri John Ledkov]

python3 port and other fixes

[Colin Watson (cjwatson)]

Looks good to me, though I'd like Michael to review this and merge if he's happy.

review: Approve

[Barry Warsaw (barry)]

On Jun 11, 2012, at 01:20 PM, Dmitrijs Ledkovs wrote:

>Dmitrijs Ledkovs has proposed merging lp:~dmitrij.ledkov/apt-btrfs-snapshot/py3 into lp:apt-btrfs-snapshot.
>
>Requested reviews:
> Canonical Foundations Team (canonical-foundations)
> Michael Vogt (mvo)
>
>For more details, see:
>https://code.launchpad.net/~dmitrij.ledkov/apt-btrfs-snapshot/py3/+merge/109632
>
>python3 port and other fixes

=== modified file 'apt-btrfs-snapshot'
--- apt-btrfs-snapshot 2011-07-19 11:42:45 +0000
+++ apt-btrfs-snapshot 2012-06-11 13:19:19 +0000
> @@ -1,4 +1,4 @@
> -#!/usr/bin/python
> +#!/usr/bin/python3
> # Copyright (C) 2011 Canonical
> #
> # Author:
> @@ -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

This import got me thinking. If apt-btrfs-snapshot (the script) will only run
in Python 3, then clearly we don't need this. If it could run in Python 2
(e.g. explicitly invoked ignoring the #! line, or if you want to hold out the
possibility of reverting to Python 2), then the import is fine, but possibly
incomplete.

(Aside: print_function was introduced in Python 2.6.0a2:
http://docs.python.org/py3k/library/__future__.html)

It might be incompletely because you may also need to import unicode_literals
(FWIW, just as a blanket rule, I always import those two plus
absolute_import). The other thing you'll need to check is that the _()
function is defined properly for cross-py2/py3 compatibility. It's not clear
from the diff if this is the case.

Here's the thing: you always want _() to return unicode strings, and you want
the input to be unicode strings. In Python 2, gettext.gettext() is *not* the
right thing to use, you need gettext.ugettext(). In Python 3,
gettext.gettext() does operate on and return unicodes.

You can do something like this to make it work right (this is pulled from the
c-j code in u-m).

def setup_gettext():
    domain = 'foo'
    localedir = os.environ.get('LOCPATH')
    t = gettext.translation(domain, localedir=localedir, fallback=True)
    try:
        return t.ugettext
    except AttributeError:
        return t.gettext

Then...

_ = setup_gettext()

OTOH, if the script will always run under Python 3, most of that won't be
necessary.

=== modified file 'apt_btrfs_snapshot.py'
--- apt_btrfs_snapshot.py 2011-10-17 08:05:22 +0000
+++ apt_btrfs_snapshot.py 2012-06-11 13:19:19 +0000
> @@ -16,9 +16,10 @@
> # 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

Even though you're not using gettext, I'd still consider including
unicode_literals here. The problem is that if in Python 2, you're operating
on 8-bit strings, they'll magically become unicodes in Python 3, and then
you're just asking for trouble. If the literals really should be 8-bit
strings (a.k.a. bytes) in both versions, then import unicode_literals and
prefix your byte literals with b''. That way, the intentions are clear, and
things will work the same in both versions.

> @@ -57,14 +58,16 @@
...

[lp:~xnox/apt-btrfs-snapshot/py3]

43. By Dimitri John Ledkov on 2012-06-11

Address barry's comments

[Michael Vogt (mvo)]

Thanks for your work on this branch!

it looks good. One comment below:

On Mon, Jun 11, 2012 at 01:20:25PM -0000, Dmitrijs Ledkovs wrote:
> - def test_fstab_detect_snapshot(self):
> + @mock.patch('os.path.exists', side_effect=lambda f: f in ('/sbin/btrfs'))

This seems this requires python-mock 0.8, it would be nice if that
would still work on 12.04 for now if its not too much hassle to make
it easy for contributors to hack on it without the need to upgrade to
quantal just yet.

We should probably add a generic pyflakes and pep8 test to the tests
dir, I will do that in a bit.

Thanks,
 Michael

[Dimitri John Ledkov (xnox)]

Pushed upto -r44, which uses python-mock 0.7 style api/syntax. Works in precise pbuilder.
Regards,
Dmitrijs.

[lp:~xnox/apt-btrfs-snapshot/py3]

44. By Dimitri John Ledkov on 2012-06-12

use python-mock 0.7 API/syntax

=== modified file 'apt-btrfs-snapshot'
--- apt-btrfs-snapshot	2011-07-19 11:42:45 +0000
+++ apt-btrfs-snapshot	2012-06-11 17:29:49 +0000
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/python3
 # Copyright (C) 2011 Canonical
 #
 # Author:
@@ -84,21 +84,21 @@
         logging.basicConfig(level=logging.INFO)
 
     if os.getuid() != 0:
-        print _("Sorry, you need to be root to run this program")
+        print(_("Sorry, you need to be root to run this program"))
         sys.exit(1)
 
     apt_btrfs = AptBtrfsSnapshot()
     if not apt_btrfs.snapshots_supported():
-        print _("Sorry, your system lacks support for the snapshot feature")
+        print(_("Sorry, your system lacks support for the snapshot feature"))
         sys.exit(1)
 
     res = False
     if args.command == "supported":
         res = apt_btrfs.snapshots_supported()
         if res:
-            print _("Supported")
+            print(_("Supported"))
         else:
-            print _("Not supported")
+            print(_("Not supported"))
     elif args.command == "snapshot":
         res = apt_btrfs.create_btrfs_root_snapshot()
     elif args.command == "list":
@@ -112,7 +112,7 @@
     elif args.command == "delete-older-than":
         res = apt_btrfs.clean_btrfs_root_snapshots_older_than(args.time)
     else:
-        print _("ERROR: Unhandled command: '%s'") % args.command
+        print(_("ERROR: Unhandled command: '%s'") % args.command)
 
     # return the right exit code
     if res:

=== modified file 'apt_btrfs_snapshot.py'
--- apt_btrfs_snapshot.py	2011-10-17 08:05:22 +0000
+++ apt_btrfs_snapshot.py	2012-06-11 17:29:49 +0000
@@ -16,9 +16,10 @@
 # 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, unicode_literals
+
 import datetime
 import os
-import string
 import subprocess
 import sys
 import time
@@ -57,14 +58,16 @@
     """ a list of FstabEntry items """
     def __init__(self, fstab="/etc/fstab"):
         super(Fstab, self).__init__()
-        for line in map(string.strip, open(fstab)):
-            if line == "" or line.startswith("#"):
-                continue
-            try:
-                entry = FstabEntry.from_line(line)
-            except ValueError:
-                continue
-            self.append(entry)
+        
+        with open(fstab) as fstab_file:
+            for line in (l.strip() for l in fstab_file):
+                if line == "" or line.startswith("#"):
+                    continue
+                try:
+                    entry = FstabEntry.from_line(line)
+                except ValueError:
+                    continue
+                self.append(entry)
 
 class LowLevelCommands(object):
     """ lowlevel commands invoked to perform various tasks like
@@ -133,7 +136,7 @@
         self._btrfs_root_mountpoint = None
         return res
     def _get_now_str(self):
-        return  datetime.datetime.now().replace(microsecond=0).isoformat("_")
+        return  datetime.datetime.now().replace(microsecond=0).isoformat(str('_'))
     def create_btrfs_root_snapshot(self, additional_prefix=""):
         mp = self.mount_btrfs_root_volume()
         snap_id = self._get_now_str()
@@ -153,7 +156,7 @@
         if older_than != 0:
             entry = self._get_supported_btrfs_root_fstab_entry()
             if not entry:
-                raise AptBtrfsSnapshotNotSupportedError()
+                raise AptBtrfsNotSupportedError()
             if "noatime" in entry.options:
                 raise AptBtrfsRootWithNoatimeError()
         # if there is no older than, interpret that as "now"
@@ -170,8 +173,8 @@
         self.umount_btrfs_root_volume()
         return l
     def print_btrfs_root_snapshots(self):
-        print "Available snapshots:"
-        print "  \n".join(self.get_btrfs_root_snapshots_list())
+        print("Available snapshots:")
+        print("  \n".join(self.get_btrfs_root_snapshots_list()))
         return True
     def _parse_older_than_to_unixtime(self, timefmt):
         now = time.time()
@@ -182,9 +185,9 @@
     def print_btrfs_root_snapshots_older_than(self, timefmt):
         older_than_unixtime = self._parse_older_than_to_unixtime(timefmt)
         try:
-            print "Available snapshots older than '%s':" % timefmt
-            print "  \n".join(self.get_btrfs_root_snapshots_list(
-                    older_than=older_than_unixtime))
+            print("Available snapshots older than '%s':" % timefmt)
+            print("  \n".join(self.get_btrfs_root_snapshots_list(
+                    older_than=older_than_unixtime)))
         except AptBtrfsRootWithNoatimeError:
             sys.stderr.write("Error: fstab option 'noatime' incompatible with option")
             return False
@@ -202,7 +205,7 @@
         return res
     def command_set_default(self, snapshot_name):
         res = self.set_default(snapshot_name)
-        print "Please reboot"
+        print("Please reboot")
         return res
     def set_default(self, snapshot_name, backup=True):
         """ set new default """

=== modified file 'debian/changelog'
--- debian/changelog	2011-10-17 08:05:22 +0000
+++ debian/changelog	2012-06-11 17:29:49 +0000
@@ -1,5 +1,6 @@
-apt-btrfs-snapshot (0.2.2) UNRELEASEDoneiric-proposed; urgency=low
+apt-btrfs-snapshot (0.2.2) UNRELEASED; urgency=low
 
+  [ Michael Vogt ]
   * apt_btrfs_snapshot.py, test/test_apt_btrfs_snapshot.py:
     - disable "delete-older-than" command if noatime is in use on 
       the snapshot fs (LP: #833980)
@@ -8,7 +9,30 @@
     - be even more robust against invalid fstab entries 
       (LP: #873411 comment #7)
 
- -- Michael Vogt <[e-mail]>  Tue, 11 Oct 2011 10:20:45 +0200
+  [ Dmitrijs Ledkovs ]
+  * apt_btrfs_snapshot.py, apt-btrfs-snapshot:
+    - port to python3
+  * apt_btrfs_snapshot.py:
+    - fix pyflakes warning use 'AptBtrfsNotSupportedError' instead of
+      undefined name 'AptBtrfsSnapshotNotSupportedError'
+  * test/test_apt_btrfs_snapshot.py:
+    - remove unused imports 'LowLevelCommands' and 'apt_btrfs_snapshot'
+    - mock sys.stdout and sys.stderr to avoid confusing and spurious output
+    - mock /sbin/btrfs to allow running unit-tests without btrfs-tools
+  * debian/rules:
+    - build with supported versions of python2 and python3
+    - run unittests at build time, honoring nocheck
+  * debian/control:
+    - adjust dependencies for python3 support
+    - bump standards version to 3.9.3
+  
+ -- Dmitrijs Ledkovs <[e-mail]>  Mon, 11 Jun 2012 14:07:41 +0100
+
+apt-btrfs-snapshot (0.2.1build1) precise; urgency=low
+
+  * Rebuild to drop python2.6 dependencies.
+
+ -- Matthias Klose <[e-mail]>  Sat, 31 Dec 2011 02:00:37 +0000
 
 apt-btrfs-snapshot (0.2.1) oneiric; urgency=low
 

=== modified file 'debian/control'
--- debian/control	2011-10-10 21:21:46 +0000
+++ debian/control	2012-06-11 17:29:49 +0000
@@ -2,16 +2,22 @@
 Section: admin
 Priority: optional
 Maintainer: Michael Vogt <[e-mail]>
-Build-Depends: debhelper (>= 7),
+Build-Depends: debhelper (>= 7.0.50~),
+               python3,
+               python3-distutils-extra,
+               python3-mock,
                python (>= 2.6.5-2~),
                python-distutils-extra,
+               python-mock,
 X-Python-Version: all
-Standards-Version: 3.9.1
+X-Python3-Version: >= 3.2
+Standards-Version: 3.9.3
 Vcs-Bzr: https://code.launchpad.net/apt-btrfs-snapshot
 
 Package: apt-btrfs-snapshot
 Architecture: all
 Depends: ${python:Depends},
+         ${python3:Depends},
          ${misc:Depends},
          btrfs-tools
 Description: Automatically create snapshot on apt operations

=== modified file 'debian/rules'
--- debian/rules	2011-02-16 11:56:59 +0000
+++ debian/rules	2012-06-11 17:29:49 +0000
@@ -1,5 +1,27 @@
 #!/usr/bin/make -f
+PYTHON2:=$(shell pyversions -r)
+PYTHON3:=$(shell py3versions -r)
+py3sdo=set -e; $(foreach py, $(PYTHON3), $(py) $(1);)
+pyalldo=set -e; $(foreach py, $(PYTHON2) $(PYTHON3), $(py) $(1);)
 
 %:
-	dh $@ --with python2
+	dh $@ --with python2,python3
+
+ifeq (,$(filter nocheck,$(DEB_BUILD_OPTIONS)))
+override_dh_auto_test:
+	$(call pyalldo, -m unittest discover -vv test/)
+endif
+
+override_dh_auto_build:
+	dh_auto_build
+	$(call py3sdo, setup.py build)
+
+override_dh_auto_install:
+	dh_auto_install
+	$(call py3sdo, setup.py install --root=$(CURDIR)/debian/apt-btrfs-snapshot --install-layout=deb)
+
+override_dh_auto_clean:
+	dh_auto_clean
+	rm -rf build
+	rm -rf *.egg-info
 

=== modified file 'test/test_apt_btrfs_snapshot.py'
--- test/test_apt_btrfs_snapshot.py	2011-10-17 08:05:22 +0000
+++ test/test_apt_btrfs_snapshot.py	2012-06-11 17:29:49 +0000
@@ -1,5 +1,9 @@
 #!/usr/bin/python
 
+try:
+    from StringIO import StringIO
+except ImportError:
+    from io import StringIO
 import mock
 import os
 import sys
@@ -8,17 +12,16 @@
 
 sys.path.insert(0, "..")
 sys.path.insert(0, ".")
-import apt_btrfs_snapshot
 from apt_btrfs_snapshot import (AptBtrfsSnapshot,
-                                AptBtrfsRootWithNoatimeError,
-                                LowLevelCommands)
+                                AptBtrfsRootWithNoatimeError)
 
 class TestFstab(unittest.TestCase):
 
     def setUp(self):
         self.testdir = os.path.dirname(os.path.abspath(__file__))
 
-    def test_fstab_detect_snapshot(self):
+    @mock.patch('os.path.exists', side_effect=lambda f: f in ('/sbin/btrfs'))
+    def test_fstab_detect_snapshot(self, mock_commands):
         apt_btrfs = AptBtrfsSnapshot(
             fstab=os.path.join(self.testdir, "data", "fstab"))
         self.assertTrue(apt_btrfs.snapshots_supported())
@@ -38,7 +41,9 @@
         self.assertEqual(apt_btrfs._uuid_for_mountpoint("/"),
                          "UUID=fe63f598-1906-478e-acc7-f74740e78d1f")
 
-    def test_fstab_noatime(self):
+    @mock.patch('sys.stdout', new_callable=StringIO)
+    @mock.patch('sys.stderr', new_callable=StringIO)
+    def test_fstab_noatime(self, mock_stdout, mock_stderr):
         apt_btrfs = AptBtrfsSnapshot(
             fstab=os.path.join(self.testdir, "data", "fstab.bug833980"))
         # ensure our test is right