Merge lp:~jderose/filestore/py3.3 into lp:filestore

[Jason Gerard DeRose]

For details, see this bug:

  https://bugs.launchpad.net/filestore/+bug/1129090

Changes include:

* Dropped back-ported posix_fadvise() C extension as now we can just use os.posix_fadvise() from the standard lib

* FileStore.allocate_tmp() and FileStore.allocate_partial() now take advantage of new 'xb' mode for open()

* FileStore.random_tmp_path() now uses dbase32.random_id()

* setup.py now enforces python3 >= 3.3

* Updated debian/control to require python3 >= 3.3, build-depend and depend on python3-dbase32

* Removed filestore-crashdb.conf, and updated source_filestore.py to provide the same info as we only need Apport 2.5+ compatibility now

* Added missing debian/watch file

* Added filestore.tests.run entry point for running the tests against the installed package like this:

  python3 -m filestore.tests.run

[David Jordan (dmj726)]

Looks good, approved.

review: Approve

=== modified file '.bzrignore'
--- .bzrignore	2011-08-13 06:47:34 +0000
+++ .bzrignore	2013-02-18 13:31:28 +0000
@@ -1,2 +1,2 @@
-__pycache__
-doc/_build/*
+build/
+doc/_build/

=== modified file '_filestore.c'
--- _filestore.c	2013-02-16 01:28:23 +0000
+++ _filestore.c	2013-02-18 13:31:28 +0000
@@ -22,26 +22,6 @@
     return 1;
 }
 
-static PyObject *
-filestore_posix_fadvise(PyObject *self, PyObject *args)
-{
-    off_t len, offset;
-    int res, fd, advice;
-
-    if (!PyArg_ParseTuple(args, "iO&O&i:posix_fadvise",
-            &fd, _parse_off_t, &offset, _parse_off_t, &len, &advice))
-        return NULL;
-
-    Py_BEGIN_ALLOW_THREADS
-    res = posix_fadvise(fd, offset, len, advice);
-    Py_END_ALLOW_THREADS
-    if (res != 0) {
-        errno = res;
-        return filestore_error();
-    }
-    Py_RETURN_NONE;
-}
-
 
 /* Our custom fallocate */
 static PyObject *
@@ -121,8 +101,6 @@
 static struct PyMethodDef filestore_functions[] = {
     {"fallocate", filestore_fallocate, METH_VARARGS,
      "fallocate(fd, mode, offset, len)"},
-    {"posix_fadvise", filestore_posix_fadvise, METH_VARARGS,
-     "posix_fadvise(fd, offset, len, advice)"},
     {"fastread", filestore_fastread, METH_VARARGS,
      "fastread(fd, leaf_index, leaf_size)"},
     {NULL, NULL, 0, NULL}

=== modified file 'debian/control'
--- debian/control	2012-11-01 05:37:37 +0000
+++ debian/control	2013-02-18 13:31:28 +0000
@@ -3,9 +3,10 @@
 Priority: optional
 Maintainer: Jason Gerard DeRose <[e-mail]>
 Build-Depends: debhelper (>= 9),
-    python3-all-dev (>= 3.2.3),
+    python3-all-dev (>= 3.3),
+    python3-sphinx,
     python3-skein (>= 0.7.1),
-    python3-sphinx (>= 1.1),
+    python3-dbase32 (>= 0.3),
 Standards-Version: 3.9.3
 X-Python3-Version: >= 3.2
 Homepage: https://launchpad.net/filestore
@@ -13,7 +14,8 @@
 Package: python3-filestore
 Architecture: any
 Depends: ${python3:Depends}, ${shlibs:Depends}, ${misc:Depends},
-    python3-skein (>= 0.7.1)
+    python3-skein (>= 0.7.1),
+    python3-dbase32 (>= 0.3),
 Description: Dmedia hashing protocol and file layout
  This is the heart of Dmedia.  It has been split out of Dmedia to make it easier
  to review, and in hopes that other apps might use the hashing protocol and

=== removed file 'debian/filestore-crashdb.conf'
--- debian/filestore-crashdb.conf	2012-09-07 00:29:56 +0000
+++ debian/filestore-crashdb.conf	1970-01-01 00:00:00 +0000
@@ -1,4 +0,0 @@
-filestore = {
-    'impl': 'launchpad',
-    'project': 'filestore',
-}

=== modified file 'debian/python3-filestore.install'
--- debian/python3-filestore.install	2012-09-07 00:29:56 +0000
+++ debian/python3-filestore.install	2013-02-18 13:31:28 +0000
@@ -1,2 +1,1 @@
-debian/filestore-crashdb.conf etc/apport/crashdb.conf.d/
 debian/source_filestore.py usr/share/apport/package-hooks/

=== modified file 'debian/source_filestore.py'
--- debian/source_filestore.py	2012-09-07 00:29:56 +0000
+++ debian/source_filestore.py	2013-02-18 13:31:28 +0000
@@ -1,9 +1,10 @@
-'''apport package hook for filestore.
+"""
+Apport package hook for filestore (requires Apport 2.5 or newer).
 
 (c) 2012 Novacut Inc
 Author: Jason Gerard DeRose <[e-mail]>
-'''
+"""
 
 def add_info(report):
-    report['CrashDB'] = 'filestore'
+    report['CrashDB'] = "{'impl': 'launchpad', 'project': 'filestore'}"
 

=== added file 'debian/watch'
--- debian/watch	1970-01-01 00:00:00 +0000
+++ debian/watch	2013-02-18 13:31:28 +0000
@@ -0,0 +1,2 @@
+version=3
+https://launchpad.net/filestore/+download https://launchpad.net/filestore/.*/filestore-(.+).tar.gz

=== removed directory 'doc/_build'
=== modified file 'filestore/__init__.py'
--- filestore/__init__.py	2013-02-16 04:18:54 +0000
+++ filestore/__init__.py	2013-02-18 13:31:28 +0000
@@ -82,17 +82,16 @@
 from collections import namedtuple
 import logging
 
+from dbase32 import random_id
+
 from .protocols import VERSION0, VERSION1
 
 try:
-    from _filestore import fallocate, posix_fadvise, fastread
+    from _filestore import fallocate, fastread
 except ImportError:
     def fallocate(fd, mode, offset, length):
         os.ftruncate(fd, length)
 
-    def posix_fadvise(fd, offset, length, advice):
-        pass
-
     def fastread(fd, index, size):
         return os.read(fd, size)
 
@@ -146,10 +145,6 @@
     https://launchpad.net/dmedia
 """
 
-POSIX_FADV_SEQUENTIAL = 2
-POSIX_FADV_WILLNEED = 3
-POSIX_FADV_DONTNEED = 4
-
 
 ##############################
 # The Dmedia Hashing Protocol:
@@ -602,7 +597,7 @@
         fd = src_fp.fileno()
         st = os.fstat(fd)
         if st.st_size > LEAF_SIZE:
-            posix_fadvise(fd, 0, st.st_size, POSIX_FADV_SEQUENTIAL)
+            os.posix_fadvise(fd, 0, st.st_size, os.POSIX_FADV_SEQUENTIAL)
         index = 0
         while True:
             data = fastread(fd, index, LEAF_SIZE)
@@ -668,7 +663,7 @@
             log.info('Opening %r', file)
             src_fp = open(file.name, 'rb')
             fd = src_fp.fileno()
-            posix_fadvise(fd, 0, file.size, POSIX_FADV_SEQUENTIAL)
+            os.posix_fadvise(fd, 0, file.size, os.POSIX_FADV_SEQUENTIAL)
             index = 0
             while True:
                 data = fastread(fd, index, LEAF_SIZE)
@@ -1044,8 +1039,7 @@
         If this file will be opened by another application, you should try and
         do the same, using the O_CREAT and O_EXCL flags with OPEN(2).
         """
-        name = b32encode(os.urandom(15)).decode('utf-8')
-        return path.join(self.tmp, name)
+        return path.join(self.tmp, random_id())
 
     def exists(self, _id):
         """
@@ -1258,10 +1252,8 @@
             raise ValueError(
                 'Need size >= 1; got size={}'.format(size)
             )
-        rand = b32encode(os.urandom(15)).decode('utf-8')
         filename = self.random_tmp_path()
-        # FIXME: open in mode 'xb' once we support only Python 3.3 and newer:
-        tmp_fp = open(filename, 'wb')
+        tmp_fp = open(filename, 'xb')
         fd = tmp_fp.fileno()
         if size is not None:
             try:
@@ -1289,14 +1281,11 @@
         filename = self.partial_path(_id)
         try:
             tmp_fp = open(filename, 'rb+')
-            reopened = True
+            fd = tmp_fp.fileno()
+            os.ftruncate(fd, size)
         except IOError:
-            tmp_fp = open(filename, 'wb')
-            reopened = False
-        fd = tmp_fp.fileno()
-        if reopened:
-            os.ftruncate(fd, size)
-        else:
+            tmp_fp = open(filename, 'xb')
+            fd = tmp_fp.fileno()
             try:
                 fallocate(fd, 0, 0, size)
             except OSError:

=== modified file 'filestore/tests/__init__.py'
--- filestore/tests/__init__.py	2013-02-16 02:35:34 +0000
+++ filestore/tests/__init__.py	2013-02-18 13:31:28 +0000
@@ -35,6 +35,7 @@
 from random import SystemRandom
 
 from skein import skein512
+from dbase32 import isdb32
 
 from filestore import protocols
 from filestore.protocols import PROTOCOLS, VERSION0, VERSION1
@@ -1811,9 +1812,7 @@
         self.assertEqual(path.dirname(filename), fs.tmp)
         name = path.basename(filename)
         self.assertEqual(len(name), 24)
-        self.assertEqual(name,
-            b32encode(b32decode(name.encode('utf-8'))).decode('utf-8')
-        )
+        self.assertTrue(isdb32(name))
         self.assertEqual(path.abspath(filename), filename)
 
     def test_exists(self):
@@ -2299,7 +2298,7 @@
         # Test with size=None
         fp1 = fs.allocate_tmp()
         self.assertIsInstance(fp1, io.BufferedWriter)
-        self.assertEqual(fp1.mode, 'wb')
+        self.assertEqual(fp1.mode, 'xb')
         self.assertEqual(os.fstat(fp1.fileno()).st_size, 0)
         self.assertEqual(stat.S_IMODE(os.fstat(fp1.fileno()).st_mode), 0o666)
         self.assertTrue(fp1.name.startswith(t + os.sep))
@@ -2307,7 +2306,7 @@
         # Test with size=2311
         fp2 = fs.allocate_tmp(2311)
         self.assertIsInstance(fp2, io.BufferedWriter)
-        self.assertEqual(fp2.mode, 'wb')
+        self.assertEqual(fp2.mode, 'xb')
         self.assertEqual(os.fstat(fp2.fileno()).st_size, 2311)
         self.assertEqual(stat.S_IMODE(os.fstat(fp2.fileno()).st_mode), 0o666)
         self.assertTrue(fp2.name.startswith(t + os.sep))
@@ -2332,7 +2331,7 @@
         # But make sure that size=1 works:
         fp3 = fs.allocate_tmp(1)
         self.assertIsInstance(fp3, io.BufferedWriter)
-        self.assertEqual(fp3.mode, 'wb')
+        self.assertEqual(fp3.mode, 'xb')
         self.assertEqual(os.fstat(fp3.fileno()).st_size, 1)
         self.assertEqual(stat.S_IMODE(os.fstat(fp3.fileno()).st_mode), 0o666)
         self.assertTrue(fp3.name.startswith(t + os.sep))
@@ -2349,7 +2348,7 @@
         fp = fs.allocate_partial(2311, _id)
         self.assertEqual(fp.name, filename)
         self.assertIsInstance(fp, io.BufferedWriter)
-        self.assertEqual(fp.mode, 'wb')
+        self.assertEqual(fp.mode, 'xb')
         self.assertEqual(os.fstat(fp.fileno()).st_size, 2311)
         self.assertEqual(stat.S_IMODE(os.fstat(fp.fileno()).st_mode), 0o666)
 
@@ -2361,23 +2360,22 @@
         fp = fs.allocate_partial(2311, _id)
         self.assertEqual(fp.name, filename)
         self.assertIsInstance(fp, io.BufferedRandom)
-        self.assertIn(fp.mode, ('r+b', 'rb+'))
+        self.assertEqual(fp.mode, 'rb+')
         self.assertEqual(os.fstat(fp.fileno()).st_size, 2311)
         self.assertEqual(stat.S_IMODE(os.fstat(fp.fileno()).st_mode), 0o666)
         fp.close()
 
-        open(filename, 'wb').write(b'a' * 200)  # Truncate to smaller than size
+        open(filename, 'wb+').write(b'a' * 200)  # Truncate to smaller than size
 
         # Test with pre-existing file smaller than size:
         self.assertEqual(os.stat(filename).st_size, 200)
         fp = fs.allocate_partial(2311, _id)
         self.assertEqual(fp.name, filename)
         self.assertIsInstance(fp, io.BufferedRandom)
-        self.assertIn(fp.mode, ('r+b', 'rb+'))
+        self.assertEqual(fp.mode, 'rb+')
         self.assertEqual(os.fstat(fp.fileno()).st_size, 2311)
         self.assertEqual(stat.S_IMODE(os.fstat(fp.fileno()).st_mode), 0o666)
 
-
         _id = random_id()  # We'll use a new ID for below
         filename = tmp.join('.dmedia', 'partial', _id)
 
@@ -2401,14 +2399,14 @@
         fp = fs.allocate_partial(1, _id)
         self.assertEqual(fp.name, filename)
         self.assertIsInstance(fp, io.BufferedWriter)
-        self.assertEqual(fp.mode, 'wb')
+        self.assertEqual(fp.mode, 'xb')
         self.assertEqual(os.fstat(fp.fileno()).st_size, 1)
         self.assertEqual(stat.S_IMODE(os.fstat(fp.fileno()).st_mode), 0o666)
         fp.close()
         fp = fs.allocate_partial(1, _id)
         self.assertEqual(fp.name, filename)
         self.assertIsInstance(fp, io.BufferedRandom)
-        self.assertIn(fp.mode, ('r+b', 'rb+'))
+        self.assertEqual(fp.mode, 'rb+')
         self.assertEqual(os.fstat(fp.fileno()).st_size, 1)
         self.assertEqual(stat.S_IMODE(os.fstat(fp.fileno()).st_mode), 0o666)
 

=== added file 'filestore/tests/run.py'
--- filestore/tests/run.py	1970-01-01 00:00:00 +0000
+++ filestore/tests/run.py	2013-02-18 13:31:28 +0000
@@ -0,0 +1,70 @@
+# filestore: dmedia hashing protocol and file layout
+# Copyright (C) 2012 Novacut Inc
+#
+# This file is part of `filestore`.
+#
+# `filestore` is free software: you can redistribute it and/or modify it under
+# the terms of the GNU Affero General Public License as published by the Free
+# Software Foundation, either version 3 of the License, or (at your option) any
+# later version.
+#
+# `filestore` is distributed in the hope that it will be useful, but WITHOUT ANY
+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
+# A PARTICULAR PURPOSE.  See the GNU Affero General Public License for more
+# details.
+#
+# You should have received a copy of the GNU Affero General Public License along
+# with `filestore`.  If not, see <http://www.gnu.org/licenses/>.
+#
+# Authors:
+#   Jason Gerard DeRose <[e-mail]>
+
+"""
+Run `filestore` unit tests.
+"""
+import sys
+from os import path
+from unittest import TestLoader, TextTestRunner
+from doctest import DocTestSuite
+
+import filestore
+
+
+pynames = (
+    'filestore',
+    'filestore.protocols',
+    'filestore.misc',
+    'filestore.tests',
+    'filestore.tests.test_protocols',
+    'filestore.tests.test_misc'
+)
+
+
+def run_tests():
+    # Add unit-tests:
+    loader = TestLoader()
+    suite = loader.loadTestsFromNames(pynames)
+
+    # Add doc-tests:
+    for name in pynames:
+        suite.addTest(DocTestSuite(name))
+
+    # Run the tests:
+    runner = TextTestRunner(verbosity=2)
+    result = runner.run(suite)
+    success =  result.wasSuccessful()
+    print(
+        'filestore: {!r}'.format(path.abspath(filestore.__file__)),
+        file=sys.stderr
+    )
+    if success:
+        print('Tests passed.', file=sys.stderr)
+    else:
+        print('Tests FAILED!', file=sys.stderr)
+    return success
+
+
+if __name__ == '__main__':
+    if not run_tests():
+        raise SystemExit('2')
+

=== modified file 'setup.py'
--- setup.py	2012-12-30 11:29:17 +0000
+++ setup.py	2013-02-18 13:31:28 +0000
@@ -26,15 +26,14 @@
 """
 
 import sys
-if sys.version_info < (3, 2):
-    sys.exit('filestore requires Python 3.2 or newer')
+if sys.version_info < (3, 3):
+    sys.exit('filestore requires Python 3.3 or newer')
 
 from distutils.core import setup, Extension
 from distutils.cmd import Command
-from unittest import TestLoader, TextTestRunner
-from doctest import DocTestSuite
 
 import filestore
+from filestore.tests.run import run_tests
 
 
 class Test(Command):
@@ -49,27 +48,7 @@
         pass
 
     def run(self):
-        pynames = [
-            'filestore',
-            'filestore.protocols',
-            'filestore.misc',
-            'filestore.tests',
-            'filestore.tests.test_protocols',
-            'filestore.tests.test_misc'
-        ]
-
-        # Add unit-tests:
-        loader = TestLoader()
-        suite = loader.loadTestsFromNames(pynames)
-
-        # Add doc-tests:
-        for name in pynames:
-            suite.addTest(DocTestSuite(name))
-
-        # Run the tests:
-        runner = TextTestRunner(verbosity=2)
-        result = runner.run(suite)
-        if not result.wasSuccessful():
+        if not run_tests():
             raise SystemExit(2)
 
 
@@ -84,7 +63,10 @@
     author='Jason Gerard DeRose',
     author_email='[e-mail]',
     license='AGPLv3+',
-    packages=['filestore', 'filestore.tests'],
+    packages=[
+        'filestore',
+        'filestore.tests'
+    ],
     package_data={'filestore': ['data/test-vectors.json']},
     scripts=['dmediasum'],
     cmdclass={'test': Test},