skywalking icon indicating copy to clipboard operation
skywalking copied to clipboard

[Bug] Compatibility Issue with Node 24

Open hanahmily opened this issue 3 months ago • 13 comments

Search before asking

  • [x] I had searched in the issues and found no similar issues.

Apache SkyWalking Component

License Tools (apache/skywalking-eyes)

What happened

I encountered a significant issue while trying to run the make license-dep command in the banyandb project with Node.js version 24. It appears that the Skywalking eye does not function correctly with this version, resulting in license identification failures.

Observed Outcome:

the failure to identify licenses for the following packages:

  • @parcel/watcher-android-arm64
  • @parcel/watcher-darwin-x64
  • @parcel/watcher-freebsd-x64
  • @parcel/watcher-linux-arm-glibc
  • @parcel/watcher-linux-arm-musl
  • @parcel/watcher-linux-arm64-glibc
  • @parcel/watcher-linux-arm64-musl
  • @parcel/watcher-linux-x64-glibc
  • @parcel/watcher-linux-x64-musl
  • @parcel/watcher-win32-arm64
  • @parcel/watcher-win32-ia32
  • @parcel/watcher-win32-x64

Make Output:

make[1]: *** [license-dep] Error 1
make: *** [default] Error 1

Environment:

  • Node.js version: 24.6.0
  • npm version: 11.5.1

What you expected to happen

The command should execute successfully without any warnings or errors related to unsupported Node versions and should identify all licenses correctly.

How to reproduce

  1. Use Node.js version 24.6.0.
  2. Navigate to the banyandb project directory.
  3. Run the command:
    make license-dep
    
  4. Observe the output, particularly the warnings and the subsequent errors.

Anything else

No response

Are you willing to submit a pull request to fix on your own?

  • [ ] Yes I am willing to submit a pull request on my own!

Code of Conduct

hanahmily avatar Sep 25 '25 05:09 hanahmily

I’m interested in this issue. May I ask whether it is currently being fixed or has already been resolved?

XL-Zhao-23 avatar Oct 20 '25 08:10 XL-Zhao-23

No, we are not working on this. Please go ahead.

wu-sheng avatar Oct 20 '25 08:10 wu-sheng

@hanahmily May I ask, what is the BanyanDB that you are using?

XL-Zhao-23 avatar Oct 20 '25 09:10 XL-Zhao-23

Here is the repo. https://github.com/apache/skywalking-banyandb

wu-sheng avatar Oct 20 '25 09:10 wu-sheng

Thank you for the information! I’ll take a look at the repository and start working on this issue.

XL-Zhao-23 avatar Oct 20 '25 09:10 XL-Zhao-23

Problem Analysis

In the Node.js 24 environment, npm ls --all --production --parseable outputs package paths for all platforms, including both the current platform and other platforms. However, in the actual filesystem, only packages for the current platform exist. This causes license verification to attempt to resolve non-existent cross-platform package paths.

Proposed Solutions

Solution 1: Intelligent Platform Filtering

Core Concept

Intelligently distinguish and retain only cross-platform packages required for the current platform by detecting platform identifiers in package paths.

Implementation Logic

  1. Cross-platform package identification: Detect known cross-platform package patterns in paths (e.g., @parcel/watcher-, @node-rs/, etc.)
  2. Platform matching verification: For identified cross-platform packages, further check if they contain current platform identifiers
  3. Selective retention:
    • If it's a package for the current platform (e.g., @parcel/watcher-linux-x64-glibc on Linux) → Keep
    • If it's a package for other platforms (e.g., @parcel/watcher-darwin-arm64 on Linux) → Skip

Technical Details

  • Use runtime.GOOS and runtime.GOARCH to get current platform information
  • Establish platform identifier mapping table (e.g., linux-x64, darwin-arm64, win32-x64, etc.)
  • Perform pattern matching on common cross-platform package prefixes

Advantages

  • Clear logic, easy to understand and maintain
  • Comprehensive coverage, handles various cross-platform package patterns
  • Good performance, only requires string matching

Potential Challenges

  • Need to maintain a pattern library for cross-platform packages
  • May miss some unrecognized cross-platform package patterns

Solution 2: Path Pattern Analysis

Core Concept

Analyze path strings from npm ls output and use path pattern logic to determine whether it's an actually installed package.

Implementation Logic

  1. Path pattern recognition: Analyze platform identifier patterns in path strings
  2. Logical judgment:
    • If it's a cross-platform package path matching current platform → Keep
    • If it's a cross-platform package path not matching current platform → Skip
    • If it's a regular package path → Keep

Technical Details

  • Pure string operations, no filesystem access required
  • Pattern matching based on path naming conventions
  • Utilize complete path information already contained in npm output

Advantages

  • High accuracy, completely based on path pattern logic
  • Excellent performance, pure string operations
  • No need to maintain complex pattern libraries
  • Strong adaptability, handles various path patterns

Potential Challenges

  • Relies on standardization of npm output path naming
  • Needs to correctly handle various path formats

Solution Comparison

Aspect Solution 1 (Intelligent Filtering) Solution 2 (Path Analysis)
Accuracy High (requires pattern library) Highest (based on path logic)
Performance High (string matching) High (pure string operations)
Maintenance Cost Medium (needs pattern updates) Low (based on path logic)
Adaptability Medium (relies on preset patterns) High (adaptive to path patterns)
Implementation Complexity Low Low

Recommendation

Based on the above analysis, I would tentatively recommend:

Preferred Approach: Solution 2 (Path Pattern Analysis)

  • Performance comparable to Solution 1, both using pure string operations
  • Higher accuracy, completely based on path logic judgment
  • Lower maintenance cost, no need to maintain complex pattern libraries
  • Better adaptability, can handle various unknown path patterns

Implementation Suggestions

  1. Core logic: Add path analysis filtering in the GetInstalledPkgs method
  2. Platform detection: Use runtime.GOOS and runtime.GOARCH to identify current platform
  3. Path matching: Pattern matching based on cross-platform package path naming conventions
  4. Logging: Add debug logs for skipped package paths to facilitate troubleshooting

Opening for Discussion

I would like to emphasize that these are just initial thoughts to spark discussion. My experience in this specific area is limited, and I'm very eager to hear better suggestions or alternative approaches from the community.

Some questions I have:

  • Are there other cross-platform package patterns we should consider?
  • Would a hybrid approach combining both solutions be more robust?
  • Are there any edge cases or platform-specific nuances I might have overlooked?
  • Has anyone encountered similar issues and found different solutions?

Thank you for considering these suggestions - I'm looking forward to learning from your expertise and collaborating on the best approach to solve this issue!

XL-Zhao-23 avatar Oct 21 '25 14:10 XL-Zhao-23

Hi, thanks for the summary. It is clear option2 is better as less maintenance. The key question is around whether there is(are) clear patterns in x-platform package names? Although it could be conventions, but are they clear and consistent? If they are not, we have to consider implementing some regular patterns, and provide pattern customization for specific other lib list.

wu-sheng avatar Oct 21 '25 14:10 wu-sheng

Currently, npm does not enforce strict constraints on package naming. Most packages follow common conventions, so there is a general level of consistency, but some exceptions and irregular naming patterns still exist.

To address this, we plan to implement several regular pattern checks and allow pattern customization for specific library lists. As an initial step, I will focus on resolving the issue observed in the BanyanDB scenario.

In addition, according to the official npm documentation, the npm ls command is described as:

“This command will print to stdout all the versions of packages that are installed.”

However, our actual observation shows that npm ls outputs include cross-platform packages, which seems inconsistent with the documentation. I’ve submitted an issue to npm for clarification on this behavior.

XL-Zhao-23 avatar Oct 23 '25 12:10 XL-Zhao-23

Let's see. Those x-platform thing relies on whether platform is native concept of npm, or version name hijacking. Java pom uses name hijacking today.

wu-sheng avatar Oct 23 '25 15:10 wu-sheng

Option 2 is good enough for me. Our consideration is to identify if the package is os-related ONLY when the package files cannot be found on disk. It should be a common sense to only add os-related prefix/suffix in the names, I don't think there are packages that use name foo for os one and bar for os two. Let's just use option 2 and make it extensible to add regex patterns for identifying os'es

kezhenxu94 avatar Oct 24 '25 09:10 kezhenxu94

The concept of version name hijacking is new to me, so I tried to understand it in the context of npm. It seems that npm actually combines both approaches: it has a native concept of platform support while also using a form of name-based platform differentiation.

In npm’s package.json, each subpackage can specify os and cpu fields, which is the native way npm recognizes platform constraints. At the same time, each platform-specific implementation is published as a separate package, for example:

"node_modules/@parcel/watcher": {
  "version": "2.5.1",
  "optionalDependencies": {
    "@parcel/watcher-android-arm64": "2.5.1",
    "@parcel/watcher-darwin-arm64": "2.5.1",
    "@parcel/watcher-darwin-x64": "2.5.1",
    "@parcel/watcher-linux-x64-glibc": "2.5.1",
    "@parcel/watcher-win32-x64": "2.5.1"
  }
}

The main package @parcel/watcher acts as an abstract package, which references the platform-specific packages by name. Each of these subpackages also defines the os and cpu fields, for example:

"node_modules/@parcel/watcher-android-arm64": {
  "version": "2.5.1",
  "os": ["android"],
  "cpu": ["arm64"],
  "optional": true
}

XL-Zhao-23 avatar Oct 24 '25 14:10 XL-Zhao-23

@kezhenxu94 +1 for Option 2. The approach is practical and extensible enough for our use case.

XL-Zhao-23 avatar Oct 24 '25 14:10 XL-Zhao-23

The issue has been fixed in apache/skywalking-eyes#252.

XL-Zhao-23 avatar Nov 12 '25 06:11 XL-Zhao-23