Skip to content

Commit

Permalink
Fixes to support newest stable v8.
Browse files Browse the repository at this point in the history
- Update github workflow to use go 18 & 19
- Update github workflow to use macos-latest
- Update github build workflow to use ubuntu 22.04
- Add gitignore for jetbrains and .gclient_previous files
- Switch cgo build to C++17 and enable sandbox at build time
- Update test with update to date error message
- Remove no longer supported build flag.
- Move initialization to v8go.go and include flag set to avoid flag freezing
- Reorder initialization so allocator is initialized after v8 (required by latest v8)
  • Loading branch information
jacques-n committed Jan 12, 2023
1 parent fdc11ef commit 3f28808
Show file tree
Hide file tree
Showing 9 changed files with 24 additions and 16 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/test.yml
Expand Up @@ -12,11 +12,11 @@ jobs:
name: Tests on ${{ matrix.go-version }} ${{ matrix.platform }}
strategy:
matrix:
go-version: [1.16.8, 1.17.1]
go-version: [1.18.10, 1.19.5]
# 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]
platform: [ubuntu-latest, macos-latest]
runs-on: ${{ matrix.platform }}

steps:
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/v8build.yml
Expand Up @@ -14,7 +14,7 @@ jobs:
#
# 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]
platform: [ubuntu-22.04, macos-11]
arch: [x86_64, arm64]
runs-on: ${{ matrix.platform }}
steps:
Expand All @@ -27,10 +27,10 @@ jobs:
run: cd deps/depot_tools && git config --unset-all remote.origin.fetch; git config --add remote.origin.fetch +refs/heads/*:refs/remotes/origin/*
shell: bash
- name: Install g++-aarch64-linux-gnu
if: matrix.platform == 'ubuntu-18.04' && matrix.arch == 'arm64'
if: matrix.platform == 'ubuntu-22.04' && matrix.arch == 'arm64'
run: sudo apt update && sudo apt install g++-aarch64-linux-gnu -y
- name: Build V8 linux
if: matrix.platform == 'ubuntu-18.04'
if: matrix.platform == 'ubuntu-22.04'
run: cd deps && ./build.py --no-clang --arch ${{ matrix.arch }}
- name: Build V8 macOS
if: matrix.platform == 'macos-11'
Expand Down
4 changes: 2 additions & 2 deletions .gitignore
Expand Up @@ -6,7 +6,7 @@
.gclient_entries

deps/darwin-x86_64/libv8_debug.a

deps/.gclient_previous*
c.out

.idea/*
/v8go.test
2 changes: 1 addition & 1 deletion cgo.go
Expand Up @@ -6,7 +6,7 @@ package v8go

//go:generate clang-format -i --verbose -style=Chromium v8go.h v8go.cc

// #cgo CXXFLAGS: -fno-rtti -fPIC -std=c++14 -DV8_COMPRESS_POINTERS -DV8_31BIT_SMIS_ON_64BIT_ARCH -I${SRCDIR}/deps/include -Wall
// #cgo CXXFLAGS: -fno-rtti -fPIC -std=c++17 -DV8_COMPRESS_POINTERS -DV8_31BIT_SMIS_ON_64BIT_ARCH -I${SRCDIR}/deps/include -Wall -DV8_ENABLE_SANDBOX
// #cgo LDFLAGS: -pthread -lv8
// #cgo darwin,amd64 LDFLAGS: -L${SRCDIR}/deps/darwin_x86_64
// #cgo darwin,arm64 LDFLAGS: -L${SRCDIR}/deps/darwin_arm64
Expand Down
2 changes: 1 addition & 1 deletion context_test.go
Expand Up @@ -48,7 +48,7 @@ func TestJSExceptions(t *testing.T) {
origin string
err string
}{
{"SyntaxError", "bad js syntax", "syntax.js", "SyntaxError: Unexpected identifier"},
{"SyntaxError", "bad js syntax", "syntax.js", "SyntaxError: Unexpected identifier 'js'"},
{"ReferenceError", "add()", "add.js", "ReferenceError: add is not defined"},
}

Expand Down
1 change: 0 additions & 1 deletion deps/build.py
Expand Up @@ -67,7 +67,6 @@
v8_enable_i18n_support=true
icu_use_data_file=false
v8_enable_test_features=false
v8_untrusted_code_mitigations=false
exclude_unwind_tables=true
"""

Expand Down
4 changes: 1 addition & 3 deletions isolate.go
Expand Up @@ -52,9 +52,7 @@ type HeapStatistics struct {
// An *Isolate can be used as a v8go.ContextOption to create a new
// Context, rather than creating a new default Isolate.
func NewIsolate() *Isolate {
v8once.Do(func() {
C.Init()
})
initializeIfNecessary()
iso := &Isolate{
ptr: C.NewIsolate(),
cbs: make(map[int]FunctionCallback),
Expand Down
8 changes: 5 additions & 3 deletions v8go.cc
Expand Up @@ -18,7 +18,7 @@
using namespace v8;

auto default_platform = platform::NewDefaultPlatform();
auto default_allocator = ArrayBuffer::Allocator::NewDefaultAllocator();
ArrayBuffer::Allocator* default_allocator;

const int ScriptCompilerNoCompileOptions = ScriptCompiler::kNoCompileOptions;
const int ScriptCompilerConsumeCodeCache = ScriptCompiler::kConsumeCodeCache;
Expand Down Expand Up @@ -149,6 +149,8 @@ void Init() {
#endif
V8::InitializePlatform(default_platform.get());
V8::Initialize();

default_allocator = ArrayBuffer::Allocator::NewDefaultAllocator();
return;
}

Expand Down Expand Up @@ -243,7 +245,7 @@ RtnUnboundScript IsolateCompileUnboundScript(IsolatePtr iso,
opts.cachedData.length);
}

ScriptOrigin script_origin(ogn);
ScriptOrigin script_origin(iso, ogn);

ScriptCompiler::Source source(src, script_origin, cached_data);

Expand Down Expand Up @@ -634,7 +636,7 @@ RtnValue RunScript(ContextPtr ctx, const char* source, const char* origin) {
return rtn;
}

ScriptOrigin script_origin(ogn);
ScriptOrigin script_origin(iso, ogn);
Local<Script> script;
if (!Script::Compile(local_ctx, src, &script_origin).ToLocal(&script)) {
rtn.error = ExceptionError(try_catch, iso, local_ctx);
Expand Down
9 changes: 9 additions & 0 deletions v8go.go
Expand Up @@ -29,3 +29,12 @@ func SetFlags(flags ...string) {
C.SetFlags(cflags)
C.free(unsafe.Pointer(cflags))
}

func initializeIfNecessary() {
v8once.Do(func() {
cflags := C.CString("--no-freeze_flags_after_init")

This comment has been minimized.

Copy link
@anshuldata

anshuldata Jan 13, 2023

Just curious, what this flag does and without it what behaviour were you seeing ?

This comment has been minimized.

Copy link
@jacques-n

jacques-n Jan 13, 2023

Author Contributor

The v8 package now freezes the configuration by default on initialization. You can optionally disable this behavior before first initialization (with the flag here). Since there were existing tests for setting flags post initialization in v8go, I decided to avoid freezing for now. Realistically, I think we should probably remove the SetFlags method and remove this.

This comment has been minimized.

Copy link
@jacques-n

jacques-n Jan 13, 2023

Author Contributor

Without this change, any call to SetFlags after initialization would crash since it hits an assertion.

This comment has been minimized.

Copy link
@anshuldata

anshuldata Jan 13, 2023

got it, Thanks

defer C.free(unsafe.Pointer(cflags))
C.SetFlags(cflags)
C.Init()
})
}

0 comments on commit 3f28808

Please sign in to comment.