Skip to content

chore(v8bridge) minor fixes for v8bridge build #633

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

Merged
merged 2 commits into from
Dec 3, 2024

Conversation

hishamhm
Copy link
Contributor

@hishamhm hishamhm commented Dec 3, 2024

These need to be fixed in order to produce a bump to ngx_wasm_runtimes.

This puts us in an almost chicken-and-egg situation when bumping V8. We need to make a 3 (or rather 4) step process for bumping V8 if their build process changes:

  1. make the tweaks in ngx_wasm_module
  2. start an ARM VM and build the ARM binary image
  3. bump in ngx_wasm_runtimes
  4. bump the CI in ngx_wasm_module

This time I've done things out-of-order by tweaking the repo by hand in the ARM VM to get step 2 done first, but then I realized that step 3 won't work without step 1 (this PR). Another PR with step 4 will then follow.

(Perhaps the build part of util/runtimes/v8.sh should instead live in ngx_wasm_runtimes, so that all steps for building a new V8 artifact lived there, and then ngx_wasm_module could just treat ngx_wasm_runtimes as its V8 upstream, and in its own util/runtimes/v8.sh, if a build from source is desired, it could fetch that repo and run make on it.)

@hishamhm hishamhm changed the title chore(v8bridge) fixes for v8bridge build chore(v8bridge) minor fixes for v8bridge build Dec 3, 2024
Copy link

codecov bot commented Dec 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.86064%. Comparing base (d834bb0) to head (8151559).
Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@                 Coverage Diff                 @@
##                main        #633         +/-   ##
===================================================
+ Coverage   90.84288%   90.86064%   +0.01776%     
===================================================
  Files             52          52                 
  Lines          11259       11259                 
===================================================
+ Hits           10228       10230          +2     
+ Misses          1031        1029          -2     

see 1 file with indirect coverage changes

Flag Coverage Δ
unit 90.60539% <ø> (ø)
valgrind 82.53782% <ø> (+0.08783%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@hishamhm hishamhm requested a review from thibaultcha December 3, 2024 19:58
@thibaultcha thibaultcha merged commit cb7e69a into main Dec 3, 2024
32 checks passed
@thibaultcha thibaultcha deleted the chore/fix-v8bridge-build branch December 3, 2024 20:40
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.

2 participants