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.

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

Upgrade V8 binaries for 10.9.194.9 version **working** #363

Merged
merged 12 commits into from Jan 30, 2023

Conversation

jacques-n
Copy link
Contributor

Use existing workflows to build latest V8 artifacts as well as code changes to ensure v8go continues to work.

Includes:

  • Update github workflow to use go 1.8 & 1.9
  • 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)
  • Commits are kept separate with merges to certify provenance of all library objects built using existing rogchap/v8go mechanisms.

This is reopen of the previous PR using a different branch name so github action automation doesn't accidentally close or overwrite.

jacques-n and others added 10 commits January 12, 2023 23:11
- 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)
Auto-generated pull request to build V8 for ubuntu-22.04 x86_64
Auto-generated pull request to build V8 for macos-11 x86_64
Auto-generated pull request to build V8 for macos-11 arm64
Auto-generated pull request to build V8 for ubuntu-22.04 arm64
@jacques-n
Copy link
Contributor Author

@fizx, can you review this? It's basically trivial changes other than the v8 upgrade.

@codecov
Copy link

codecov bot commented Jan 19, 2023

Codecov Report

Base: 95.28% // Head: 95.31% // Increases project coverage by +0.03% 🎉

Coverage data is based on head (ab039ba) compared to base (7d843f1).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #363      +/-   ##
==========================================
+ Coverage   95.28%   95.31%   +0.03%     
==========================================
  Files          17       17              
  Lines         594      598       +4     
==========================================
+ Hits          566      570       +4     
  Misses         19       19              
  Partials        9        9              
Impacted Files Coverage Δ
isolate.go 100.00% <100.00%> (ø)
v8go.go 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Collaborator

@RGood RGood left a comment

Choose a reason for hiding this comment

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

Changes look great, thanks for the repo maintenance.

Would like an additional reviewer but you have a 👍 from me

deps/build.py Show resolved Hide resolved
Copy link
Collaborator

@RGood RGood left a comment

Choose a reason for hiding this comment

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

Last ask I have would be to update the CHANGELOG.md

Copy link
Collaborator

@dylanahsmith dylanahsmith left a comment

Choose a reason for hiding this comment

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

https://github.com/rogchap/v8go#v8-dependency says

Due to security concerns of binary blobs hiding malicious code, the V8 binary is built via CI ONLY.

Could you instead open a PR against the #357 branch with the changes needed to fix the build, but without committing the actual V8 build. I can then merge this PR into that branch and trigger the V8 Build workflow (as documented in https://github.com/rogchap/v8go#upgrading-the-v8-binaries) to merge in the V8 binaries.

Comment on lines 16 to 18
# 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
Copy link
Collaborator

Choose a reason for hiding this comment

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

This comment is no longer relevant

Suggested change
# 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

@jacques-n
Copy link
Contributor Author

https://github.com/rogchap/v8go#v8-dependency says

Due to security concerns of binary blobs hiding malicious code, the V8 binary is built via CI ONLY.

Could you instead open a PR against the #357 branch with the changes needed to fix the build, but without committing the actual V8 build. I can then merge this PR into that branch and trigger the V8 Build workflow (as documented in https://github.com/rogchap/v8go#upgrading-the-v8-binaries) to merge in the V8 binaries.

Can do.

FYI: this patch already followed that pattern to address this concern: the v8 build commits within this patch are were generated by Github action CI and are maintained as separate commits to guarantee hash provenance. (You'll need to look at the individual commits of this PR to actually see this-- each is a GH generated PR with merge commit to guarantee hash history)

@dylanahsmith
Copy link
Collaborator

Sorry, my mistake about thinking the build wasn't CI generated. Github Actions seems to now attribute the automated commits to a person (e.g. 208b63e is attributed to me) which made it look like a local build.

@dylanahsmith dylanahsmith dismissed their stale review January 22, 2023 15:35

misunderstanding of build providence

@fizx fizx closed this Jan 23, 2023
@fizx fizx reopened this Jan 23, 2023
@jacques-n
Copy link
Contributor Author

Sorry, my mistake about thinking the build wasn't CI generated. Github Actions seems to now attribute the automated commits to a person (e.g. 208b63e is attributed to me) which made it look like a local build.

I've kept as is.

If you'd like, I can back out the v8 builds but then someone else just has to do them again :D

I've updated the changelog. Let me know if you are happy with this.

@dylanahsmith
Copy link
Collaborator

This branch is out-of-date with the base branch

Looks like the branch protection requires branches to be up to date before merging. I also don't have permission to push a merge of master into this branch to fix it myself for merging. @jacques-n could you get this branch up-to-date so it can be merged?

@jacques-n
Copy link
Contributor Author

Updated

@dylanahsmith dylanahsmith merged commit 02e1763 into rogchap:master Jan 30, 2023
rajantikare pushed a commit to esoptra/v8go that referenced this pull request May 11, 2023
* Upgrade V8 binaries for 10.9.194.9 version

* Fixes to support newest stable v8.

- 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)

* Update V8 static library for macos-11 x86_64

* Update V8 static library for ubuntu-22.04 x86_64

* Update V8 static library for macos-11 arm64

* Update V8 static library for ubuntu-22.04 arm64

* Update changelog and remove no-longer valid comment

---------

Co-authored-by: jacques-n <jacques-n@users.noreply.github.com>
wowngasb added a commit to wowngasb/v8go that referenced this pull request Jan 19, 2024
commit cee5f84
Author: Jonathan Geddes <geddes.jonathan@gmail.com>
Date:   Mon Apr 10 10:09:13 2023 -0600

    shared array buffers (rogchap#378)

    * Initial support for SharedArrayBuffer

commit 0e40e6e
Author: Randy Goodman <mullan17@gmail.com>
Date:   Mon Mar 20 19:27:55 2023 -0400

    v0.9.0 (rogchap#376)

commit be4ff5e
Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Date:   Tue Mar 21 07:17:39 2023 +1100

    Upgrade V8 binaries for 11.1.277.13 version (rogchap#373)

commit 02e1763
Author: Jacques Nadeau <jacquesnadeau@gmail.com>
Date:   Mon Jan 30 06:51:45 2023 -0800

    Upgrade V8 binaries for 10.9.194.9 version **working** (rogchap#363)

    * Upgrade V8 binaries for 10.9.194.9 version

    * Fixes to support newest stable v8.

    - 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)

    * Update V8 static library for macos-11 x86_64

    * Update V8 static library for ubuntu-22.04 x86_64

    * Update V8 static library for macos-11 arm64

    * Update V8 static library for ubuntu-22.04 arm64

    * Update changelog and remove no-longer valid comment

    ---------

    Co-authored-by: jacques-n <jacques-n@users.noreply.github.com>

commit 7d843f1
Author: Kyle Maxwell <kyle@kylemaxwell.com>
Date:   Thu Jan 19 13:17:30 2023 -0800

    v8.Value becomes manually releaseable (rogchap#361)

    * values are manually releaseable

    Co-authored-by: Kyle Maxwell <kyle.maxwell@reddit.com>

commit 1f00b50
Author: Ryan H. Lewis <ryan@ryanlewis.dev>
Date:   Wed Nov 2 14:15:10 2022 -0600

    adding additional profile values (rogchap#341)

commit fc8b9f1
Author: Ryan H. Lewis <ryan@ryanlewis.dev>
Date:   Mon Aug 8 16:10:42 2022 -0600

    updating context documentation (rogchap#335)

    Co-authored-by: Ryan H Lewis <ryan.lewis@reddit.com>

commit da7f43c
Author: Lukas Malkmus <lukasmalkmus@users.noreply.github.com>
Date:   Wed Apr 13 16:20:09 2022 +0200

    Fix typo in promise.go (rogchap#310)

    Literally just fixing a typo in `promise.go` that I discovered while strolling through the code.

commit db78170
Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Date:   Wed Mar 9 14:29:03 2022 -0800

    Upgrade V8 binaries for 9.7.106.19 version (rogchap#278)

    * Upgrade V8 binaries for 9.7.106.19 version

    * Update V8 static library for ubuntu-18.04 x86_64 (rogchap#292)

    Co-authored-by: genevieve <genevieve@users.noreply.github.com>

    * Update V8 static library for ubuntu-18.04 arm64 (rogchap#293)

    Co-authored-by: genevieve <genevieve@users.noreply.github.com>

    * Update V8 static library for macos-11 x86_64 (rogchap#294)

    Co-authored-by: genevieve <genevieve@users.noreply.github.com>

    * Update V8 static library for macos-11 arm64 (rogchap#295)

    Co-authored-by: genevieve <genevieve@users.noreply.github.com>

    Co-authored-by: dylanahsmith <dylanahsmith@users.noreply.github.com>
    Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
    Co-authored-by: genevieve <genevieve@users.noreply.github.com>

commit 5e91d3d
Author: Dylan Thacker-Smith <dylan.smith@shopify.com>
Date:   Wed Jan 12 17:06:50 2022 -0500

    Fix Object.Set with an empty key string (rogchap#276)

commit 1d433f7
Author: Genevieve <genevieve.lesperance@shopify.com>
Date:   Wed Jan 12 10:41:04 2022 -0800

    Use length to ensure null chars do not cause early termination of C string copies/reads (rogchap#272)

    * Intro test case for null terminator string

    * unexpected result: expected ss, got sx00s

    * Fix the assertion, add Unicode symbols

    * Pass go string length into v8::String::New to ensure it does not cut off
    at null chars

    * Reuse the existing RtnString type

    * Formatting

    * Update value_test.go

    Co-authored-by: Dylan Thacker-Smith <dylan.smith@shopify.com>

    * Table tests for NewValue

    * Use std::string constructor in CopyString(Utf8Value) to keep whole
    string

    * Update changelog

    Co-authored-by: Genevieve L'Esperance <glesperance@doximity.com>
    Co-authored-by: Maxim Shirshin <maxim.shirshin@shopify.com>
    Co-authored-by: Dylan Thacker-Smith <dylan.smith@shopify.com>

commit cb8779b
Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Date:   Wed Jan 12 07:51:03 2022 -0800

    Upgrade V8 binaries for 9.7.106.18 version (rogchap#264)

    Co-authored-by: dylanahsmith <dylanahsmith@users.noreply.github.com>
    Co-authored-by: Genevieve <genevieve.lesperance@shopify.com>

commit c0dbfd3
Author: Dylan Thacker-Smith <dylan.smith@shopify.com>
Date:   Wed Jan 12 09:58:36 2022 -0500

    Remove unnecessary nested Locker and Isolate::Scope use (rogchap#275)

    * Remove unnecessary nested Locker and Isolate::Scope use

    * Make ExceptionError static, since it isn't used outside that file

commit ede7cee
Author: Liu Xiangchao <iwind.liu@gmail.com>
Date:   Fri Jan 7 01:33:29 2022 +0800

    modify -fpic to -fPIC (rogchap#263)

commit a0417ad
Author: Dylan Thacker-Smith <dylan.smith@shopify.com>
Date:   Wed Dec 22 15:48:45 2021 -0500

    CHANGELOG: Highlight the subtlety of the Function Call breaking change (rogchap#259)

commit 943fcf9
Author: Aditya Sharma <git@adi.run>
Date:   Wed Dec 22 09:30:54 2021 -0800

    Build v8 for linux_arm64 (rogchap#223)

    * Build v8 for linux_arm64

    * Update V8 static library for ubuntu-18.04 arm64

    Co-authored-by: epk <epk@users.noreply.github.com>

commit e635d48
Author: Genevieve <genevieve.lesperance@shopify.com>
Date:   Mon Dec 13 09:28:33 2021 -0800

    Allocate children array with count (rogchap#256)

commit 6e4af34
Author: Roger Chapman <rogchap@gmail.com>
Date:   Thu Dec 9 10:29:56 2021 +1100

    v0.7.0

commit 762c7a8
Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Date:   Wed Dec 8 15:22:43 2021 -0800

    Upgrade V8 binaries for 9.6.180.12 version (rogchap#231)

    * Upgrade V8 binaries for 9.6.180.12 version

    * Add -ldl flag to linux build to fix missing dlsym errors

    * Update V8 static library for macos-11 x86_64 (rogchap#249)

    Co-authored-by: genevieve <genevieve@users.noreply.github.com>

    * Update V8 static library for ubuntu-18.04 x86_64 (rogchap#250)

    Co-authored-by: genevieve <genevieve@users.noreply.github.com>

    * Update V8 static library for macos-11 arm64 (rogchap#251)

    Co-authored-by: genevieve <genevieve@users.noreply.github.com>

    * Update changelog

    Co-authored-by: dylanahsmith <dylanahsmith@users.noreply.github.com>
    Co-authored-by: Genevieve L'Esperance <genevieve.lesperance@shopify.com>
    Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
    Co-authored-by: genevieve <genevieve@users.noreply.github.com>

commit a92639e
Author: Genevieve <genevieve.lesperance@shopify.com>
Date:   Tue Nov 30 12:59:03 2021 -0800

    Enable intl support and store data in v8 binary (rogchap#242)

    * Enable intl support and store data in v8 binary

    * Update V8 static library for ubuntu-18.04 x86_64

    * Update V8 static library for macos-11 arm64

    * Update changelog

    Co-authored-by: genevieve <genevieve@users.noreply.github.com>

commit f55271d
Author: Genevieve <genevieve.lesperance@shopify.com>
Date:   Mon Nov 22 14:50:43 2021 -0800

    Remove Windows support (rogchap#234)

    * Remove Windows binary support

    * Update CHANGELOG.md

    Co-authored-by: Oliver Fuerst <87430407+ofuerst@users.noreply.github.com>

    * Update CHANGELOG.md

    Co-authored-by: Dylan Thacker-Smith <dylan.smith@shopify.com>

    * Update windows section

    * Update README.md

    Co-authored-by: Dylan Thacker-Smith <dylan.smith@shopify.com>

    * Update README.md

    Co-authored-by: Dylan Thacker-Smith <dylan.smith@shopify.com>

    * Remove windows fossa step

    * Remove deps/windows_x86_64

    Co-authored-by: Oliver Fuerst <87430407+ofuerst@users.noreply.github.com>
    Co-authored-by: Dylan Thacker-Smith <dylan.smith@shopify.com>
    Co-authored-by: Roger Chapman <rogchap@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants