[GR-52191] Fix cpyext test errors on windows

PullRequest: graalpython/3233

Author: msimacek

graalpython/com.oracle.graal.python.cext/src/bytesobject.c

@@ -2839,8 +2839,8 @@ _Py_COMP_DIAG_POP
 }
 #endif // GraalPy change
 
-// GraalPy change: different signature, uses C array instead of bytes
-PyObject *
+// GraalPy change: export for downcall, uses C array instead of bytes
+PyAPI_FUNC(PyObject *)
 bytes_subtype_new(PyTypeObject *type, int8_t* contents, Py_ssize_t n) {
     // GraalPy change: different implementation
     PyObject* bytes = type->tp_alloc(type, n);

graalpython/com.oracle.graal.python.cext/src/exceptions.c

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * The Universal Permissive License (UPL), Version 1.0
@@ -52,7 +52,8 @@ void initialize_exceptions() {
 #undef EXCEPTION
 }
 
-PyObject* exception_subtype_new(PyTypeObject *type, PyObject *args) {
+PyAPI_FUNC(PyObject *)
+exception_subtype_new(PyTypeObject *type, PyObject *args) {
     PyBaseExceptionObject *self;
 
     self = (PyBaseExceptionObject *)type->tp_alloc(type, 0);

graalpython/com.oracle.graal.python.cext/src/memoryobject.c

@@ -3320,11 +3320,15 @@ PyTypeObject PyMemoryView_Type = {
 
 // GraalPy additions
 /* called from memoryview implementation to do pointer arithmetics currently not possible from Java */
-int8_t* truffle_add_suboffset(int8_t *ptr, Py_ssize_t offset, Py_ssize_t suboffset) {
+PyAPI_FUNC(int8_t *)
+truffle_add_suboffset(int8_t *ptr, Py_ssize_t offset, Py_ssize_t suboffset)
+{
         return *(int8_t**)(ptr + offset) + suboffset;
 }
 
-PyObject* PyTruffle_MemoryViewFromObject(PyObject *v, int flags) {
+PyAPI_FUNC(PyObject *)
+PyTruffle_MemoryViewFromObject(PyObject *v, int flags)
+{
     if (PyObject_CheckBuffer(v)) {
         Py_buffer* buffer = malloc(sizeof(Py_buffer));
         if (PyObject_GetBuffer(v, buffer, flags) < 0) {
@@ -3364,7 +3368,9 @@ PyObject* PyTruffle_MemoryViewFromObject(PyObject *v, int flags) {
 }
 
 /* Release buffer struct allocated in PyTruffle_MemoryViewFromObject */
-void PyTruffle_ReleaseBuffer(Py_buffer* buffer) {
+PyAPI_FUNC(void)
+PyTruffle_ReleaseBuffer(Py_buffer* buffer)
+{
     if (buffer->obj != NULL) {
         PyBufferProcs *pb;
         pb = Py_TYPE(buffer->obj)->tp_as_buffer;

graalpython/com.oracle.graal.python.cext/src/modsupport.c

@@ -68,7 +68,7 @@ static int getbuffer(PyObject *arg, Py_buffer *view, const char **errmsg) {
     return 0;
 }
 
-int get_buffer_r(PyObject *arg, Py_buffer *view) {
+PyAPI_FUNC(int) get_buffer_r(PyObject *arg, Py_buffer *view) {
     if (PyObject_GetBuffer(arg, view, PyBUF_SIMPLE) != 0) {
         return -1;
     }
@@ -79,7 +79,7 @@ int get_buffer_r(PyObject *arg, Py_buffer *view) {
     return 0;
 }
 
-int get_buffer_rw(PyObject *arg, Py_buffer *view) {
+PyAPI_FUNC(int) get_buffer_rw(PyObject *arg, Py_buffer *view) {
     if (PyObject_GetBuffer(arg, view, PyBUF_WRITABLE) != 0) {
         PyErr_Clear();
         return -1;
@@ -91,7 +91,7 @@ int get_buffer_rw(PyObject *arg, Py_buffer *view) {
     return 0;
 }
 
-Py_ssize_t convertbuffer(PyObject *arg, const void **p) {
+PyAPI_FUNC(Py_ssize_t) convertbuffer(PyObject *arg, const void **p) {
     PyBufferProcs *pb = Py_TYPE(arg)->tp_as_buffer;
     Py_ssize_t count;
     Py_buffer view;

graalpython/com.oracle.graal.python.cext/src/tupleobject.c

@@ -758,7 +758,7 @@ tuple_vectorcall(PyObject *type, PyObject * const*args,
 
 PyObject* PyTruffle_Tuple_Alloc(PyTypeObject* cls, Py_ssize_t nitems);
 
-PyObject * // GraalPy change: not static
+PyAPI_FUNC(PyObject *) // GraalPy change: export for downcall
 tuple_subtype_new(PyTypeObject *type, PyObject *iterable)
 {
     // GraalPy change: different implementation

graalpython/com.oracle.graal.python.cext/src/unicodeobject.c

@@ -14461,7 +14461,7 @@ unicode_new_impl(PyTypeObject *type, PyObject *x, const char *encoding,
 }
 #endif // GraalPy change
 
-PyObject * // GraalPy change: not static
+PyAPI_FUNC(PyObject *) // GraalPy change: export for downcall
 unicode_subtype_new(PyTypeObject *type, PyObject *unicode)
 {
     // GraalPy change: temporarily define struct access macros

graalpython/com.oracle.graal.python.test/src/tests/cpyext/__init__.py

@@ -36,14 +36,17 @@
 # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 # SOFTWARE.
-import os
 from importlib import invalidate_caches
-from io import StringIO
-from pathlib import Path
-from string import Formatter
 
 import gc
+import os
+import shutil
 import sys
+import unittest
+from copy import deepcopy
+from io import StringIO
+from pathlib import Path
+from string import Formatter
 
 DIR = Path(__file__).parent.absolute()
 
@@ -81,23 +84,14 @@ def unhandled_error_compare_with_message(x, y):
     else:
         return x == y
 
-class CPyExtTestCase():
 
-    def setUpClass(self):
-        for typ in type(self).mro():
-            for k, v in typ.__dict__.items():
-                if k.startswith("test_"):
-                    modname = k.replace("test_", "")
-                    if k.startswith("test_graalpython_"):
-                        if not GRAALPYTHON:
-                            continue
-                        else:
-                            modname = k.replace("test_graalpython_", "")
-                    self.compile_module(modname)
+class CPyExtTestCase(unittest.TestCase):
+    pass
 
 
 compiled_registry = set()
 
+
 def ccompile(self, name, check_duplicate_name=True):
     from distutils.core import setup, Extension
     from distutils.sysconfig import get_config_var
@@ -115,19 +109,21 @@ def ccompile(self, name, check_duplicate_name=True):
             m.update(block)
     cur_checksum = m.hexdigest()
 
+    build_dir = DIR / 'build' / name
+
     # see if there is already a checksum file
-    checksum_file = DIR / f'{name}{EXT_SUFFIX}.sha256'
+    checksum_file = build_dir / f'{name}{EXT_SUFFIX}.sha256'
     available_checksum = ""
     if checksum_file.exists():
         # read checksum file
         with open(checksum_file, "r") as f:
             available_checksum = f.readline()
 
     # note, the suffix is already a string like '.so'
-    lib_file = DIR / f'{name}{EXT_SUFFIX}'
+    lib_file = build_dir / f'{name}{EXT_SUFFIX}'
 
     if check_duplicate_name and available_checksum != cur_checksum and name in compiled_registry:
-        print(f"\n\nWARNING: module with name '{name}' was already compiled, but with different source code. "
+        raise RuntimeError(f"\n\nModule with name '{name}' was already compiled, but with different source code. "
               "Have you accidentally used the same name for two different CPyExtType, CPyExtHeapType, "
               "or similar helper calls? Modules with same name can sometimes confuse the import machinery "
               "and cause all sorts of trouble.\n")
@@ -138,17 +134,28 @@ def ccompile(self, name, check_duplicate_name=True):
     # Note: It could be that the C source file's checksum didn't change but someone
     # manually deleted the shared library file.
     if available_checksum != cur_checksum or not lib_file.exists():
-        module = Extension(name, sources=[str(source_file)])
-        verbosity = '--verbose' if sys.flags.verbose else '--quiet'
-        args = [verbosity, 'build', 'install_lib', '-f', f'--install-dir={DIR}', 'clean']
-        setup(
-            script_name='setup',
-            script_args=args,
-            name=name,
-            version='1.0',
-            description='',
-            ext_modules=[module]
-        )
+        os.makedirs(build_dir, exist_ok=True)
+        # MSVC linker doesn't like absolute paths in some parameters, so just run from the build dir
+        old_cwd = os.getcwd()
+        os.chdir(build_dir)
+        try:
+            shutil.copy(source_file, '.')
+            module = Extension(name, sources=[source_file.name])
+            args = [
+                '--verbose' if sys.flags.verbose else '--quiet',
+                'build',
+                'install_lib', '-f', '--install-dir=.',
+            ]
+            setup(
+                script_name='setup',
+                script_args=args,
+                name=name,
+                version='1.0',
+                description='',
+                ext_modules=[module]
+            )
+        finally:
+            os.chdir(old_cwd)
 
         # write new checksum
         with open(checksum_file, "w") as f:
@@ -164,6 +171,8 @@ def ccompile(self, name, check_duplicate_name=True):
     if GRAALPYTHON:
         file_not_empty(lib_file)
 
+    return str(build_dir)
+
 
 def file_not_empty(path):
     for i in range(3):
@@ -340,13 +349,13 @@ def file_not_empty(path):
 """
 
 
-class CPyExtFunction():
+class CPyExtFunction:
 
     def __init__(self, pfunc, parameters, template=c_template, cmpfunc=None, stderr_validator=None, **kwargs):
         self.template = template
         self.pfunc = pfunc
         self.parameters = parameters
-        kwargs["name"] = kwargs["name"] if "name" in kwargs else None
+        kwargs.setdefault("name", None)
         self.name = kwargs["name"]
         if "code" in kwargs:
             kwargs["customcode"] = kwargs["code"]
@@ -401,12 +410,28 @@ def _insert(self, d, name, default_value):
     def __repr__(self):
         return "<CPyExtFunction %s>" % self.name
 
-    def test(self):
-        sys.path.insert(0, str(DIR))
+    def __set_name__(self, owner, name):
+        if self.name:
+            raise RuntimeError(f"{type(self)} already assigned to a test suite. Use copy() method to duplicate a test")
+        self.name = name.removeprefix('test_')
+        self.__name__ = name
+        self.__qualname__ = f'{owner.__qualname__}.{name}'
+
+    def copy(self):
+        inst = deepcopy(self)
+        inst.name = None
+        return inst
+
+    @property
+    def __code__(self):
+        return type(self).__call__.__code__
+
+    def __call__(self):
         try:
-            cmodule = __import__(self.name)
-        finally:
-            sys.path.pop(0)
+            self.create_module(self.name)
+            cmodule = compile_module_from_file(self.name)
+        except Exception as e:
+            raise RuntimeError(f"{self.__qualname__}: Failed to create module") from e
         ctest = getattr(cmodule, "test_%s" % self.name)
         cargs = self.parameters()
         pargs = self.parameters()
@@ -416,7 +441,7 @@ def test(self):
                 sys.stderr = StringIO()
             try:
                 cresult = ctest(cargs[i])
-            except BaseException as e:
+            except Exception as e:
                 cresult = e
             else:
                 if self.stderr_validator:
@@ -430,20 +455,13 @@ def test(self):
             except BaseException as e:
                 presult = e
 
-            if not self.cmpfunc:
-                assert cresult == presult, ("%r == %r in %s(%s)" % (cresult, presult, self.name, pargs[i]))
+            if self.cmpfunc:
+                success = self.cmpfunc(cresult, presult)
             else:
-                assert self.cmpfunc(cresult, presult), ("%r == %r in %s(%s)" % (cresult, presult, self.name, pargs[i]))
+                success = cresult == presult
+            assert success, f"{self.__qualname__}: C result not equal to python reference implementation result.\n\tArguments: {pargs[i]!r}\n\tExpected result: {presult!r}\n\tActual C result: {cresult!r}"
         gc.collect()
 
-    def __get__(self, instance, typ=None):
-        if typ is None:
-            return self
-        else:
-            CPyExtFunction.test.__name__ = self.name
-            CPyExtFunction.test.__qualname__ = f'{CPyExtFunction.__name__}.test_{self.name}'
-            return self.test
-
 
 class CPyExtFunctionOutVars(CPyExtFunction):
     '''
@@ -509,27 +527,28 @@ def get_value(self, key, args, kwds):
             return Formatter.get_value(key, args, kwds)
 
 
-def _compile_module(c_source, name):
+def compile_module_from_string(c_source: str, name: str):
     source_file = DIR / f'{name}.c'
     with open(source_file, "wb", buffering=0) as f:
         f.write(bytes(c_source, 'utf-8'))
-    # ensure file was really written
-    try:
-        stat_result = os.stat(source_file)
-        if stat_result[6] == 0:
-            raise SystemError("empty source file %s" % (source_file,))
-    except FileNotFoundError:
-        raise SystemError("source file %s not available" % (source_file,))
-    ccompile(None, name)
-    sys.path.insert(0, str(DIR))
+    return compile_module_from_file(name)
+
+
+def compile_module_from_file(module_name: str):
+    install_dir = ccompile(None, module_name)
+    sys.path.insert(0, install_dir)
     try:
-        cmodule = __import__(name)
+        cmodule = __import__(module_name)
     finally:
         sys.path.pop(0)
     return cmodule
 
 
 def CPyExtType(name, code='', **kwargs):
+    kwargs['set_base_code'] = ''
+    # We set tp_base later, because MSVC doesn't see the usual &PySomething_Type expressions as constants
+    if tp_base := kwargs.get('tp_base'):
+        kwargs['set_base_code'] = f'{name}Type.tp_base = {tp_base};'
     mod_template = """
     static PyModuleDef {name}module = {{
         PyModuleDef_HEAD_INIT,
@@ -546,6 +565,7 @@ def CPyExtType(name, code='', **kwargs):
         if (m == NULL)
             return NULL;
 
+        {set_base_code}
         {ready_code}
         if (PyType_Ready(&{name}Type) < 0)
             return NULL;
@@ -564,7 +584,7 @@ def CPyExtType(name, code='', **kwargs):
     kwargs.setdefault("post_ready_code", "")
     c_source = type_decl + UnseenFormatter().format(mod_template, **kwargs)
 
-    cmodule = _compile_module(c_source, name)
+    cmodule = compile_module_from_string(c_source, name)
     return getattr(cmodule, name)
 
 
@@ -690,7 +710,7 @@ def CPyExtTypeDecl(name, code='', **kwargs):
         {name}_methods,             /* tp_methods */
         {name}_members,             /* tp_members */
         {name}_getset,              /* tp_getset */
-        {tp_base},                  /* tp_base */
+        0, /* set later */          /* tp_base */
         {tp_dict},                  /* tp_dict */
         {tp_descr_get},             /* tp_descr_get */
         {tp_descr_set},             /* tp_descr_set */
@@ -781,7 +801,7 @@ def CPyExtHeapType(name, bases=(object), code='', slots=None, **kwargs):
     kwargs.setdefault("ready_code", "")
     kwargs.setdefault("post_ready_code", "")
     code = UnseenFormatter().format(template, **kwargs)
-    mod = _compile_module(code, name)
+    mod = compile_module_from_string(code, name)
     return mod.create(bases)