Fix C compilation warnings, revert python-build-standalone to 20221106 (next version is broken)

Author: touilleMan

meson.build

@@ -10,7 +10,7 @@ project(
         # always done with a --destdir given installing this project globally on
         # the system is meaningless
         'prefix=/',
-        'c_std=c11',
+        'c_std=c17',
     ]
 )
 

meson_options.txt

@@ -1,3 +1,3 @@
-option('cpython_distrib_version', type: 'string', value: '3.9.7', description: 'Version of CPython to ship')
+option('cpython_distrib_version', type: 'string', value: '3.10.8', description: 'Version of CPython to ship')
 option('godot_headers', type: 'string', value: 'godot_headers', description: 'Path to Godot extension headers directory')
 option('real_is_double', type: 'boolean', value: false, description: 'Built for Godot with 64bits floating point numbers (i.e. double) in builtins instead of 32bits (i.e. float)')

scripts/python_distrib.py

@@ -6,16 +6,17 @@
 import shutil
 import tarfile
 import json
-from zstandard import ZstdDecompressor
+import gzip
+import zstandard
 
 
 PREBUILDS_BASE_URL = "https://github.com/indygreg/python-build-standalone/releases/download"
 PLATFORM_TO_PREBUILDS = {
-    "3.9.7": {
-        "linux-x86_64": f"{PREBUILDS_BASE_URL}/20211017/cpython-3.9.7-x86_64-unknown-linux-gnu-pgo+lto-20211017T1616.tar.zst",
-        "windows-x86": f"{PREBUILDS_BASE_URL}/20211017/cpython-3.9.7-i686-pc-windows-msvc-shared-pgo-20211017T1616.tar.zst",
-        "windows-x86_64": f"{PREBUILDS_BASE_URL}/20211017/cpython-3.9.7-x86_64-pc-windows-msvc-shared-pgo-20211017T1616.tar.zst",
-        "macos-x86_64": f"{PREBUILDS_BASE_URL}/20211017/cpython-3.9.7-x86_64-apple-darwin-pgo+lto-20211017T1616.tar.zst",
+    "3.10.8": {
+        "linux-x86_64": f"{PREBUILDS_BASE_URL}/20221106/cpython-3.10.8+20221106-x86_64-unknown-linux-gnu-pgo+lto-full.tar.zst",
+        "windows-x86": f"{PREBUILDS_BASE_URL}/20221106/cpython-3.10.8+20221106-i686-pc-windows-msvc-shared-pgo-full.tar.zst",
+        "windows-x86_64": f"{PREBUILDS_BASE_URL}/20221106/cpython-3.10.8+20221106-x86_64-pc-windows-msvc-shared-pgo-full.tar.zst",
+        "macos-x86_64": f"{PREBUILDS_BASE_URL}/20221106/cpython-3.10.8+20221106-x86_64-apple-darwin-pgo+lto-full.tar.zst",
     }
 }
 
@@ -39,21 +40,36 @@ def fetch_prebuild(
     if not archive_path.exists() or force:
         print(f"Downloading {archive_url}...")
         tmp_archive_path = archive_path.parent / f"{archive_path.name}.tmp"
-        with urlopen(archive_url) as infd:
+        with urlopen(archive_url) as rep:
             with open(tmp_archive_path, "bw") as outfd:
-                outfd.write(infd.read())
+                length = int(rep.headers.get("Content-Length"))
+                # Poor's man progress bar
+                while True:
+                    if outfd.write(rep.read(2**20)) == 0:
+                        break
+                    print(f"{outfd.tell()//2**20}Mo/{length//2**20}Mo", flush=True, end="\r")
+
         shutil.move(tmp_archive_path, archive_path)
 
     if force and prebuild_dir.exists():
         shutil.rmtree(prebuild_dir)
 
     if not prebuild_dir.exists():
         print(f"Extracting {archive_path}...")
-        with open(archive_path, "rb") as fh:
-            dctx = ZstdDecompressor()
-            with dctx.stream_reader(fh) as reader:
-                with tarfile.open(mode="r|", fileobj=reader) as tf:
-                    tf.extractall(prebuild_dir)
+
+        def _tar_extract(reader):
+            with tarfile.open(mode="r|", fileobj=reader) as tf:
+                tf.extractall(prebuild_dir)
+
+        if archive_path.suffix == ".gz":
+            with gzip.open(archive_path, mode="rb") as reader:
+                _tar_extract(reader)
+        else:
+            assert archive_path.suffix == ".zst"
+            with open(archive_path, mode="rb") as fh:
+                dctx = zstandard.ZstdDecompressor()
+                with dctx.stream_reader(fh) as reader:
+                    _tar_extract(reader)
 
 
 def load_config(prebuild_dir: Path) -> dict:

src/_pythonscript.pyx

@@ -123,7 +123,9 @@ cdef api void _pythonscript_late_init() with gil:
         pythonscript_gdextension.object_method_bind_ptrcall(
             bind,
             singleton,
-            args,
+            # Cast on args is required given autopxd2 incorrectly removes the const
+            # attributes when converting gdextension_interface.c to .pxd
+            <const void * const*>args,
             NULL,
         )
 

src/godot/classes.pyx.j2

@@ -189,7 +189,9 @@ cdef object _object_call(GDExtensionObjectPtr obj, str meth, args):
     pythonscript_gdextension.object_method_bind_call(
         Object_call,
         classdb,
-        <GDExtensionVariantPtr *>&variant_args_ptrs,
+        # Cast is required given autopxd2 incorrectly removes the const attributes
+        # when converting gdextension_interface.c to .pxd
+        <const void * const*>&variant_args_ptrs,
         args_with_meth_len,
         &ret,
         &call_error,

src/godot/hazmat/gdapi.pxd.j2

@@ -30,6 +30,7 @@
 
 cimport cython
 from libc.stdint cimport *
+from libc.string cimport memset
 
 from .gdextension_interface cimport *
 from .gdnative_ptrs cimport *

src/godot/hazmat/gdapi_pxd/meth.pxd.j2

@@ -47,13 +47,20 @@ cdef inline {{ render_utility_function_signature(builtin) }}:
 {%   if not builtin.return_type.is_stack_only %}
     # ptrcall makes us jump right into C++ code (i.e. Godot internals) that expects constructor to be called on each parameter
     __ret = {{ builtin.return_type.c_name_prefix }}_new()
+{%   else %}
+    # C compiler doesn't like passing to a function a pointer on uninitialized
+    # data (see `-Wmaybe-uninitialized`).
+    # Should be optimized out in the end given the called function end up inlined.
+    memset(&__ret, 0, sizeof(__ret))
 {%   endif %}
 {% endif %}
     gd_utility_{{ builtin.original_name }}_ptr(
         {# GDExtensionTypePtr ret #}
         {{ "NULL" if builtin.return_type.is_nil else "&__ret" }},
         {# const GDExtensionTypePtr *p_arguments #}
-        {{ "NULL" if (builtin.arguments | length) == 0 else "__args" }},
+        # Cast on args is required given autopxd2 incorrectly removes the const
+        # attributes when converting gdextension_interface.c to .pxd
+        <const void *const*>{{ "NULL" if (builtin.arguments | length) == 0 else "__args" }},
         {# int p_argument_count #}
         {{ builtin.arguments | length }}
     )
@@ -70,12 +77,16 @@ cdef inline {{ render_utility_function_signature(builtin) }}:
 {# to pass a `gd_variant_t**` as parameter) whose size is passed in the number #}
 {# of parameters fields... Yes this is very confusing stuff, my brain hurts ! #}
 {% if builtin.return_type.is_nil %}
-    gd_utility_{{ builtin.original_name }}_ptr(NULL, <GDExtensionTypePtr *>&args, args_count)
+    # Cast on args is required given autopxd2 incorrectly removes the const
+    # attributes when converting gdextension_interface.c to .pxd
+    gd_utility_{{ builtin.original_name }}_ptr(NULL, <const void *const*>&args, args_count)
 {% else %}
     cdef gd_variant_t __ret  # TODO: improve return type !
     # ptrcall makes us jump right into C++ code (i.e. Godot internals) that expects constructor to be called on each parameter
     __ret = gd_variant_new()
-    gd_utility_{{ builtin.original_name }}_ptr(&__ret, <GDExtensionTypePtr *>&args, args_count)
+    # Cast on args is required given autopxd2 incorrectly removes the const
+    # attributes when converting gdextension_interface.c to .pxd
+    gd_utility_{{ builtin.original_name }}_ptr(&__ret, <const void *const*>&args, args_count)
     return __ret
 {% endif %}
 {% endmacro %}
@@ -151,7 +162,9 @@ cdef inline {{ render_builtin_constructor_signature(builtin, constructor) }}:
 {% endif %}
     {{ builtin.c_name_prefix }}_constructor_{{ constructor.index }}_ptr(
         &obj,
-        {{ "NULL" if (constructor.arguments | length) == 0 else "p_args" }}
+        # Cast is required given autopxd2 incorrectly removes the const attributes
+        # when converting gdextension_interface.c to .pxd
+        <const void * const*>{{ "NULL" if (constructor.arguments | length) == 0 else "p_args" }}
     )
     return obj
 {% endmacro %}
@@ -195,13 +208,20 @@ cdef inline {{ render_builtin_method_signature(builtin, meth) }}:
 {%   if not meth.return_type.is_stack_only %}
     # ptrcall makes us jump right into C++ code (i.e. Godot internals) that expects constructor to be called on each parameter
     __ret = {{ meth.return_type.c_name_prefix }}_new()
+{%   else %}
+    # C compiler doesn't like passing to a function a pointer on uninitialized
+    # data (see `-Wmaybe-uninitialized`).
+    # Should be optimized out in the end given the called function end up inlined.
+    memset(&__ret, 0, sizeof(__ret))
 {%   endif %}
 {% endif %}
     {{ builtin.c_name_prefix }}_meth_{{ meth.name }}_ptr(
         {# GDExtensionTypePtr p_base #}
         self,
         {# const GDExtensionTypePtr *__args #}
-        __args,
+        # Cast is required given autopxd2 incorrectly removes the const attributes
+        # when converting gdextension_interface.c to .pxd
+        <const void * const*>__args,
         {# GDExtensionTypePtr __ret #}
         {{ "NULL" if meth.return_type.is_nil else "&__ret" }},
         {# int p_argument_count #}
@@ -224,6 +244,11 @@ cdef inline {{ m.type.c_type }} {{ builtin.c_name_prefix }}_get_{{ m.name }}({{
 {% if not m.type.is_stack_only %}1
     # ptrcall makes us jump right into C++ code (i.e. Godot internals) that expects constructor to be called on each parameter
     __ret = {{ m.type.c_name_prefix }}_new()
+{% else %}
+    # C compiler doesn't like passing to a function a pointer on uninitialized
+    # data (see `-Wmaybe-uninitialized`).
+    # Should be optimized out in the end given the called function end up inlined.
+    memset(&__ret, 0, sizeof(__ret))
 {% endif %}
     {{ builtin.c_name_prefix }}_get_{{ m.name }}_ptr(self, &__ret)
     return __ret
@@ -250,6 +275,11 @@ cdef inline {{ builtin.indexing_return_type.c_type }} {{ builtin.c_name_prefix }
 {%   if not builtin.indexing_return_type.is_stack_only %}
     # ptrcall makes us jump right into C++ code (i.e. Godot internals) that expects constructor to be called on each parameter
     __ret = {{ builtin.indexing_return_type.c_name_prefix }}_new()
+{%   else %}
+    # C compiler doesn't like passing to a function a pointer on uninitialized
+    # data (see `-Wmaybe-uninitialized`).
+    # Should be optimized out in the end given the called function end up inlined.
+    memset(&__ret, 0, sizeof(__ret))
 {%   endif %}
     {{ builtin.c_name_prefix }}_indexed_getter_ptr(self, index, &__ret)
     return __ret
@@ -277,6 +307,11 @@ cdef inline {{ builtin.indexing_return_type.c_type }} {{ builtin.c_name_prefix }
 {%   if not builtin.indexing_return_type.is_stack_only %}
     # ptrcall makes us jump right into C++ code (i.e. Godot internals) that expects constructor to be called on each parameter
     __ret = {{ builtin.indexing_return_type.c_name_prefix }}_new()
+{%   else %}
+    # C compiler doesn't like passing to a function a pointer on uninitialized
+    # data (see `-Wmaybe-uninitialized`).
+    # Should be optimized out in the end given the called function end up inlined.
+    memset(&__ret, 0, sizeof(__ret))
 {%   endif %}
     {{ builtin.c_name_prefix }}_keyed_getter_ptr(self, key, &__ret)
     return __ret

src/pythonscript.c

@@ -77,10 +77,12 @@ DLL_EXPORT void pythonscript_gdstringname_new(GDExtensionStringNamePtr ptr, cons
     // Method name must be passed as a StringName object... which itself has
     // to be built from a String object :(
     GDExtensionStringPtr as_gdstring;
+    const GDExtensionConstTypePtr args[1] = {&as_gdstring};
     pythonscript_gdextension->string_new_with_utf8_chars(&as_gdstring, cstr);
-    gdstringname_constructor(ptr, &as_gdstring);
+    gdstringname_constructor(ptr, args);
     gdstring_destructor(&as_gdstring);
 }
+
 DLL_EXPORT void pythonscript_gdstringname_delete(GDExtensionStringNamePtr ptr) {
     gdstringname_destructor(ptr);
 }

tests/1-cython-access-gdextension/build.py

@@ -2,31 +2,64 @@
 from pathlib import Path
 import subprocess
 import shutil
-
+import zipfile
+import tempfile
 
 PROJECT_DIR = Path(__file__).resolve().parent
 
+
 if platform.system() == "Windows":
     python_path = PROJECT_DIR / "addons/pythonscript/windows-x86_64/python.exe"
     lib_pattern = "my.*.pyd"
 elif platform.system() == "Darwin":
-    python_path = PROJECT_DIR / "addons/pythonscript/macos-x86_64/bin/python"
+    python_path = PROJECT_DIR / "addons/pythonscript/macos-x86_64/bin/python3"
     lib_pattern = "my.*.dylib"
 else:
     assert platform.system() == "Linux"
-    python_path = PROJECT_DIR / "addons/pythonscript/linux-x86_64/bin/python"
+    python_path = PROJECT_DIR / "addons/pythonscript/linux-x86_64/bin/python3"
     lib_pattern = "my.*.so"
 
+
 cmd = [str(python_path), "setup.py", "build_ext", "--build-lib", str(PROJECT_DIR)]
 print(" ".join(cmd))
 subprocess.check_call(cmd, cwd=PROJECT_DIR)
 
 # Finally remove the platform info from the shared library, this is to avoid
-# annoying update everytime we change CPython embedded version
-
+# annoying update everytime we change CPython embedded version.
 lib_candidates = list(PROJECT_DIR.glob(lib_pattern))
 assert len(lib_candidates) == 1, lib_candidates
 lib = lib_candidates[0]
 lib_new_name = f"my{lib.suffix}"
 print(f"rename {lib.name} -> {lib_new_name}")
 shutil.move(lib, lib.parent / lib_new_name)
+
+
+# with tempfile.TemporaryDirectory() as wheeldir:
+
+#     # 1) Build the project into a wheel
+#     # This is needed to have a build that takes the `pyproject.toml` into account
+#     # and install the correct build depedencies beforehand.
+#     cmd = [str(python_path), "-m", "pip", "wheel", "--wheel-dir", wheeldir, "."]
+#     print(" ".join(cmd))
+#     subprocess.check_call(cmd, cwd=PROJECT_DIR)
+
+#     outputs = list(Path(wheeldir).iterdir())
+#     if len(outputs) != 1:
+#         raise RuntimeError(f"Target dir unexpectedly contains multiple or no files: {outputs}")
+#     wheel_path = outputs[0]
+
+#     # 2) Extract the .so from the wheel (only thing we care about).
+#     with zipfile.ZipFile(wheel_path) as wheel:
+#         for info in wheel.infolist():
+#             info.filename.rsplit("/", 1)[-1].startswith("my.")
+#             wheel.extract(info.filename, path=PROJECT_DIR)
+#             break
+
+# # 3) Finally remove the platform info from the shared library, this is to avoid
+# # annoying update everytime we change CPython embedded version.
+# lib_candidates = list(PROJECT_DIR.glob(lib_pattern))
+# assert len(lib_candidates) == 1, lib_candidates
+# lib = lib_candidates[0]
+# lib_new_name = f"my{lib.suffix}"
+# print(f"rename {lib.name} -> {lib_new_name}")
+# shutil.move(lib, lib.parent / lib_new_name)

tests/1-cython-access-gdextension/my.pyx

@@ -19,8 +19,8 @@ cdef extern GDExtensionBool _my_init(
     # print("==> _my_init")
     r_initialization.minimum_initialization_level  = GDEXTENSION_INITIALIZATION_SERVERS
     r_initialization.userdata = NULL
-    r_initialization.initialize = _my_initialize
-    r_initialization.deinitialize = _my_deinitialize
+    #  r_initialization.initialize = _my_initialize
+    #  r_initialization.deinitialize = _my_deinitialize
     return True