Add support for Python's "Limited API"; Test cibuildwheel and py >=3.5 using TestPythonSML.py

.github/workflows/build.yml

@@ -272,7 +272,8 @@ jobs:
       fail-fast: true
       matrix:
         os: [
-            ubuntu-latest,
+            # 20.04 to preserve compatibility with testing stage
+            ubuntu-20.04,
             # latest available X86_64 target
             macos-12,
             # latest is ARM
@@ -311,16 +312,117 @@ jobs:
         if: matrix.os == 'macos-latest'
         run: brew install swig
 
+      - name: Setup SWIG (windows-latest)
+        if: matrix.os == 'windows-latest'
+        uses: MinoruSekine/setup-scoop@v4
+        with:
+          apps: swig
+
       - name: Build wheels
         uses: pypa/[email protected]
         with:
           package-dir: Core/ClientSMLSWIG/Python/
 
+      - name: Ensure unit tests built (ubuntu-20.04)
+        if: matrix.os == 'ubuntu-20.04'
+        # On Ubuntu, we run cibuildwheel inside docker containers, which don't retain the outputs.
+        # However, we want out/SoarUnitTests/ to be availible for future tests, so we build it again here.
+        run: python scons/scons.py tests
+
+        # Save out/SoarUnitTests/
+      - uses: actions/cache/save@v3
+        id: cache
+        with:
+          path: out/SoarUnitTests/
+          key: ${{ runner.os }}-${{ github.sha }}
+
+      - name: ABI3Audit (*nix)
+        if: matrix.os != 'windows-latest'
+        run: pipx run abi3audit -S -v wheelhouse/*
+
+      - name: ABI3Audit (Windows)
+        if: matrix.os == 'windows-latest'
+        # For windows we can't use glob expansion, so we need to enumerate the files manually.
+        # https://stackoverflow.com/a/16804630/8700553
+        run: Get-ChildItem -File wheelhouse | Foreach {pipx run abi3audit -S -v $_.fullname}
+
       - uses: actions/upload-artifact@v4
         with:
           name: cibw-wheels-${{ matrix.os }}-${{ strategy.job-index }}
           path: ./wheelhouse/*.whl
 
+  test_python:
+    name: Test wheel on all python versions
+    runs-on: ${{ matrix.os }}
+    strategy:
+      fail-fast: false
+      matrix:
+        os:
+          # linux target which supports all python versions we want to install for
+          - ubuntu-20.04
+            # latest available X86_64 target
+          - macos-12
+            # latest is ARM
+          - macos-latest
+        python-minor:
+          - 5
+          - 6
+          - 7
+          - 8
+          - 9
+          - 10
+          - 11
+          - 12
+
+        # for macos-latest, we can't use python 3.5 - 3.7
+        exclude:
+          - os: macos-latest
+            python-minor: 5
+          - os: macos-latest
+            python-minor: 6
+          - os: macos-latest
+            python-minor: 7
+
+    needs:
+      # We depend on the python wheel builds because we need their artifacts
+      - python_wheels
+
+      # We depend on the builds themselves to gatekeep the upload if build + testing fails,
+      # usually then also the python wheel build should fail, but the normal builds do more thorough checking.
+      - Posix
+      - Windows
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+
+      - uses: actions/download-artifact@v4
+        with:
+          pattern: cibw-wheels-*
+          path: wheelhouse/
+          merge-multiple: true
+
+      - name: Setup Base Python
+        uses: actions/setup-python@v4
+        with:
+          python-version: 3.${{ matrix.python-minor }}
+        env:
+          # Older python versions fail install if we dont do this: https://github.com/actions/setup-python/issues/866
+          PIP_TRUSTED_HOST: "pypi.python.org pypi.org files.pythonhosted.org"
+
+        # Restore out/SoarUnitTests/
+      - uses: actions/cache/restore@v3
+        id: cache
+        with:
+          path: out/SoarUnitTests/
+          key: ${{ runner.os }}-${{ github.sha }}
+          fail-on-cache-miss: true
+
+      - name: Run python test command
+        # TODO: replace with actual test command: https://github.com/SoarGroup/Soar/issues/460
+        run: |
+          pip3.${{ matrix.python-minor }} install soar-sml -f wheelhouse --no-index
+          python3.${{ matrix.python-minor }} Core/ClientSMLSWIG/Python/TestPythonSML.py
+
   python_push_dev:
     name: Publish to test.pypi.org
     runs-on: ubuntu-latest
@@ -329,14 +431,8 @@ jobs:
     if: github.event_name == 'schedule' || github.event_name == 'workflow_dispatch'
     # Alternative, to upload on every commit to development;
     # if: github.event_name != 'schedule' && github.event_name != 'release'
-    needs:
-      # We depend on the python wheel builds because we need their artifacts
-      - python_wheels
 
-      # We depend on the builds themselves to gatekeep the upload if build + testing fails,
-      # usually then also the python wheel build should fail, but the normal builds do more thorough checking.
-      - Posix
-      - Windows
+    needs: test_python
 
     # We use Trusted Publishing to manage our access to pypi:
     # https://github.com/marketplace/actions/pypi-publish#trusted-publishing
@@ -370,14 +466,7 @@ jobs:
     name: Publish to pypi.org
     runs-on: ubuntu-latest
     if: github.event_name == 'release'
-    needs:
-      # We depend on the python wheel builds because we need their artifacts
-      - python_wheels
-
-      # We depend on the builds themselves to gatekeep the upload if build + testing fails,
-      # usually then also the python wheel build should fail, but the normal builds do more thorough checking.
-      - Posix
-      - Windows
+    needs: test_python
 
     # We use Trusted Publishing to manage our access to pypi:
     # https://github.com/marketplace/actions/pypi-publish#trusted-publishing

Core/ClientSMLSWIG/Python/Python_sml_ClientInterface.i

@@ -252,7 +252,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 +306,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. */

Core/ClientSMLSWIG/Python/SConscript

@@ -59,6 +59,35 @@ install_source = env.Install(lib_install_dir, source)
 install_lib = env.Install(lib_install_dir, shlib)
 install_test = env.Install(lib_install_dir, env.File('TestPythonSML.py'))
 
+if "SOAR_PYTHON_ABI3" in env["ENV"]:
+    if env.get('PY_ABI3_VERSION', None) is None:
+        print("SOAR_PYTHON_ABI3 requested, but PY_ABI3_VERSION was not defined in env, aborting.", file=sys.stderr)
+        Exit(1)
+    elif not isinstance(env["PY_ABI3_VERSION"], tuple) or len(env["PY_ABI3_VERSION"]) != 2:
+        print("PY_ABI3_VERSION malformed, expected 2-tuple.", file=sys.stderr)
+        Exit(1)
+    elif env["PY_ABI3_VERSION"] < (3, 2):
+        print("PY_ABI3_VERSION below supported version for ABI3", file=sys.stderr)
+        Exit(1)
+
+    # Only SWIG 4.2.0 added Python Limited API support:
+    # https://github.com/swig/swig/issues/1009
+    if env["SWIG_VERSION"] >= (4, 2, 0):
+        print("SOAR_PYTHON_ABI3 defined, building Python Limited API...")
+        # Signal to enscons that we're building the limited API
+        # also deconstruct (major, minor) tuple
+        major, minor = env["LIMITED_API_TARGET"] = env["PY_ABI3_VERSION"]
+
+        # CPP Definition constructed from the minor/major ABI3 version parts
+        api_version = (0x1000000 * major) + (0x10000 * minor)
+
+        # Signal to the .i file that we're building the limited API
+        clone.Append(CPPDEFINES = { 'Py_LIMITED_API': api_version })
+    else:
+        print("'Requested building Python SML bindings with Python's Limited API, "
+              "while SWIG does not support it, aborting.'", file=sys.stderr)
+        Exit(1)
+
 # We add soarlib to the python_sml explicitly, as some operating systems don't pick up on this dependency,
 # and crash the build.
 Import('soarlib')

Core/ClientSMLSWIG/Python/TestPythonSML.py

@@ -5,32 +5,40 @@
 #  several kinds of callbacks, reinitializing, agent destruction, and kernel
 #  destruction (and maybe some other things, too).
 #
-from dataclasses import dataclass
+# This file needs to be compatible with python 3.5, to run on CI jobs testing the lowest supported python version.
 from pathlib import Path
 import sys
+import os
 import time
 
-import Python_sml_ClientInterface
-from Python_sml_ClientInterface import *
+try:
+    import Python_sml_ClientInterface
+    from Python_sml_ClientInterface import *
 
-AGENT_DIR = Path(Python_sml_ClientInterface.__file__).parent / 'SoarUnitTests'
+    BASE_DIR = Path(Python_sml_ClientInterface.__file__).parent
+except ImportError:
+    from soar_sml import *
+
+    BASE_DIR = Path(os.environ.get("SOAR_UNIT_TEST_BASE_DIR", os.path.abspath("out/")))
+
+
+AGENT_DIR = BASE_DIR / 'SoarUnitTests'
 
-@dataclass
 class CalledSignal:
-    called: bool = False
+    called = False
 
 # towers_of_hanoi_file = AGENT_DIR / 'test-towers-of-hanoi-SML.soar'
 towers_of_hanoi_file = AGENT_DIR / 'Chunking' / 'tests' / 'towers-of-hanoi-recursive' / 'towers-of-hanoi-recursive' / 'towers-of-hanoi-recursive_source.soar'
 toh_test_file = AGENT_DIR / 'TOHtest.soar'
 for source_file in (towers_of_hanoi_file, toh_test_file):
     if not source_file.is_file():
-        raise FileNotFoundError(f"Source file doesn't exist: {source_file}")
+        raise FileNotFoundError("Source file doesn't exist: %s" % source_file)
 
 def PrintCallback(id, userData, agent, message):
     print(message)
 
 def ProductionExcisedCallback(id, signal: CalledSignal, agent, prodName, instantiation):
-    print(f"removing {prodName} ({instantiation})")
+    print("removing %s (%s)" % (prodName, instantiation))
     signal.called = True
 
 def ProductionFiredCallback(id, signal: CalledSignal, agent, prodName, instantiation):
@@ -72,7 +80,7 @@ def UpdateEventCallback(id, signal: CalledSignal, kernel, runFlags):
 
 def UserMessageCallback(id, tester, agent, clientName, message):
     print("Agent", agent.GetAgentName(), "received usermessage event for clientName '", clientName, "' with message '", message, "'")
-    assert tester(clientName, message), f"❌ UserMessageCallback called with unexpected clientName '{clientName}' or message '{message}'"
+    assert tester(clientName, message), ("❌ UserMessageCallback called with unexpected clientName '%s' or message '%s'" % (clientName, message))
     return ""
 
 kernel = Kernel.CreateKernelInNewThread()
@@ -148,7 +156,7 @@ def test_excise(kernel):
     assert not prod_removed_handler_signal.called, "❌ Production excise handler called before excise"
     result = kernel.ExecuteCommandLine("excise towers-of-hanoi*monitor*operator-execution*move-disk", "Soar1")
     assert prod_removed_handler_signal.called, "❌ Production excise handler not called"
-    print(f"✅ Production excise: {result}")
+    print("✅ Production excise: %s" % result)
 
 test_excise(kernel)
 
@@ -161,7 +169,7 @@ def test_excise(kernel):
 def check_rhs_handler_called(kernel):
     s1 = kernel.ExecuteCommandLine("print s1", "Soar1")
     if s1.find("^rhstest success") == -1:
-        print(f"\n❌RHS test FAILED; s1 is {s1}", file=sys.stderr)
+        print("\n❌RHS test FAILED; s1 is %s" % s1, file=sys.stderr)
         sys.exit(1)
     else:
         print("\n✅RHS test PASSED")

Core/ClientSMLSWIG/Python/pyproject.toml

@@ -50,7 +50,7 @@ authors = [
     { name = "Y. Wang" },
 ]
 readme = "README.md"
-requires-python = ">=3.8"
+requires-python = ">=3.5"
 dependencies = []
 keywords = ["soar", "sml", "cog-arch", "cognitive architecture", "cognitive", "soar-sml", "ai"]
 classifiers = [
@@ -70,7 +70,7 @@ requires = [
     # and other python build/install frontends (such as pip itself).
     #
     # We use a forked version of enscons to add python 3.12 support.
-    "enscons @ git+https://github.com/ShadowJonathan/enscons-soar@544f39f",
+    "enscons @ git+https://github.com/ShadowJonathan/enscons-soar@190091866ac35fb5d390c425bafaf13443baee5e",
 
     # Required sub-dependencies of enscons.
     "toml>=0.1",
@@ -123,24 +123,28 @@ skip = [
     # which would be a duplicate with the x86 runner.
     "cp38-macosx_arm64"
 ]
-# This test command:
-#  - imports soar_sml
-#  - creates the kernel
-#  - creates an agent
-#  - executes a command on that agent
-#  - fails if the result of this command isn't as expected
-# to test if Soar properly boots and runs.
-test-command = """\
-python -c "import soar_sml;\
-k=soar_sml.Kernel.CreateKernelInNewThread();\
-a=k.CreateAgent('soar');\
-assert(a.ExecuteCommandLine('echo hello world').strip()=='hello world')"
-"""
+# We only need one target to build on, since we test all versions later.
+#
+# We pick python 3.9, since macOS ARM64 doesn't have 3.8, and so this is the earliest version on all platforms.
+build = "cp39*"
+# Inside CIs, we want to build the limited API, as we control SWIG's version there, so we opt-in.
+environment = { SOAR_PYTHON_ABI3="1" }
+# Create the unit tests under out/SoarUnitTests/
+before-test = "python scons/scons.py tests"
+# This test command will ruin the unit tests to make sure the build succeeded without oddities.
+test-command = "SOAR_UNIT_TEST_BASE_DIR={project}/out/ python {project}/Core/ClientSMLSWIG/Python/TestPythonSML.py"
 
 # Skip testing python 3.8 on ARM64. This is due to a limitation with cibuildwheel:
 # https://github.com/pypa/cibuildwheel/pull/1169#issuecomment-1178727618
 test-skip = "cp38-macosx_*:arm64"
 
+[tool.cibuildwheel.windows]
+# For windows we need a slightly different command
+test-command = """\
+set SOAR_UNIT_TEST_BASE_DIR={project}/out/
+python {project}/Core/ClientSMLSWIG/Python/TestPythonSML.py
+"""
+
 [tool.cibuildwheel.linux]
 # Specific instructions for cibuildwheel when running on linux:
 #  - add the /project/out directory for the linker to look at.

Core/SConscript

@@ -16,21 +16,28 @@ def recursive_glob(treeroot, pattern):
 	return results
 # 	('SoarKernel/SoarKernel.cxx',       recursive_glob(kernel_env.Dir('SoarKernel/src').abspath(), '*.cpp')),    # Kernel
 
-def CheckSWIG(env):
+def SWIGVersion(env):
 	if not env.WhereIs('swig'):
-		return False
-
+		return None
 	p = sub.Popen('swig -version'.split(), stdout=sub.PIPE)
 	out = p.communicate()[0].decode().split()
 	p.wait()
 
-	version = tuple(int(x) for x in out[2].split('.'))
+	return tuple(int(x) for x in out[2].split('.'))
+
+
+def CheckSWIG(env):
+	version = SWIGVersion(env)
+	if version is None:
+		return False
+
 	if version >= (1, 3, 31):
 		return True
 
 	print('swig version 1.3.31 or higher is required')
 	return False
 
+
 kernel_env = env.Clone()
 
 # Windows DLLs need to get linked to dependencies, whereas Linux and Mac shared objects do not
@@ -110,6 +117,7 @@ env.Alias('headers', headers)
 if not CheckSWIG(env):
 	print('SWIG not found. Will not define sml_* build targets. Install swig to build wrappers for other languages.')
 else:
+	env["SWIG_VERSION"] = SWIGVersion(env)
 	for x in 'Python Java Tcl CSharp'.split():
 		SConscript(os.path.join('ClientSMLSWIG', x, 'SConscript'))