-
-
Notifications
You must be signed in to change notification settings - Fork 221
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
Remove Windows support #234
Conversation
Co-authored-by: Oliver Fuerst <[email protected]>
Co-authored-by: Dylan Thacker-Smith <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #234 +/- ##
=======================================
Coverage 95.87% 95.87%
=======================================
Files 17 17
Lines 582 582
=======================================
Hits 558 558
Misses 15 15
Partials 9 9 Continue to review full report at Codecov.
|
Co-authored-by: Dylan Thacker-Smith <[email protected]>
Co-authored-by: Dylan Thacker-Smith <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving, but let's wait to hear from @rogchap
Probably should also delete https://github.com/rogchap/v8go/tree/master/deps/windows_x86_64 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to me; I've spent too many sleepless nights fighting to get Windows building right, with little success.
@rogchap looks like these will need to be removed from the required checks before merging. |
commit cee5f84 Author: Jonathan Geddes <[email protected]> Date: Mon Apr 10 10:09:13 2023 -0600 shared array buffers (rogchap#378) * Initial support for SharedArrayBuffer commit 0e40e6e Author: Randy Goodman <[email protected]> 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 <[email protected]> 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 <[email protected]> commit 7d843f1 Author: Kyle Maxwell <[email protected]> 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 <[email protected]> commit 1f00b50 Author: Ryan H. Lewis <[email protected]> Date: Wed Nov 2 14:15:10 2022 -0600 adding additional profile values (rogchap#341) commit fc8b9f1 Author: Ryan H. Lewis <[email protected]> Date: Mon Aug 8 16:10:42 2022 -0600 updating context documentation (rogchap#335) Co-authored-by: Ryan H Lewis <[email protected]> commit da7f43c Author: Lukas Malkmus <[email protected]> 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 <[email protected]> * Update V8 static library for ubuntu-18.04 arm64 (rogchap#293) Co-authored-by: genevieve <[email protected]> * Update V8 static library for macos-11 x86_64 (rogchap#294) Co-authored-by: genevieve <[email protected]> * Update V8 static library for macos-11 arm64 (rogchap#295) Co-authored-by: genevieve <[email protected]> Co-authored-by: dylanahsmith <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: genevieve <[email protected]> commit 5e91d3d Author: Dylan Thacker-Smith <[email protected]> Date: Wed Jan 12 17:06:50 2022 -0500 Fix Object.Set with an empty key string (rogchap#276) commit 1d433f7 Author: Genevieve <[email protected]> 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 <[email protected]> * Table tests for NewValue * Use std::string constructor in CopyString(Utf8Value) to keep whole string * Update changelog Co-authored-by: Genevieve L'Esperance <[email protected]> Co-authored-by: Maxim Shirshin <[email protected]> Co-authored-by: Dylan Thacker-Smith <[email protected]> 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 <[email protected]> Co-authored-by: Genevieve <[email protected]> commit c0dbfd3 Author: Dylan Thacker-Smith <[email protected]> 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 <[email protected]> Date: Fri Jan 7 01:33:29 2022 +0800 modify -fpic to -fPIC (rogchap#263) commit a0417ad Author: Dylan Thacker-Smith <[email protected]> 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 <[email protected]> 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 <[email protected]> commit e635d48 Author: Genevieve <[email protected]> Date: Mon Dec 13 09:28:33 2021 -0800 Allocate children array with count (rogchap#256) commit 6e4af34 Author: Roger Chapman <[email protected]> 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 <[email protected]> * Update V8 static library for ubuntu-18.04 x86_64 (rogchap#250) Co-authored-by: genevieve <[email protected]> * Update V8 static library for macos-11 arm64 (rogchap#251) Co-authored-by: genevieve <[email protected]> * Update changelog Co-authored-by: dylanahsmith <[email protected]> Co-authored-by: Genevieve L'Esperance <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: genevieve <[email protected]> commit a92639e Author: Genevieve <[email protected]> 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 <[email protected]> commit f55271d Author: Genevieve <[email protected]> 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 <[email protected]> * Update CHANGELOG.md Co-authored-by: Dylan Thacker-Smith <[email protected]> * Update windows section * Update README.md Co-authored-by: Dylan Thacker-Smith <[email protected]> * Update README.md Co-authored-by: Dylan Thacker-Smith <[email protected]> * Remove windows fossa step * Remove deps/windows_x86_64 Co-authored-by: Oliver Fuerst <[email protected]> Co-authored-by: Dylan Thacker-Smith <[email protected]> Co-authored-by: Roger Chapman <[email protected]>
Context
The v8go library currently supports binaries for mac, linux, and windows. The last release of this library with an updated version of v8 was in May. We have been making efforts to automate the upgrade process here so that we can ship releases more regularly as patches are released/CVEs are found and fixed. However, we've been unable to merge any of those upgrade PRs opened by the automation @GustavoCaso added because it fails to build the window binary on later versions of v8.
The windows binary is built using MinGW and relies on patches in a different repository to make the v8 binary compile. These patches have not been updated for later versions and so they fail to apply on upgrade attempts.
Additionally, there seems to be incompatibility with enabling Internationalization/ICU support with the existing windows build process because of what appears to be a reliance on something provided by VSCode.
As such:
Proposal
This PR removes support for windows from v8go as a result of the above issues. If a user of the library can contribute a fix to the above issues now, great. We tried asking in the #v8go gophers channel a couple weeks ago but did not get much of a response. Until such time as a user is able to contribute the changes needed to make the windows binary build on recent versions of v8 & with intl support enabled, we can remove it as a blocker allowing us to cut releases of v8go with the latest v8 update for mac and linux, as well as with intl support.