Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Build v8 for Apple Silicon #202

Merged
merged 11 commits into from
Oct 28, 2021
Merged

Build v8 for Apple Silicon #202

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
8394753
Build v8 for darwin_arm64
epk Oct 24, 2021
de01fba
./deps/build.py --arch arm64
epk Oct 24, 2021
e59012b
import rogchap.com/v8go/deps/darwin_arm64 in cgo.go
epk Oct 24, 2021
960dec5
Bump Go version
epk Oct 24, 2021
89d7ee9
revert to ubuntu-18.04
epk Oct 24, 2021
0e4fe11
PR comments
epk Oct 27, 2021
567e50b
update PR template to include arch
epk Oct 27, 2021
e063528
Revert "./deps/build.py --arch arm64"
dylanahsmith Oct 28, 2021
f3289dd
Update V8 static library for macos-11 arm64 (#216)
github-actions[bot] Oct 28, 2021
bfcd56f
Add changelog entry
dylanahsmith Oct 28, 2021
f4c24fb
Merge branch 'master' into epk/darwin_arm64
dylanahsmith Oct 28, 2021
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
9 changes: 6 additions & 3 deletions .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,13 @@ jobs:
name: Tests on ${{ matrix.go-version }} ${{ matrix.platform }}
strategy:
matrix:
Copy link
Owner

Choose a reason for hiding this comment

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

Are we missing the arch values for this pipeline?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can't test against darwin/arm64 without access to a M1 action runner

Run CGO_ENABLED=1 GOOS=darwin GOARCH=arm64 SDKROOT=$(xcrun --sdk macosx --show-sdk-path) go test -v -coverprofile c.out ./...
6
fork/exec /var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/go-build300881077/b001/v8go.test: bad CPU type in executable
7
FAIL	rogchap.com/v8go	0.005s
8
?   	rogchap.com/v8go/deps/darwin_arm64	[no test files]
9
?   	rogchap.com/v8go/deps/darwin_x86_64	[no test files]
10
?   	rogchap.com/v8go/deps/include	[no test files]
11
?   	rogchap.com/v8go/deps/include/cppgc	[no test files]
12
?   	rogchap.com/v8go/deps/include/libplatform	[no test files]
13
?   	rogchap.com/v8go/deps/linux_x86_64	[no test files]
14
?   	rogchap.com/v8go/deps/windows_x86_64	[no test files]
15
FAIL

go-version: [1.12.17, 1.16.4]
platform: [ubuntu-latest, macos-latest, windows-latest]
go-version: [1.16.8, 1.17.1]
Copy link
Collaborator

@dylanahsmith dylanahsmith Oct 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Github is showing "Some checks haven’t completed yet", but then lists the status for Go versions that were updated here to no longer run:

Tests on 1.12.17 ubuntu-latest Expected — Waiting for status to be reported (Required)
Tests on 1.12.17 windows-latest Expected — Waiting for status to be reported (Required)
Tests on 1.16.4 macos-latest Expected — Waiting for status to be reported (Required)
Tests on 1.16.4 ubuntu-latest Expected — Waiting for status to be reported (Required)
Tests on 1.16.4 windows-latest Expected — Waiting for status to be reported (Required)

This is blocking me from merging. @rogchap you could do the merge to workaround this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, it looks like the github branch protection rules for the repo need to be updated, since the name of these CI jobs has changed.

Copy link
Owner

Choose a reason for hiding this comment

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

I can fix this.

# We use macos-11 over macos-latest because macos-latest defaults to Catalina(10.15) and not Big Sur(11.0)
# We can switch to macos-latest whenever Big Sur becomes the default
# See https://github.com/actions/virtual-environments#available-environments
platform: [ubuntu-latest, macos-11, windows-latest]
epk marked this conversation as resolved.
Show resolved Hide resolved
runs-on: ${{ matrix.platform }}

steps:
- name: Install Go
uses: actions/setup-go@v2
Expand Down
32 changes: 21 additions & 11 deletions .github/workflows/v8build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,23 @@ on: workflow_dispatch

jobs:
build:
name: Build V8 for ${{ matrix.platform }}
name: Build V8 for ${{ matrix.platform }} ${{ matrix.arch }}
strategy:
fail-fast: false
matrix:
platform: [ubuntu-18.04, macos-latest, windows-latest]
epk marked this conversation as resolved.
Show resolved Hide resolved
# We use macos-11 over macos-latest because macos-latest defaults to Catalina(10.15) and not Big Sur(11.0)
# We can switch to macos-latest whenever Big Sur becomes the default
# See https://github.com/actions/virtual-environments#available-environments
#
# We need xcode 12.4 or newer to cross compile between arm64/amd64
# https://github.com/actions/virtual-environments/blob/main/images/macos/macos-11-Readme.md#xcode
platform: [ubuntu-18.04, macos-11, windows-latest]
arch: [x86_64, arm64]
exclude:
- platform: ubuntu-18.04
arch: arm64
epk marked this conversation as resolved.
Show resolved Hide resolved
- platform: windows-latest
arch: arm64
runs-on: ${{ matrix.platform }}
steps:
- name: Checkout
Expand All @@ -21,27 +33,25 @@ jobs:
shell: bash
- name: Build V8 linux
if: matrix.platform == 'ubuntu-18.04'
run: cd deps && ./build.py --no-clang
run: cd deps && ./build.py --no-clang --arch ${{ matrix.arch }}
- name: Build V8 macOS
if: matrix.platform == 'macos-latest'
run: cd deps && ./build.py
if: matrix.platform == 'macos-11'
run: cd deps && ./build.py --arch ${{ matrix.arch }}
- name: Add MSYS2 to PATH
if: matrix.platform == 'windows-latest'
run: echo "C:\msys64\mingw64\bin" >> $GITHUB_PATH
shell: bash
- name: Build V8 windows
if: matrix.platform == 'windows-latest'
run: cd deps; python build.py --no-clang
run: cd deps; python build.py --no-clang --arch ${{ matrix.arch }}
env:
MSYSTEM: MINGW64
DEPOT_TOOLS_WIN_TOOLCHAIN: 0
- name: Create PR
uses: peter-evans/create-pull-request@v3
with:
commit-message: Update V8 static library for ${{ matrix.platform }}
branch: v8-lib
commit-message: Update V8 static library for ${{ matrix.platform }} ${{ matrix.arch }}
branch-suffix: random
delete-branch: true
title: V8 static library for ${{ matrix.platform }}
body: Auto-generated pull request to build V8 for ${{ matrix.platform }}

title: V8 static library for ${{ matrix.platform }} ${{ matrix.arch }}
body: Auto-generated pull request to build V8 for ${{ matrix.platform }} ${{ matrix.arch }}
2 changes: 0 additions & 2 deletions .github/workflows/v8upgrade.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,5 +40,3 @@ jobs:
delete-branch: true
title: ${{steps.pr_metadata.outputs.pr_commit_message}}
body: ${{steps.pr_metadata.outputs.pr_body}}


1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Support for calling `IsExecutionTerminating` on isolate to check if execution is still terminating.
- Support for setting and getting internal fields for template object instances
- Support for CPU profiling
- Add V8 build for Apple Silicon

### Changed
- Removed error return value from NewIsolate which never fails
Expand Down
4 changes: 3 additions & 1 deletion cgo.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ package v8go

// #cgo CXXFLAGS: -fno-rtti -fpic -std=c++14 -DV8_COMPRESS_POINTERS -DV8_31BIT_SMIS_ON_64BIT_ARCH -I${SRCDIR}/deps/include
// #cgo LDFLAGS: -pthread -lv8
// #cgo darwin LDFLAGS: -L${SRCDIR}/deps/darwin_x86_64
// #cgo darwin,amd64 LDFLAGS: -L${SRCDIR}/deps/darwin_x86_64
// #cgo darwin,arm64 LDFLAGS: -L${SRCDIR}/deps/darwin_arm64
Comment on lines +11 to +12
Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/veandco/go-sdl2/blob/master/mix/sdl_mixer_cgo_static.go#L7-L8

// #cgo linux LDFLAGS: -L${SRCDIR}/deps/linux_x86_64
// #cgo windows LDFLAGS: -L${SRCDIR}/deps/windows_x86_64 -static -ldbghelp -lssp -lwinmm -lz
import "C"
Expand All @@ -17,6 +18,7 @@ import "C"
// contain V8 libraries and headers which otherwise would be ignored.
// DO NOT REMOVE
import (
_ "rogchap.com/v8go/deps/darwin_arm64"
_ "rogchap.com/v8go/deps/darwin_x86_64"
_ "rogchap.com/v8go/deps/include"
_ "rogchap.com/v8go/deps/linux_x86_64"
Expand Down
29 changes: 24 additions & 5 deletions deps/build.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,20 @@
import shutil
import argparse

valid_archs = ['arm64', 'x86_64']
# "x86_64" is called "amd64" on Windows
current_arch = platform.uname()[4].lower().replace("amd64", "x86_64")
default_arch = current_arch if current_arch in valid_archs else None

parser = argparse.ArgumentParser()
parser.add_argument('--debug', dest='debug', action='store_true')
parser.add_argument('--no-clang', dest='clang', action='store_false')
parser.add_argument('--arch',
dest='arch',
action='store',
choices=valid_archs,
default=default_arch,
required=default_arch is None)
parser.set_defaults(debug=False, clang=True)
args = parser.parse_args()

Expand Down Expand Up @@ -40,6 +51,8 @@
gn_args = """
is_debug=%s
is_clang=%s
target_cpu="%s"
v8_target_cpu="%s"
clang_use_chrome_plugins=false
use_custom_libcxx=false
use_sysroot=false
Expand Down Expand Up @@ -70,8 +83,12 @@ def cmd(args):

def os_arch():
u = platform.uname()
# "x86_64" is called "amd64" on Windows
return (u[0] + "_" + u[4]).lower().replace("amd64", "x86_64")
return u[0].lower() + "_" + args.arch

def v8_arch():
if args.arch == "x86_64":
return "x64"
return args.arch

def apply_mingw_patches():
v8_build_path = os.path.join(v8_path, "build")
Expand All @@ -96,7 +113,7 @@ def main():
v8deps()
if is_windows:
apply_mingw_patches()

gn_path = os.path.join(tools_path, "gn")
assert(os.path.exists(gn_path))
ninja_path = os.path.join(tools_path, "ninja" + (".exe" if is_windows else ""))
Expand All @@ -112,9 +129,11 @@ def main():
# compiled library by an order of magnitude and further slow down compilation
symbol_level = 1 if args.debug else 0
strip_debug_info = 'false' if args.debug else 'true'
gnargs = gn_args % (is_debug, is_clang, symbol_level, strip_debug_info)

arch = v8_arch()
gnargs = gn_args % (is_debug, is_clang, arch, arch, symbol_level, strip_debug_info)
gen_args = gnargs.replace('\n', ' ')

subprocess.check_call(cmd([gn_path, "gen", build_path, "--args=" + gen_args]),
cwd=v8_path,
env=env)
Expand Down
Binary file added deps/darwin_arm64/libv8.a
View file
Open in desktop
Binary file not shown.
3 changes: 3 additions & 0 deletions deps/darwin_arm64/vendor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// Package darwin_arm64 is required to provide support for vendoring modules
// DO NOT REMOVE
package darwin_arm64
2 changes: 1 addition & 1 deletion deps/depot_tools
Open in desktop
Submodule depot_tools updated from 1dbd65 to c54d00
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
module rogchap.com/v8go

go 1.12
go 1.16
Copy link
Owner

Choose a reason for hiding this comment

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

Likely a good idea to update to minimum of 1.16; but just making sure this will be ok for others? @dylanahsmith

Copy link
Collaborator

Choose a reason for hiding this comment

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

We are on 1.17. I think we should support what Go itself supports, so 👍 to this change.