Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 27 additions & 30 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,23 +12,19 @@ on:

jobs:
build:
runs-on: ${{ matrix.os }}
runs-on: ${{ matrix.target.os }}
strategy:
fail-fast: false
matrix:
include:
- platform: linux
arch: x86_64
os: ubuntu-latest
- platform: windows
arch: x86_64
os: windows-latest
- platform: macos
arch: x86_64
os: macos-latest
- platform: macos
arch: arm64
os: macos-latest
python_standalone_build: [20231002]
python_version: [3.12.0]
target:
[
{ platform: linux, arch: x86_64, os: ubuntu-latest },
{ platform: windows, arch: x86_64, os: windows-latest },
{ platform: macos, arch: x86_64, os: macos-latest },
{ platform: macos, arch: arm64, os: macos-latest },
]

steps:
- name: Checkout repository
Expand All @@ -39,7 +35,8 @@ jobs:
- name: Setup Python
uses: actions/setup-python@v5
with:
python-version: '3.12'
# Note that this is for scons, not for the python version we are building.
python-version: 3.12

- name: Setup SCons
shell: bash
Expand All @@ -49,56 +46,56 @@ jobs:
scons --version

- name: Build extension (Linux)
if: matrix.os == 'ubuntu-latest'
if: matrix.target.os == 'ubuntu-latest'
run: |
scons platform=${{ matrix.platform }} arch=${{ matrix.arch }} single_source=true
scons platform=${{ matrix.target.platform }} arch=${{ matrix.target.arch }} python_standalone_build=${{ matrix.python_standalone_build }} python_version=${{ matrix.python_version }} single_source=true

- name: Build extension (Windows)
if: matrix.os == 'windows-latest'
if: matrix.target.os == 'windows-latest'
shell: pwsh
run: |
scons platform=${{ matrix.platform }} arch=${{ matrix.arch }} single_source=true
scons platform=${{ matrix.target.platform }} arch=${{ matrix.target.arch }} python_standalone_build=${{ matrix.python_standalone_build }} python_version=${{ matrix.python_version }} single_source=true

- name: Build extension (macOS)
if: matrix.os == 'macos-latest'
if: matrix.target.os == 'macos-latest'
run: |
scons platform=${{ matrix.platform }} arch=${{ matrix.arch }} single_source=true
scons platform=${{ matrix.target.platform }} arch=${{ matrix.target.arch }} python_standalone_build=${{ matrix.python_standalone_build }} python_version=${{ matrix.python_version }} single_source=true

- name: Upload artifacts (Linux)
if: matrix.os == 'ubuntu-latest'
if: matrix.target.os == 'ubuntu-latest'
uses: actions/upload-artifact@v4
with:
name: godot-python-${{ matrix.platform }}-${{ matrix.arch }}
name: godot-python-${{ matrix.target.platform }}-${{ matrix.target.arch }}-py${{ matrix.python_version }}-ig${{ matrix.python_standalone_build }}
path: bin/**/*
retention-days: 30

- name: Upload artifacts (Windows)
if: matrix.os == 'windows-latest'
if: matrix.target.os == 'windows-latest'
uses: actions/upload-artifact@v4
with:
name: godot-python-${{ matrix.platform }}-${{ matrix.arch }}
name: godot-python-${{ matrix.target.platform }}-${{ matrix.target.arch }}-py${{ matrix.python_version }}-ig${{ matrix.python_standalone_build }}
path: |
bin/**/*
!bin/**/*.lib
!bin/**/*.exp
retention-days: 30

- name: Upload artifacts (macOS)
if: matrix.os == 'macos-latest'
if: matrix.target.os == 'macos-latest'
uses: actions/upload-artifact@v4
with:
name: godot-python-${{ matrix.platform }}-${{ matrix.arch }}
name: godot-python-${{ matrix.target.platform }}-${{ matrix.target.arch }}-py${{ matrix.python_version }}-ig${{ matrix.python_standalone_build }}
path: bin/**/*
retention-days: 30

- name: Release artifact
if: ${{ inputs.release }}
run: |
if [[ "${{ matrix.os }}" == "ubuntu-latest" ]]; then
if [[ "${{ matrix.target.os }}" == "ubuntu-latest" ]]; then
echo "Releasing artifact for linux"
elif [[ "${{ matrix.os }}" == "windows-latest" ]]; then
elif [[ "${{ matrix.target.os }}" == "windows-latest" ]]; then
echo "Releasing artifact for windows"
elif [[ "${{ matrix.os }}" == "macos-latest" ]]; then
elif [[ "${{ matrix.target.os }}" == "macos-latest" ]]; then
echo "Releasing artifact for macOS"
merge:
runs-on: ubuntu-latest
Expand Down
131 changes: 73 additions & 58 deletions SConstruct
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,15 @@ opts.Add(
# godot python options

opts.Add(
PathVariable(
key="python",
help="Path to the `python` to build against.",
default='python3',
validator=(lambda key, val, env: build_utils.validate_executable(key, val, env)
if not env.get('python_lib_dir') else None),
)
key="python_version",
help="Version of python to use. Must be available in the python_standalone_build.",
default='3.12.0',
)

opts.Add(
key="python_standalone_build",
help="Build ID to use for the python version. You can find the list of builds at https://github.com/indygreg/python-build-standalone/releases.",
default="20231002",
)

opts.Add(
Expand Down Expand Up @@ -311,44 +313,52 @@ env.Alias("godot_module_archive", [

from tools.build import prepare_python

prepared_python_config = prepare_python.platform_configs[(env['platform'], env['arch'])]
prepared_python_config = prepare_python.configure(
platform=env['platform'],
arch=env['arch'],
python_version=env['python_version'],
build=env['python_standalone_build'],
)


def _fetch_python(target, source, env):
dest = pathlib.Path(target[0].path).parent
dest.mkdir(parents=True, exist_ok=True)
prepare_python.fetch_python_for_platform(env['platform'], env['arch'], dest)

fetch_python_alias = env.Alias("fetch_python", [
Builder(action = env.Action(_fetch_python, "Fetching Python"))(
env,
target = os.fspath(generated_path / 'python'
/ prepared_python_config.name / pathlib.Path(prepared_python_config.source_url).name),
source = [
],
)
])
try:
prepare_python.fetch_python_for_config(prepared_python_config, target[0])
except Exception as exc:
print(f"Error while fetching python: {exc}")
return 1
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this return code documented by scons? Is there a symbol to use instead?

Copy link
Contributor Author

@Ivorforce Ivorforce Feb 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC any non-0 return code can be returned to make the action fail. Interestingly, it won't even fail if a python exception bubbles up, IIRC. I just defaulted to 1 as a "generic" exception code.


fetched_python_files = env.Command(
target = os.fspath(
generated_path / 'python' / prepared_python_config.name / pathlib.Path(prepared_python_config.source_url).name
),
source = [
],
action=_fetch_python
)


def _prepare_python(target, source, env):
dest = pathlib.Path(target[0].path).parent.resolve()
dest.mkdir(parents=True, exist_ok=True)

src = pathlib.Path(source[0].path).parent.resolve()

env['python'] = prepare_python.prepare_for_platform(env['platform'], env['arch'],
src_dir = src, dest_dir = dest)

prepare_python_alias = env.Alias("prepare_python", [
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was the alias removed? I think the alias may have existed to ensure something else built or for some build optimization (don't remember exactly)

May need to review this part again once I understand what's happening better.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's been a while, but IIRC the Command usage did pretty much the same as Builder(Action) which is in use right now, but more concisely and with less call overhead. I didn't find a reason why it was implemented with aliases and builders.

Builder(action = Action(_prepare_python, "Preparing Python"))(
env,
target = f'bin/{prepared_python_config.name}/python312.zip', # XXX: version
source = [
fetch_python_alias[0].children(),
prepare_python.__file__,
],
)
])
try:
dest = pathlib.Path(target[0].path).parent.resolve()
dest.mkdir(parents=True, exist_ok=True)

src = pathlib.Path(source[0].path).parent.resolve()

env['python'] = prepare_python.prepare_for_platform(prepared_python_config,
src_dir = src, dest_dir = dest)
except Exception as exc:
print(f"Error while preparing python: {exc}")
return 1

prepared_python_files = env.Command(
target = f'bin/{prepared_python_config.name}/python{prepared_python_config.python_version_minor}-lib.zip',
source = [
*fetched_python_files,
prepare_python.__file__,
],
action=_prepare_python
)



Expand Down Expand Up @@ -386,31 +396,36 @@ env.Append(CPPDEFINES = [f'PYGODOT_ARCH=\\"{env["arch"]}\\"'])


def _append_python_config(env, target, **kwargs):
src_dir = generated_path / 'python' / prepared_python_config.name
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to self: became overwhelmed by above, once understand above better review from here to end of file.

env['python'] = os.fspath(prepare_python.get_python_for_platform(env['platform'], env['arch'], src_dir))
try:
src_dir = generated_path / 'python' / prepared_python_config.name
env['python'] = os.fspath(prepare_python.get_python_for_platform(prepared_python_config, src_dir))

from tools.build import python_config
_config_vars = python_config.get_python_config_vars(env)
from tools.build import python_config
_config_vars = python_config.get_python_config_vars(env)

env.Append(LIBPATH = _config_vars.lib_paths)
env.Append(LINKFLAGS = _config_vars.link_flags)
env.Append(LIBS = _config_vars.link_libs)
env.Append(CPPPATH = _config_vars.include_flags)
env.Append(LIBPATH = _config_vars.lib_paths)
env.Append(LINKFLAGS = _config_vars.link_flags)
env.Append(LIBS = _config_vars.link_libs)
env.Append(CPPPATH = _config_vars.include_flags)

if env['platform'] != 'windows':
env.Append(CPPDEFINES = [f'PYTHON_LIBRARY_PATH=\\"{_config_vars.ldlibrary or ""}\\"'])
if env['platform'] != 'windows':
env.Append(CPPDEFINES = [f'PYTHON_LIBRARY_PATH=\\"{_config_vars.ldlibrary or ""}\\"'])

dest = pathlib.Path(target[0].path)
dest.write_text(repr(_config_vars))
dest = pathlib.Path(target[0].path)
dest.write_text(repr(_config_vars))
except Exception as exc:
print(f"Error while appending python config: {exc}")
return 1


append_python_config = Builder(action = Action(_append_python_config, None))(
env, target='src/.generated/.append_python_config', source=None)

env.Depends(append_python_config, prepare_python_alias)
env.AlwaysBuild(append_python_config)
append_python_config_files = env.Command(
source=prepared_python_files,
target='src/.generated/.append_python_config',
action=_append_python_config
)

env.Depends(sources, append_python_config)
env.AlwaysBuild(append_python_config_files)
env.Depends(sources, append_python_config_files)


# library name
Expand All @@ -430,7 +445,7 @@ env["OBJSUFFIX"] = suffix + env["OBJSUFFIX"]
library_name = "libgodot-python{}{}".format(env["suffix"], env["SHLIBSUFFIX"])

library = env.SharedLibrary(
target = f"bin/{env['platform']}-{env['arch']}/{library_name}",
target = f"bin/{prepared_python_config.name}/{library_name}",
source = sources,
)

Expand Down
3 changes: 1 addition & 2 deletions src/extension/extension.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,7 @@ static bool init_python_isolated() {
auto py_major = std::to_string(PY_MAJOR_VERSION);
auto py_minor = std::to_string(PY_MINOR_VERSION);
auto py_version = py_major + "." + py_minor;
auto py_version_no_dot = py_major + py_minor;
auto python_zip_name = "python" + py_version_no_dot + ".zip";
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naming of the zip here is pretty standard. I believe there is a document in the python manual detailing the layout, but don't have a link on hand.

Copy link
Contributor Author

@Ivorforce Ivorforce Feb 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's been a while so I don't remember my reason for changing it. I think it was because I wanted the zip name to be consistent with the lib name, which uses dots (e.g. python3.2). That makes more sense to me, but I suppose if there's a PEP for a python lib zip name, I agree we should adhere to it.

auto python_zip_name = "python" + py_version + "-lib.zip";
auto python_lib_name = "python" + py_version;

add_module_search_path((runtime_config.python_home_path / python_zip_name).string());
Expand Down
Loading