Skip to content

Fix the builds on macOS (universal). #26

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 28, 2024
Merged
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
34 changes: 21 additions & 13 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,22 @@ on:

jobs:
build:
name: Build on ${{ matrix.os }}
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [ubuntu-latest, windows-latest]
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

steps:
- name: Checkout repository
Expand All @@ -39,39 +50,39 @@ jobs:
- name: Build extension (Linux)
if: matrix.os == 'ubuntu-latest'
run: |
scons platform=linux arch=x86_64 single_source=true
scons platform=${{ matrix.platform }} arch=${{ matrix.arch }} single_source=true

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

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

- name: Create archive (Linux)
if: matrix.os == 'ubuntu-latest'
run: |
cd bin/
zip -q -r ../godot-python-linux-x86_64.zip *
zip -q -r ../godot-python-${{ matrix.platform }}-${{ matrix.arch }}.zip *
cd ../

- name: Upload artifacts (Linux)
if: matrix.os == 'ubuntu-latest'
uses: actions/upload-artifact@v3
with:
name: godot-python-linux-x86_64
name: godot-python-${{ matrix.platform }}-${{ matrix.arch }}
path: godot-python*.zip
retention-days: 30

- name: Upload artifacts (Windows)
if: matrix.os == 'windows-latest'
uses: actions/upload-artifact@v3
with:
name: godot-python-windows-x86_64
name: godot-python-${{ matrix.platform }}-${{ matrix.arch }}
path: |
bin/**/*
!bin/**/*.lib
Expand All @@ -82,11 +93,8 @@ jobs:
if: matrix.os == 'macos-latest'
uses: actions/upload-artifact@v3
with:
name: godot-python-macos-universal
path: |
bin/**/*
!bin/**/*.lib
!bin/**/*.exp
name: godot-python-${{ matrix.platform }}-${{ matrix.arch }}
path: bin/**/*
retention-days: 30

- name: Release artifact
Expand Down
11 changes: 10 additions & 1 deletion SConstruct
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,11 @@ python_sources = set()

sources.update(pathlib.Path('src').glob('**/*.cpp'))

if env['platform'] == 'macos':
# For Objective-C++ files
env.Append(CCFLAGS = ['-ObjC++'])
sources.update(pathlib.Path('src').glob('**/*.mm'))

for ext in ('py', 'pyc', 'json', 'svg', 'md'):
python_sources.update(pathlib.Path('lib').glob(f'**/*.{ext}'))

Expand Down Expand Up @@ -340,14 +345,18 @@ strip = env.get('strip', False)
if not env.get('is_msvc'):
env.Append(CCFLAGS = ['-fvisibility=hidden', *['-flto'] * with_lto]) # XXX
env.Append(LINKFLAGS = ['-fvisibility=hidden', *['-flto'] * with_lto, *['-s'] * strip]) # XXX

else:
env.Append(LIBS = ['Shell32.lib', ])


if env['platform'] == 'windows':
# linker has trouble if the table is too large
env.Append(CPPDEFINES = ['CLASS_VIRTUAL_CALL_TABLE_SIZE=512'])
elif env['platform'] == 'macos':
# Need to set the rpath for relative loading of libpython to succeed.
# This doesn't work for some reason
# env.Replace(RPATH=['@loader_path'])
env.Append(LINKFLAGS=['-Wl,-rpath,@loader_path'])


env.Prepend(CPPPATH=['src', os.fspath(generated_path), 'extern/pybind11/include'])
Expand Down
15 changes: 15 additions & 0 deletions src/util/macos.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#ifndef MACOS_H
#define MACOS_H

#include <vector>
#include <string>
#include <filesystem>

namespace macos {

// get the full argv passed to the main process
std::vector<std::string> get_argv();

}

#endif //MACOS_H
13 changes: 13 additions & 0 deletions src/util/macos.mm
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#include "util/macos.h"

#import <Foundation/Foundation.h>


std::vector<std::string> macos::get_argv() {
std::vector<std::string> argv;
NSArray *argv_objc = [[NSProcessInfo processInfo] arguments];
for (NSString *arg in argv_objc) {
argv.push_back([arg UTF8String]);
}
return argv;
}
34 changes: 28 additions & 6 deletions src/util/system.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,13 @@
#ifdef UNIX_ENABLED
#include <dlfcn.h>
#include <unistd.h>
#include <cstdlib>
#endif

#ifdef MACOS_ENABLED
#include <cerrno>
#include <libproc.h>
#include "util/macos.h"
#endif

#ifdef WINDOWS_ENABLED
Expand All @@ -29,8 +36,10 @@ namespace pygodot {
void system_quick_exit(int status) {
#ifdef WINDOWS_ENABLED
TerminateProcess(GetCurrentProcess(), status);
#else
#elif defined(_GLIBCXX_HAVE_QUICK_EXIT)
std::quick_exit(status);
#else
std::_Exit(status);
#endif
}

Expand Down Expand Up @@ -58,11 +67,22 @@ std::filesystem::path get_executable_path() {
memset(buffer, 0, sizeof(buffer));
ssize_t len = readlink("/proc/self/exe", buffer, sizeof(buffer));
return std::filesystem::path(buffer);
#endif
#ifdef WINDOWS_ENABLED
#elif defined(MACOS_ENABLED)
char pathbuf[PROC_PIDPATHINFO_MAXSIZE];

const pid_t pid = getpid();
const int ret = proc_pidpath(pid, pathbuf, sizeof(pathbuf));
if (ret <= 0) {
fprintf(stderr, "PID %d: proc_pidpath ();\n", pid);
fprintf(stderr, " %s\n", strerror(errno));
}
return std::filesystem::path(pathbuf);
#elif defined(WINDOWS_ENABLED)
WCHAR buffer[4096];
GetModuleFileNameW(nullptr, buffer, 4096);
return std::filesystem::path(buffer);
#else
#error System not supported.
#endif
}

Expand All @@ -87,9 +107,9 @@ std::vector<std::string> get_argv() {
}

fclose(cmdline_file);
#endif

#ifdef WINDOWS_ENABLED
#elif defined(MACOS_ENABLED)
return macos::get_argv();
#elif defined(WINDOWS_ENABLED)
int num_args;
LPWSTR* arg_list = CommandLineToArgvW(GetCommandLineW(), &num_args);

Expand All @@ -105,6 +125,8 @@ std::vector<std::string> get_argv() {
args.emplace_back(arg);
}
}
#else
#error System not supported.
#endif

return args;
Expand Down
4 changes: 2 additions & 2 deletions test/python.gdextension
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ entry_symbol = "python_extension_init"

[libraries]

macos = "res://bin/macos/libgodot-python.macos.framework"
macos.x86_64 = "res://bin/macos-x86_64/libgodot-python.macos.x86_64.dylib"
macos.arm64 = "res://bin/macos-arm64/libgodot-python.macos.arm64.dylib"

windows.x86_32 = "res://bin/windows-x86_32/libgodot-python.windows.x86_32.dll"
windows.x86_64 = "res://bin/windows-x86_64/libgodot-python.windows.x86_64.dll"
Expand All @@ -16,4 +17,3 @@ linux.rv64 = "res://bin/linux-rv64/libgodot-python.linux.rv64.so"

android.x86_64 = "res://bin/android-x86_64/libgodot-python.android.x86_64.so"
android.arm64 = "res://bin/android-arm64/libgodot-python.android.arm64.so"

6 changes: 3 additions & 3 deletions tools/build/build_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,9 @@ def process_arch(env):
# No architecture specified. Default to arm64 if building for Android,
# universal if building for macOS or iOS, wasm32 if building for web,
# otherwise default to the host architecture.
if env["platform"] in ["macos", "ios"]:
env["arch"] = "universal"
elif env["platform"] == "android":
# if env["platform"] in ["macos", "ios"]:
# env["arch"] = "universal"
if env["platform"] == "android":
env["arch"] = "arm64"
elif env["platform"] == "web":
env["arch"] = "wasm32"
Expand Down
40 changes: 38 additions & 2 deletions tools/build/prepare_python.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,32 @@ def add_platform_config(*args, **kwargs):
executable = 'python.exe',
)

add_platform_config(
platform = 'macos',
arch = 'x86_64',
source_url = 'https://github.com/indygreg/python-build-standalone/releases/download/'
'20231002/cpython-3.12.0+20231002-x86_64-apple-darwin-install_only.tar.gz',
so_suffixes = ['.so', '.dylib'],
ext_suffixes = ['.so'],
so_path = 'lib/libpython3.12.dylib',
python_lib_dir = 'lib/python3.12',
python_ext_dir = 'lib/python3.12/lib-dynload',
executable = 'bin/python3.12',
)

add_platform_config(
platform = 'macos',
arch = 'arm64',
source_url = 'https://github.com/indygreg/python-build-standalone/releases/download/'
'20231002/cpython-3.12.0+20231002-aarch64-apple-darwin-install_only.tar.gz',
so_suffixes = ['.so', '.dylib'],
ext_suffixes = ['.so'],
so_path = 'lib/libpython3.12.dylib',
python_lib_dir = 'lib/python3.12',
python_ext_dir = 'lib/python3.12/lib-dynload',
executable = 'bin/python3.12',
)


def fetch_python_for_platform(platform: str, arch: str, dest_dir: pathlib.Path):
config = platform_configs[(platform, arch)]
Expand All @@ -80,11 +106,21 @@ def prepare_for_platform(platform: str, arch: str,
shutil.unpack_archive(src_dir / pathlib.Path(config.source_url).name, extract_dir = src_dir)

src = src_dir / 'python'
src_lib_path = src / config.so_path
lib_filename = pathlib.Path(config.so_path).name

if platform == 'macos':
# Rename the library id (which we depend on) to be in @rpath.
# (it defaults to /install/lib/)
subprocess.run(['install_name_tool', '-id', f'@rpath/{lib_filename}', src_lib_path], check=True)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should maybe be performed on the copy instead, but that doesn't work because the copy isn't what is being linked, rendering the id change useless, and the final binary links to /install/lib/.


dest_dir.mkdir(parents=True, exist_ok=True)
shutil.copy2(src_lib_path, dest_dir)

shutil.copy2(src / config.so_path, dest_dir)
subprocess.run(['strip', '-s', str(dest_dir / pathlib.Path(config.so_path).name)], check=True)
if platform == 'macos':
subprocess.run(['strip', '-x', dest_dir / lib_filename], check=True)
else:
subprocess.run(['strip', '-s', dest_dir / lib_filename], check=True)

if (src / config.python_ext_dir).exists():
dest_ext_dir = dest_dir / 'python3.12' / 'lib-dynload'
Expand Down
2 changes: 1 addition & 1 deletion tools/build/python_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ def normpath(path: str) -> str:

config_vars.link_libs = _stable_unique([
*itertools.chain(*(
(v.removeprefix('-l') for v in value.split())
(v.removeprefix('-l') for v in value.split() if v.startswith('-l'))
for name in ('LIBS', 'SYSLIBS')
if (value := sysconfig_vars.get(name))
)),
Expand Down
Loading