Merge lp:~gz/brz/py3_static_tuple_start into lp:brz

[Martin Packman]

Start of making _static_tuple_c compatible with Python 3

This is not everything done to make StaticTuple work on Python 3, but is a reasonable checkpoint. The code compiles, runs tests, and fails in a few relevant ways. The C API level import is also broken.

[Jelmer Vernooij (jelmer)]

review: Approve

[The Breezy Bot (the-breezy-bot)]

Running landing tests failed
http://10.242.247.184:8080/job/brz-dev/168/

[The Breezy Bot (the-breezy-bot)]

Running landing tests failed
http://10.242.247.184:8080/job/brz-dev/172/

=== modified file 'breezy/_export_c_api.h'
--- breezy/_export_c_api.h	2009-10-12 21:44:27 +0000
+++ breezy/_export_c_api.h	2017-06-25 23:38:37 +0000
@@ -15,6 +15,7 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
+/* GZ 2017-06-22: This header needs updating to use Capsules over CObjects */
 
 /* This file contains helper functions for exporting a C API for a CPython
  * extension module.
@@ -45,17 +46,14 @@
     PyObject *d = NULL;
     PyObject *c_obj = NULL;
 
-    /* (char *) is because python2.4 declares this api as 'char *' rather than
-     * const char* which it really is.
-     */
-    d = PyObject_GetAttrString(module, (char *)_C_API_NAME);
+    d = PyObject_GetAttrString(module, _C_API_NAME);
     if (!d) {
         PyErr_Clear();
         d = PyDict_New();
         if (!d)
             goto bad;
         Py_INCREF(d);
-        if (PyModule_AddObject(module, (char *)_C_API_NAME, d) < 0)
+        if (PyModule_AddObject(module, _C_API_NAME, d) < 0)
             goto bad;
     }
     c_obj = PyCObject_FromVoidPtrAndDesc(func, signature, 0);

=== modified file 'breezy/_static_tuple_c.c'
--- breezy/_static_tuple_c.c	2017-05-21 18:10:28 +0000
+++ breezy/_static_tuple_c.c	2017-06-25 23:38:37 +0000
@@ -26,12 +26,6 @@
 #include "_static_tuple_c.h"
 #include "_export_c_api.h"
 
-/* Pyrex 0.9.6.4 exports _simple_set_pyx_api as
- * import__simple_set_pyx(), while Pyrex 0.9.8.5 and Cython 0.11.3 export them
- * as import_breezy___simple_set_pyx(). As such, we just #define one to be
- * equivalent to the other in our internal code.
- */
-#define import__simple_set_pyx import_breezy___simple_set_pyx
 #include "_simple_set_pyx_api.h"
 
 #if defined(__GNUC__)
@@ -242,11 +236,14 @@
                 " should not have a NULL entry.");
             return 0;
         }
-        if (PyString_CheckExact(obj)
+        if (PyBytes_CheckExact(obj)
             || StaticTuple_CheckExact(obj)
             || obj == Py_None
             || PyBool_Check(obj)
+#if PY_MAJOR_VERSION >= 3
+#else
             || PyInt_CheckExact(obj)
+#endif
             || PyLong_CheckExact(obj)
             || PyFloat_CheckExact(obj)
             || PyUnicode_CheckExact(obj)
@@ -314,8 +311,12 @@
     if (tuple_repr == NULL) {
         return NULL;
     }
+#if PY_MAJOR_VERSION >= 3
+    result = PyUnicode_FromFormat("StaticTuple%U", tuple_repr);
+#else
     result = PyString_FromFormat("StaticTuple%s",
                                  PyString_AsString(tuple_repr));
+#endif
     return result;
 }
 
@@ -362,7 +363,7 @@
 {
     PyObject *vt;
     PyObject *result = NULL;
-    
+
     vt = StaticTuple_as_tuple((StaticTuple *)v);
     if (vt == NULL) {
         goto done;
@@ -478,7 +479,7 @@
     vlen = v_st->size;
     wlen = w_st->size;
     min_len = (vlen < wlen) ? vlen : wlen;
-    string_richcompare = PyString_Type.tp_richcompare;
+    string_richcompare = PyBytes_Type.tp_richcompare;
     for (i = 0; i < min_len; i++) {
         PyObject *result = NULL;
         v_obj = StaticTuple_GET_ITEM(v_st, i);
@@ -487,7 +488,7 @@
             /* Shortcut case, these must be identical */
             continue;
         }
-        if (PyString_CheckExact(v_obj) && PyString_CheckExact(w_obj)) {
+        if (PyBytes_CheckExact(v_obj) && PyBytes_CheckExact(w_obj)) {
             result = string_richcompare(v_obj, w_obj, Py_EQ);
         } else if (StaticTuple_CheckExact(v_obj) &&
                    StaticTuple_CheckExact(w_obj))
@@ -547,7 +548,7 @@
         return Py_True;
     }
     /* It is some other comparison, go ahead and do the real check. */
-    if (PyString_CheckExact(v_obj) && PyString_CheckExact(w_obj))
+    if (PyBytes_CheckExact(v_obj) && PyBytes_CheckExact(w_obj))
     {
         return string_richcompare(v_obj, w_obj, op);
     } else if (StaticTuple_CheckExact(v_obj) &&
@@ -679,6 +680,8 @@
     return obj;
 }
 
+#if PY_MAJOR_VERSION >= 3
+#else
 static PyObject *
 StaticTuple_slice(StaticTuple *self, Py_ssize_t ilow, Py_ssize_t ihigh)
 {
@@ -692,6 +695,21 @@
     Py_DECREF(as_tuple);
     return result;
 }
+#endif
+
+static PyObject *
+StaticTuple_subscript(StaticTuple *self, PyObject *key)
+{
+    PyObject *as_tuple, *result;
+
+    as_tuple = StaticTuple_as_tuple(self);
+    if (as_tuple == NULL) {
+        return NULL;
+    }
+    result = PyTuple_Type.tp_as_mapping->mp_subscript(as_tuple, key);
+    Py_DECREF(as_tuple);
+    return result;
+}
 
 static int
 StaticTuple_traverse(StaticTuple *self, visitproc visit, void *arg)
@@ -759,29 +777,36 @@
     0,                              /* nb_or */
     0,                              /* nb_coerce */
 };
-    
+
 
 static PySequenceMethods StaticTuple_as_sequence = {
     (lenfunc)StaticTuple_length,            /* sq_length */
     0,                              /* sq_concat */
     0,                              /* sq_repeat */
     (ssizeargfunc)StaticTuple_item,         /* sq_item */
+#if PY_MAJOR_VERSION >= 3
+#else
     (ssizessizeargfunc)StaticTuple_slice,   /* sq_slice */
+#endif
     0,                              /* sq_ass_item */
     0,                              /* sq_ass_slice */
     0,                              /* sq_contains */
-};
-
-/* TODO: Implement StaticTuple_as_mapping.
- *       The only thing we really want to support from there is mp_subscript,
- *       so that we could support extended slicing (foo[::2]). Not worth it
- *       yet, though.
- */
+#if PY_MAJOR_VERSION >= 3
+    0,                              /* sq_inplace_concat */
+    0,                              /* sq_inplace_repeat */
+#endif
+};
+
+
+static PyMappingMethods StaticTuple_as_mapping = {
+    (lenfunc)StaticTuple_length,            /* mp_length */
+    (binaryfunc)StaticTuple_subscript,      /* mp_subscript */
+    0,                                      /* mp_ass_subscript */
+};
 
 
 PyTypeObject StaticTuple_Type = {
-    PyObject_HEAD_INIT(NULL)
-    0,                                           /* ob_size */
+    PyVarObject_HEAD_INIT(NULL, 0)
     "breezy._static_tuple_c.StaticTuple",        /* tp_name */
     sizeof(StaticTuple),                         /* tp_basicsize */
     sizeof(PyObject *),                          /* tp_itemsize */
@@ -793,7 +818,7 @@
     (reprfunc)StaticTuple_repr,                  /* tp_repr */
     &StaticTuple_as_number,                      /* tp_as_number */
     &StaticTuple_as_sequence,                    /* tp_as_sequence */
-    0,                                           /* tp_as_mapping */
+    &StaticTuple_as_mapping,                     /* tp_as_mapping */
     (hashfunc)StaticTuple_hash,                  /* tp_hash */
     0,                                           /* tp_call */
     0,                                           /* tp_str */
@@ -887,72 +912,32 @@
 }
 
 
-static int
-_workaround_pyrex_096(void)
-{
-    /* Work around an incompatibility in how pyrex 0.9.6 exports a module,
-     * versus how pyrex 0.9.8 and cython 0.11 export it.
-     * Namely 0.9.6 exports import__simple_set_pyx and tries to
-     * "import _simple_set_pyx" but it is available only as
-     * "import breezy._simple_set_pyx"
-     * It is a shame to hack up sys.modules, but that is what we've got to do.
-     */
-    PyObject *sys_module = NULL, *modules = NULL, *set_module = NULL;
-    int retval = -1;
-
-    /* Clear out the current ImportError exception, and try again. */
-    PyErr_Clear();
-    /* Note that this only seems to work if somewhere else imports
-     * breezy._simple_set_pyx before importing breezy._static_tuple_c
-     */
-    set_module = PyImport_ImportModule("breezy._simple_set_pyx");
-    if (set_module == NULL) {
-        goto end;
-    }
-    /* Add the _simple_set_pyx into sys.modules at the appropriate location. */
-    sys_module = PyImport_ImportModule("sys");
-    if (sys_module == NULL) {
-        goto end;
-    }
-    modules = PyObject_GetAttrString(sys_module, "modules");
-    if (modules == NULL || !PyDict_Check(modules)) {
-        goto end;
-    }
-    PyDict_SetItemString(modules, "_simple_set_pyx", set_module);
-    /* Now that we have hacked it in, try the import again. */
-    retval = import_breezy___simple_set_pyx();
-end:
-    Py_XDECREF(set_module);
-    Py_XDECREF(sys_module);
-    Py_XDECREF(modules);
-    return retval;
-}
-
-
-PyMODINIT_FUNC
-init_static_tuple_c(void)
+PYMOD_INIT_FUNC(_static_tuple_c)
 {
     PyObject* m;
 
     StaticTuple_Type.tp_getattro = PyObject_GenericGetAttr;
-    if (PyType_Ready(&StaticTuple_Type) < 0)
-        return;
+    if (PyType_Ready(&StaticTuple_Type) < 0) {
+        return PYMOD_ERROR;
+    }
 
-    m = Py_InitModule3("_static_tuple_c", static_tuple_c_methods,
-                       "C implementation of a StaticTuple structure");
-    if (m == NULL)
-      return;
+    PYMOD_CREATE(m, "_static_tuple_c",
+                 "C implementation of a StaticTuple structure",
+                 static_tuple_c_methods);
+    if (m == NULL) {
+      return PYMOD_ERROR;
+    }
 
     Py_INCREF(&StaticTuple_Type);
     PyModule_AddObject(m, "StaticTuple", (PyObject *)&StaticTuple_Type);
-    if (import_breezy___simple_set_pyx() == -1
-        && _workaround_pyrex_096() == -1)
-    {
-        return;
+    if (import_breezy___simple_set_pyx() == -1) {
+        return PYMOD_ERROR;
     }
     setup_interned_tuples(m);
     setup_empty_tuple(m);
     setup_c_api(m);
+
+    return PYMOD_SUCCESS(m);
 }
 
 // vim: tabstop=4 sw=4 expandtab

=== modified file 'breezy/python-compat.h'
--- breezy/python-compat.h	2017-06-05 23:59:08 +0000
+++ breezy/python-compat.h	2017-06-25 23:38:37 +0000
@@ -25,6 +25,37 @@
 #ifndef _BZR_PYTHON_COMPAT_H
 #define _BZR_PYTHON_COMPAT_H
 
+#if PY_MAJOR_VERSION >= 3
+
+#define PyInt_FromSsize_t PyLong_FromSsize_t
+
+/* In Python 3 the Py_TPFLAGS_CHECKTYPES behaviour is on by default */
+#define Py_TPFLAGS_CHECKTYPES 0
+
+#define PYMOD_ERROR NULL
+#define PYMOD_SUCCESS(val) val
+#define PYMOD_INIT_FUNC(name) PyMODINIT_FUNC PyInit_##name(void)
+#define PYMOD_CREATE(ob, name, doc, methods) do { \
+    static struct PyModuleDef moduledef = { \
+        PyModuleDef_HEAD_INIT, name, doc, -1, methods \
+    }; \
+    ob = PyModule_Create(&moduledef); \
+    } while(0)
+
+#else
+
+#define PyBytes_Type PyString_Type
+#define PyBytes_CheckExact PyString_CheckExact
+
+#define PYMOD_ERROR
+#define PYMOD_SUCCESS(val)
+#define PYMOD_INIT_FUNC(name) void init##name(void)
+#define PYMOD_CREATE(ob, name, doc, methods) do { \
+    ob = Py_InitModule3(name, methods, doc); \
+    } while(0)
+
+#endif
+
 #if defined(_WIN32) || defined(WIN32)
     /* Defining WIN32_LEAN_AND_MEAN makes including windows quite a bit
      * lighter weight.

=== modified file 'breezy/tests/test__static_tuple.py'
--- breezy/tests/test__static_tuple.py	2017-05-23 14:08:03 +0000
+++ breezy/tests/test__static_tuple.py	2017-06-25 23:38:37 +0000
@@ -16,7 +16,10 @@
 
 """Tests for the StaticTuple type."""
 
-import cPickle
+try:
+    import cPickle as pickle
+except ImportError:
+    import pickle
 import sys
 
 from breezy import (
@@ -26,6 +29,10 @@
     static_tuple,
     tests,
     )
+from breezy.sixish import (
+    PY3,
+    text_type,
+    )
 from breezy.tests import (
     features,
     )
@@ -213,6 +220,8 @@
         self.assertRaises(TypeError, self.module.StaticTuple, subint(2))
 
     def test_holds_long(self):
+        if PY3:
+            self.skipTest("No long type on Python 3")
         k1 = self.module.StaticTuple(2**65)
         class sublong(long):
             pass
@@ -225,15 +234,15 @@
             pass
         self.assertRaises(TypeError, self.module.StaticTuple, subfloat(1.5))
 
-    def test_holds_str(self):
-        k1 = self.module.StaticTuple('astring')
-        class substr(str):
+    def test_holds_bytes(self):
+        k1 = self.module.StaticTuple(b'astring')
+        class substr(bytes):
             pass
-        self.assertRaises(TypeError, self.module.StaticTuple, substr('a'))
+        self.assertRaises(TypeError, self.module.StaticTuple, substr(b'a'))
 
     def test_holds_unicode(self):
         k1 = self.module.StaticTuple(u'\xb5')
-        class subunicode(unicode):
+        class subunicode(text_type):
             pass
         self.assertRaises(TypeError, self.module.StaticTuple,
                           subunicode(u'\xb5'))
@@ -416,16 +425,8 @@
         k = self.module.StaticTuple('foo', 'bar', 'baz', 'bing')
         self.assertEqual(('foo', 'bar'), k[:2])
         self.assertEqual(('baz',), k[2:-1])
-        try:
-            val = k[::2]
-        except TypeError:
-            # C implementation raises a TypeError, we don't need the
-            # implementation yet, so allow this to pass
-            pass
-        else:
-            # Python implementation uses a regular Tuple, so make sure it gives
-            # the right result
-            self.assertEqual(('foo', 'baz'), val)
+        self.assertEqual(('foo', 'baz',), k[::2])
+        self.assertRaises(TypeError, k.__getitem__, 'not_slice')
 
     def test_referents(self):
         # We implement tp_traverse so that things like 'meliae' can measure the
@@ -582,20 +583,20 @@
 
     def test_pickle(self):
         st = self.module.StaticTuple('foo', 'bar')
-        pickled = cPickle.dumps(st)
-        unpickled = cPickle.loads(pickled)
+        pickled = pickle.dumps(st)
+        unpickled = pickle.loads(pickled)
         self.assertEqual(unpickled, st)
 
     def test_pickle_empty(self):
         st = self.module.StaticTuple()
-        pickled = cPickle.dumps(st)
-        unpickled = cPickle.loads(pickled)
+        pickled = pickle.dumps(st)
+        unpickled = pickle.loads(pickled)
         self.assertIs(st, unpickled)
 
     def test_pickle_nested(self):
         st = self.module.StaticTuple('foo', self.module.StaticTuple('bar'))
-        pickled = cPickle.dumps(st)
-        unpickled = cPickle.loads(pickled)
+        pickled = pickle.dumps(st)
+        unpickled = pickle.loads(pickled)
         self.assertEqual(unpickled, st)
 
     def test_static_tuple_thunk(self):