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

upgraded nodejs version to 20.18.0 #58

Merged
merged 1 commit into from
Nov 25, 2024

Conversation

avcribl
Copy link
Contributor

@avcribl avcribl commented Oct 9, 2024

  • Added patches for nodejs 20.18.0
  • Updated azure-pipelines.yml
  • Updated Dockerfile.centos7 and Dockerfile.centos7.arm64 to use the devtoolset version 10
  • Added version for js2bin-builder image

Linux x64 and Windows artifacts have been successfully built in the CI pipeline.

@avcribl avcribl force-pushed the feat-bump-20-18-0 branch from ac9d4dc to ee1e52f Compare October 9, 2024 20:18
@@ -255,7 +256,7 @@ class NodeJsBuilder {
// 4. process mainAppFile (gzip, base64 encode it) - could be a placeholder file
// 5. kick off ./configure & build
buildFromSource(uploadBuild, cache, container, arch, ptrCompression) {
const makeArgs = isWindows ? ['x64', 'noetw', 'no-cctest'] : [`-j${os.cpus().length}`];
const makeArgs = isWindows ? ['x64', 'no-cctest'] : [`-j${os.cpus().length}`];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Support for etw (Event Tracing for Windows) has been removed since nodejs v19.0.0 onward. nodejs/node#43652
It's mentioned in the nodejs v19 changelog as well.

@avcribl avcribl force-pushed the feat-bump-20-18-0 branch 2 times, most recently from 9645bde to bb13d37 Compare October 23, 2024 23:08
@ledbit
Copy link
Collaborator

ledbit commented Nov 12, 2024

@avcribl which of the patch files are new and which ones are copy/paste from prior node versions?

@avcribl
Copy link
Contributor Author

avcribl commented Nov 12, 2024

@avcribl which of the patch files are new and which ones are copy/paste from prior node versions?

@ledbit Everything is copied from the last upgrade except configure.py.patch

o['variables']['v8_enable_pointer_compression'] = 1 if options.enable_pointer_compression else 0
- o['variables']['v8_enable_sandbox'] = 1 if options.enable_pointer_compression else 0
+
+ # o['variables']['v8_enable_sandbox'] = 1 if options.enable_pointer_compression else 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do we need to disable this option?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There has been a commit that was introduced in nodejs 20.17.0 that enforces enablingv8_enable_sandbox flag whenever v8_enable_pointer_compression flag is enabled. This causes failure of building nodejs for Windows OS when we enable the pointer compression flag. Apparently, such a thing does not work well in the Windows OS. I also did not find any build with the pointer compression flag enabled to run and test for Windows OS on the nodejs CI pipeline. The build failure message can be seen in CRIBL-28701 ticket.

@ledbit ledbit merged commit f3b3883 into criblio:master Nov 25, 2024
4 of 6 checks passed
@ledbit ledbit mentioned this pull request Nov 28, 2024
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