Skip to content

Conversation

@XL-Zhao-23
Copy link

Background

This PR fixes the compatibility issue reported in apache/skywalking#13517,
where the make license-dep command 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

  • Added precise cross-platform package pattern recognition in the npm resolver.
  • Implemented OS/architecture matching via regex (e.g., -linux-x64-musl$, -win32-arm64$).
  • Updated logic to skip unsupported or non-current platform packages before license parsing.

Verification

  • Tested with Node.js 24.6.0 and npm 11.5.1 on Linux and macOS.
  • All licenses are now correctly identified when running make license-dep in the banyandb project.

Related Issue

Fixes apache/skywalking#13517

@wu-sheng wu-sheng requested a review from Copilot November 12, 2025 06:29
@wu-sheng wu-sheng added the enhancement New feature or request label Nov 12, 2025
@wu-sheng wu-sheng requested a review from kezhenxu94 November 12, 2025 06:30
@wu-sheng wu-sheng added this to the 0.9.0 milestone Nov 12, 2025
Copilot finished reviewing on behalf of wu-sheng November 12, 2025 06:32
Copy link

Copilot AI left a 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 SkippedReason field 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.

@wu-sheng
Copy link
Member

Please fix CI and take a look at Copilot review comments.

@XL-Zhao-23
Copy link
Author

Sorry for the long delay. I've completed the revisions. Could you please help rerun the CI and review the code?

@XL-Zhao-23
Copy link
Author

I fixed the CI errors.

@wu-sheng
Copy link
Member

@kezhenxu94 Please take a look.

@hanahmily I think you could build a local version, and have a try?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Compatibility Issue with Node 24

2 participants