diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 3539e014bc..2d97a1618f 100644 --- a/.github/workflows/build.yml +++ b/.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/cibuildwheel@v2.17.0 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 diff --git a/Core/ClientSMLSWIG/Python/Python_sml_ClientInterface.i b/Core/ClientSMLSWIG/Python/Python_sml_ClientInterface.i index b6b7143308..38e76fd9e3 100644 --- a/Core/ClientSMLSWIG/Python/Python_sml_ClientInterface.i +++ b/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. */ diff --git a/Core/ClientSMLSWIG/Python/SConscript b/Core/ClientSMLSWIG/Python/SConscript index 7ce28417a9..b599fed93a 100644 --- a/Core/ClientSMLSWIG/Python/SConscript +++ b/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') diff --git a/Core/ClientSMLSWIG/Python/TestPythonSML.py b/Core/ClientSMLSWIG/Python/TestPythonSML.py index a2d54da696..239dd1cf71 100755 --- a/Core/ClientSMLSWIG/Python/TestPythonSML.py +++ b/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") diff --git a/Core/ClientSMLSWIG/Python/pyproject.toml b/Core/ClientSMLSWIG/Python/pyproject.toml index e146b55a19..aed8cd2cc7 100644 --- a/Core/ClientSMLSWIG/Python/pyproject.toml +++ b/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. diff --git a/Core/SConscript b/Core/SConscript index 04eabaca4c..04210628a9 100644 --- a/Core/SConscript +++ b/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')) diff --git a/SConstruct b/SConstruct index f59e340eff..72977f35af 100644 --- a/SConstruct +++ b/SConstruct @@ -172,6 +172,7 @@ env = Environment( SOAR_VERSION=SOAR_VERSION, VISHIDDEN=False, # needed by swig JAVAVERSION='11.0', + PY_ABI3_VERSION = (3, 5), SML_CSHARP_ALIAS = SML_CSHARP_ALIAS, SML_JAVA_ALIAS = SML_JAVA_ALIAS, SML_PYTHON_ALIAS = SML_PYTHON_ALIAS, @@ -417,7 +418,6 @@ py_sources += [ env.Alias(SML_PYTHON_ALIAS + "_dev", py_sources) if enscons_active: - env['PACKAGE_METADATA'] = enscons.get_pyproject(env)['project'] # Instead of giving an explicit tag, we tell enscons that we're not building a "pure" (python-only) library, # and so we let it determine the wheel tag by itself. env['ROOT_IS_PURELIB'] = False