-
Notifications
You must be signed in to change notification settings - Fork 63
Fix: Compatibility issue with Node.js 24 (related to apache/skywalking#13517) #252
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
base: main
Are you sure you want to change the base?
Fix: Compatibility issue with Node.js 24 (related to apache/skywalking#13517) #252
Conversation
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.
Pull Request Overview
This PR fixes a compatibility issue with Node.js 24 where the make license-dep command fails due to incorrect handling of platform-specific npm packages. The solution adds intelligent platform detection to skip packages that are not compatible with the current OS/architecture before attempting license parsing.
Key Changes:
- Added platform-specific package detection using regex patterns for various OS/architecture combinations (Linux, macOS, Windows, FreeBSD, Android)
- Implemented architecture compatibility logic to handle variations like x64/amd64/x86_64
- Added
SkippedReasonfield to track why packages are skipped
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| pkg/deps/result.go | Added SkippedReason field to Result struct to track why packages are skipped during resolution |
| pkg/deps/npm.go | Core implementation: added platform pattern recognition (39 patterns), platform detection logic in isForCurrentPlatform, architecture compatibility checking in isArchCompatible, and updated resolution flow to skip non-matching platform packages |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Please fix CI and take a look at Copilot review comments. |
|
Sorry for the long delay. I've completed the revisions. Could you please help rerun the CI and review the code? |
|
I fixed the CI errors. |
|
@kezhenxu94 Please take a look. @hanahmily I think you could build a local version, and have a try? |
Background
This PR fixes the compatibility issue reported in apache/skywalking#13517,
where the
make license-depcommand fails under Node.js 24 due to incorrect handling of platform-specific npm packages.Root Cause
The SkyWalking Eyes tool failed to correctly recognize platform-specific npm packages
(e.g.,
@parcel/watcher-linux-x64-musl,@parcel/watcher-win32-arm64),resulting in license parsing errors under newer Node.js versions.
Solution
-linux-x64-musl$,-win32-arm64$).Verification
make license-depin thebanyandbproject.Related Issue
Fixes apache/skywalking#13517