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

[Improvements🛠️]:Update tools directory, CI workflow, and dependencies for better compatibility and performance #1790

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

wind111-lang
Copy link

@wind111-lang wind111-lang commented May 20, 2024

Changed tools/semver-check.js

Node.js 10 was end-of-life on December 31, 2021.
so using process.versions.node to always use the current Node.js version.
This change solves fix GitHub Actions test failures caused by specifying Node.js '10.12.0'. node-sqlite3 testing with Node.js 18, so '10.12.0' isn’t need.

Changed tools/BinaryBuilder.Dockerfile

  • Fixed network connectivity issue during tests on bullseye with Node.js 18.
    Added --maxsockets=1 to npm install as a temporary workaround.
    This fixes the error: Client network socket disconnected before secure TLS connection was established in bullseye CI test.
    • I would like to propose updating our test environment from bullseye to Bookworm. This change aims to leverage the latest software versions, security patches, and performance improvements offered by Bookworm.

Changed workflow CI

  • Improved Apple Silicon compatibility by specifying arm64 architecture for macos-latest.
    • Is macos-m1 test working in self-host-runner? If this change has problem, I undo macos-latest in arm64.
  • Updated setup-msbuild from v1 to v2 for better Windows11 compatibility.
  • Removed setuptools installation step because node-gyp v10 supports Python 3.12 by default.
  • Keep current version of upload-artifact due to test failures with the latest version.

Upgrade some dependencies

These updates improve compatibility, performance, and security of your project.

@wind111-lang wind111-lang reopened this May 20, 2024
@wind111-lang wind111-lang changed the title [Improvements🛠️]:Update tools, Dockerfile, CI workflow, and dependencies for better compatibility and performance [Improvements🛠️]:Update tools directory, CI workflow, and dependencies for better compatibility and performance May 20, 2024
@YasharF
Copy link

YasharF commented May 26, 2024

Hey there, I am curious, what kind of tests have run to verify the version bump is not creating other issues, regressions, etc. ?

@wind111-lang
Copy link
Author

wind111-lang commented May 27, 2024

Hello, @YasharF.
Thank you for your comment.

I have run the workflow in this PR and passed all environments with the changes in semver-check.js and BinaryBuilder.Dockerfile. Since I'm running the workflow on a fork repository, I'm not sure what the results will be on master repository Actions.

The issue was that the version checked in semver-check.js was specified as Node 10, causing the tests to fail. Additionally, on Debian bullseye, the tests passed after temporarily setting maxsockets=1 to fix the Client network socket disconnected before secure TLS connection was established error.

After making these changes, I ran the workflow with the updated versions from the Renovate PR, and all environments passed successfully.

Thank you.

@netroy
Copy link

netroy commented Oct 10, 2024

I ran the same test suite with this branch as I did in #1788, and everything worked as expected to me.

I get it if there are hesitations to merge these PRs, but it'd be nice to have a clarity on intent here, so that people like me can decide if we should wait for these PRs to be merged, or if we create a "temporary" fork to publish on NPM under a scope.

@daniellockyer can you please shed some light on the maintenance status of this project 🙏🏽

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.

3 participants