Have SWIG use the Python limited API

[ShadowJonathan]

This is a consideration; currently with #448 we are building for every single Python version, as this is required for Python compatibility.

However, Python has a "Limited API" that is forward compatible for all versions starting from the version that an extension (packaged in a wheel) is built for.

SWIG appears to support this: https://github.com/swig/swig/issues/1009

This would;

• decrease the amount of wheels needed to be built
• consequently; decrease CI build times
• make soar-sml binary releases forward-compatible with all Python versions that come after, even ones unknown at the time of release

[ShadowJonathan]

Already hitting a snag trying to build it without modication; the PyUnicode_AsUTF8 macro/function is not part of the stable ABI.

The nearest nice-looking API (though: I did not look very far) is PyUnicode_AsUTF8AndSize, which was only part of the stable ABI since 3.10. Nevermind, got this fixed in another comment.

(The function is used here, for example: https://github.com/SoarGroup/Soar/blame/241d05ba6506f490caedaeb82c44ffc67b1c2a7c/Core/ClientSMLSWIG/Python/Python_sml_ClientInterface.i#L255)

---

I got this to build this way by simply adding the following lines after the first #include statement in the .i file:

%begin %{
#define Py_LIMITED_API 0x03040000
%}

(This sets the targeted ABI version to Python 3.4)

---

• [x] Currently, enscons-soar does not properly recognise and auto-tag abi-built wheels, instead tagging them with platform-specific parts, I'll have to figure that out first.

[ShadowJonathan]

Hm, nevermind, all I needed to do is this, before it got SWIG+scons to successfully build it;

diff --git a/Core/ClientSMLSWIG/Python/Python_sml_ClientInterface.i b/Core/ClientSMLSWIG/Python/Python_sml_ClientInterface.i
index b6b714330..a2a7986fb 100644
--- a/Core/ClientSMLSWIG/Python/Python_sml_ClientInterface.i
+++ b/Core/ClientSMLSWIG/Python/Python_sml_ClientInterface.i
@@ -11,6 +11,10 @@
 // handle windows calling convention, __declspec(dllimport), correctly
 %include <windows.i>

+%begin %{
+#define Py_LIMITED_API 0x03040000
+%}
+
 %{
        // helps quell warnings
        #ifndef unused
@@ -252,7 +256,10 @@
                        return "";
                }

-               std::string res = PyUnicode_AsUTF8 (result);
+               PyObject* unicode = PyUnicode_AsUTF8String (result);
+               std::string res = PyBytes_AsString(unicode);
+
+               Py_DECREF(unicode);
                Py_DECREF(result);

                PyGILState_Release(gstate); /* Release the thread. No Python API allowed beyond this point. */
@@ -303,7 +310,10 @@
                        return "";
                }

-               std::string res = PyUnicode_AsUTF8 (result);
+               PyObject* unicode = PyUnicode_AsUTF8String (result);
+               std::string res = PyBytes_AsString(unicode);
+
+               Py_DECREF(unicode);
                Py_DECREF(result);

                PyGILState_Release(gstate); /* Release the thread. No Python API allowed beyond this point. */

Now, whether the fact that this is buildable means that it will work properly, is another task, but at least I'm happy that this part seems to have so little changes required.

[ShadowJonathan]

For if we decide on doing this: I found abi3audit, a tool which can help catch ABI incompatibilities, to be added as a post-build step.

[garfieldnate]

Basically a brand-new feature in SWIG! I've been worried about the Python version incompatibilities for a while, and this would solve it really nicely! I'm very much in favor of this change. I think your workaround for the particular ABI incompatibility looks totally fine, too.

The bundled SWIG for Windows does appear to be too old, so we would need to manually install it in CI: https://github.com/actions/runner-images/blob/main/images/windows/Windows2022-Readme.md#tools. For Mac we are already installing it manually.

We actually run Python SML tests in our CI already (see SML Python tests section in build.yml), so if you feel like pushing the change to a branch and seeing if the tests pass, you'd get some feedback on how well it's working already.

[ShadowJonathan]

I've determined the minimum Limited API version we can support is 3.4, since it uses PyType_GetSlot

(Limited API became available with python 3.2, so there was a possibility to support even older Python versions)

[ShadowJonathan]

With the lowest support being Python 3.4, we could technically distribute that, and support older python versions.

However, I'd like to tag it with a version we support can test for; If we tag it with python 3.4, then I'd like to have a CI step where it installs python 3.4, and runs the tests, and fails the pipeline (and not upload the artifacts) if it fails.

The setup-python action says it will download from Python's GitHub releases, which only go back to Python 3.5.

Nevermind: The version manifest here shows that it does support installing python 3.4, though only for linux and windows. Mac needs 3.5.

(This does not include ARM64 Mac, only Intel Mac, but since the lowest support for that is python 3.9, it is then already tested, though we should tag that one with cp39-abi3-macosx_11_0_arm64)

[garfieldnate]

I think 3.5 is old enough; it's close to a decade old. Java is an important component for Soar users and we require 11, which is 3 years newer than Python 3.5.

[garfieldnate]

Note to self: should also test locally with an older SWIG version, skipping the Python build, just to make sure that, if you don't care about Python, the changes don't require you to update your SWIG install.

[ShadowJonathan]

I see you added this to the .3 milestone; let me make a PR for this, with the required changes to enscons-soar.

[garfieldnate]

Adding that milestone wasn't meant as a signal to tell you to do it, but let's goooooo! I'm very happy to have you do it! This is enough of a pain point right now that a solution couldn't be ignored for the next release.

I assume this checkbox in the comment above can be checked now?

Currently, enscons-soar does not properly recognise and auto-tag abi-built...

[ShadowJonathan]

Yes, checked :)