-
-
Notifications
You must be signed in to change notification settings - Fork 223
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
Automate v8 source upgrade #181
Automate v8 source upgrade #181
Conversation
556c3b2
to
6b70808
Compare
Codecov Report
@@ Coverage Diff @@
## master #181 +/- ##
=======================================
Coverage 95.59% 95.59%
=======================================
Files 16 16
Lines 545 545
=======================================
Hits 521 521
Misses 15 15
Partials 9 9 Continue to review full report at Codecov.
|
eee259c
to
d57201c
Compare
- Created an scheduled workflow that runs every month, which automated the steps to upgrade the v8 binaries from source https://github.com/rogchap/v8go#upgrading-the-v8-binaries - Created a new workflow that is triggered by a pull_request which build the v8 binaries for MacOs and Linux. Window is excluded until we figured out a way to solve the compiler issues.
…hub actions strategy.fail-fast attribute
… upgrade workflow
d57201c
to
945bd17
Compare
@rogchap would you be able to have a look? Thanks 😄 |
… the latest v8 version
|
||
on: | ||
schedule: | ||
- cron: '0 0 * * *' # Run every day |
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.
Stable releases are on average every 4 weeks. Running every day seems excessive, but I guess most of the time it will just be a version check.
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.
Roughly every 4 weeks a new major Stable release is done.
(Source: https://v8.dev/docs/release-process#stable-releases)
Emphasis on "major" was mine, they more frequently release patch version bumps.
Currently, https://omahaproxy.appspot.com shows the most recent version bump was only about a week apart:
- current_reldate: 09/30/21
- previous_reldate: 09/24/21
If we tracked the branch head for the stable channel (currently branch-heads/9.4) then that would make the changes even more frequent. Releasing fixes that frequently might be overkill, but automating the preparation work for getting a security fix out would be useful.
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.
We can start with every day and if it gets noisy (which it shouldn't if it's largely version checks and silent exiting if releases are more like once a week), we can reduce the frequency.
6d25003
to
0946f7a
Compare
0946f7a
to
7db1d65
Compare
deps/upgrade_v8.py
Outdated
# fetch latest v8 | ||
subprocess.run(["fetch", "v8"], | ||
cwd=v8_path, | ||
env=env) |
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.
The v8 source will already be checked out as a git submodule, so I'm not sure if this step is relevant anymore. Also, if it were used, it would be used from deps_path
, otherwise it would create another v8 checkout directory under v8_path
which we don't want.
For existing git checkouts, gclient sync
would be used to fetch the dependencies, as is done in deps/build.py. Actually, I think the only reason why we might want to do that here is to update the depot_tools commit that is being used. Otherwise, this script is just using the v8 git checkout (to copy the include headers).
@genevieve Thanks for 🟢, but I'm experimenting with my own fork and there are some things that need to be addressed to be able to merge into master. Give me a little more time and I will let you know when is a good time to merge it |
1d390ce
to
5e0efdb
Compare
@rogchap @dylanahsmith @genevieve I've done some explorations on my fork and this is the conclusion I was able to gether.
Since these changes I believe are out of the scope of this PR, I think we can merge this one to start getting the benefits of the |
I validated that PR created with the In the case the workflows runs for the latest version and it takes some time to make the necessary changes for the different distributions, the workflow will be trigger everyday but since the changes to the PR are identical the workflow will exit successfully. You can see an example on my fork actions https://github.com/GustavoCaso/v8go/actions/workflows/v8upgrade.yml |
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.
A couple of minor suggestions, otherwise, LGTM
README.md
Outdated
1) Optionally build the V8 binary for your OS: `cd deps && ./build.py`. V8 is a large project, and building the binary | ||
can take up to 30 minutes. Once built all the tests should still pass via `go test -v .` | ||
1) Commit your changes, making sure that the git submodules have been updated to the new checksum for the version of V8. | ||
Make sure *NOT* to add your build of the binary, as this will be build via CI. | ||
1) Because the build is so long, this is not automatically triggered. Go to the [V8 |
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.
1) Because the build is so long, this is not automatically triggered. Go to the [V8 | |
1) The build is not yet triggered automatically. To trigger it manually, go to the [V8 |
deps/upgrade_v8.py
Outdated
with open(os.path.join(src_path, V8_VERSION_FILE), "w") as v8_file: | ||
v8_file.write(version) | ||
|
||
def read_v8_file(src_path): |
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.
nitpick: I think mentioning that it is a version file would make the code clearer
def read_v8_file(src_path): | |
def read_v8_version_file(src_path): |
same for update_v8_file
749e3ab
to
256a7c7
Compare
Related to #165
This PR adds two new workflows: upgrade and automatic build v8 binaries for macOS and Linux.
The upgrade workflow is triggered every first day of the month, and the automatic v8 build gets trigger on every pull_request which branch follows thev8_<version>_upgrade
pattern.The upgrade workflow runs every day and check the current stable v8 version with the currently installed version on the repo.
For that, I created a
v8_version
file inside thedeps
folder and a python script to check the versions. If the versions are the same the script returns 1 stopping the workflow from continuing.This would allow us to have a more robust upgrade flow.
Until we figured out how to fully automate the Windows build the new automatic build v8 workflow only takes care of building the binaries for macOS and Linux. Example build that failed for Windows.@genevieve @rogchap @dylanahsmith Please me know what you think.