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

process.versions does not list all bundled dependencies #45260

Closed
6 tasks
jasnell opened this issue Oct 31, 2022 · 14 comments
Closed
6 tasks

process.versions does not list all bundled dependencies #45260

jasnell opened this issue Oct 31, 2022 · 14 comments
Labels
good first issue Issues that are suitable for first-time contributors. process Issues and PRs related to the process subsystem.

Comments

@jasnell
Copy link
Member

jasnell commented Oct 31, 2022

The process. Versions API lists the versions of vendored dependencies that are bundled into Node.js. Unfortunately, it only shows a subset. Notably missing from the list are:

  • deps/acorn
  • deps/base64
  • deps/cjs-module-lexer
  • deps/histogram
  • deps/undici
  • deps/uvwasi
@VoltrexKeyva VoltrexKeyva added the process Issues and PRs related to the process subsystem. label Oct 31, 2022
@Trott Trott added the good first issue Issues that are suitable for first-time contributors. label Nov 1, 2022
@anonrig
Copy link
Member

anonrig commented Nov 1, 2022

For contributors looking for where to start; the versions are included in src/node_metadata.cc

@Xstoudi
Copy link
Contributor

Xstoudi commented Nov 1, 2022

Is this possible to give indications about how to get version of full-js libs (like acorn) or libs that doesn't expose their version number (like base64)?

@targos
Copy link
Member

targos commented Nov 2, 2022

For acorn and undici, the package.json is in the source tree and contains the version number. I'll let someone else suggest a good way to expose it to our C++ code.

@Xstoudi
Copy link
Contributor

Xstoudi commented Nov 2, 2022

The other concern is the policy regarding update of deps.
For example right now base64 is pointing to this commit:
aklomp/base64@dc6a41c
and it implies that the CMakeList shows 0.4.0 while the the next commit (aklomp/base64@9ae5ad3) is only bumping the version to 0.5.0. That means that node.js is shipping a 0.5.0 version that hides under 0.4.0.

@nitesh-github143

This comment was marked as off-topic.

@Trott

This comment was marked as resolved.

@debadree25
Copy link
Member

Would parsing the package.json for all js libraries and then including them be a good idea? put quite some thought into it this is the only one that came to my mind from my limited knowledge, would love to know about any other possible way of including this in cpp code

kivancbilen added a commit to kivancbilen/node that referenced this issue Nov 23, 2022
process.versions is not returning undici version,
this PR will read the version number from undici/src/package.json
and add it to result. If this approach is ok then other missing libraries
will be added

Fixes:nodejs#45260
kivancbilen added a commit to kivancbilen/node that referenced this issue Nov 23, 2022
process.versions is not returning undici version,
this PR will read the version number from undici/src/package.json
and add it to result. If this approach is ok then other missing libraries
will be added

Refs:nodejs#45260
@bnoordhuis
Copy link
Member

Let me push back on this a little. I don't think it's necessary to list version numbers for "static" dependencies (things that are always built into the binary and never linked dynamically) because those are:

a) fixed, and

b) implementation details that aren't relevant most of the time

When the configure script grows e.g. a --shared-uvwasi option, then it's reasonable to add a process.version.uvwasi key for debugging purposes.

Likewise, a hypothetical --disable-histogram should be reflected.

But things that are hidden and immutable? No point in listing them.

@jasnell
Copy link
Member Author

jasnell commented Nov 24, 2022

From a functional perspective, perhaps. However, when producing something like an SBOM, it's important detail that should be included.

@bnoordhuis
Copy link
Member

Not sure I follow. Can you give an example when that's relevant? SBOM = Source Bill Of Materials?

@jasnell
Copy link
Member Author

jasnell commented Nov 24, 2022

SBOM == Software Bill of Materials. See background info here: https://www.ntia.gov/SBOM

To be certain, the process.versions by itself does not provide all of the information necessary for this purpose but it helps.

Also to be clear, I'm not suggesting that this is the only reason we should provide this additional detail.

debadree25 added a commit to debadree25/node that referenced this issue Nov 25, 2022
Attempted to update undici version when the update scripts are run
Fixes: nodejs#45260
Refs: nodejs#45599
@MrJithil
Copy link
Member

Added version info of cjs_module_lexer and base64

@MrJithil
Copy link
Member

MrJithil commented Nov 27, 2022

Pending Version Additions

@bnoordhuis
Copy link
Member

I've looked through the linked pull requests but the version number parsing all feels kind of fragile and ad hoc. I again question the wisdom of doing this.

nodejs-github-bot pushed a commit that referenced this issue Dec 11, 2022
PR-URL: #45639
Refs: #45260
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
debadree25 added a commit to debadree25/node that referenced this issue Dec 12, 2022
Attempted to update undici version when the update scripts are run
Fixes: nodejs#45260
Refs: nodejs#45599
ErickWendel pushed a commit to ErickWendel/node that referenced this issue Dec 12, 2022
PR-URL: nodejs#45639
Refs: nodejs#45260
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
targos pushed a commit that referenced this issue Dec 12, 2022
PR-URL: #45639
Refs: #45260
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
targos pushed a commit that referenced this issue Dec 13, 2022
PR-URL: #45639
Refs: #45260
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@aduh95 aduh95 closed this as completed in 9d1d948 Dec 18, 2022
danielleadams pushed a commit that referenced this issue Dec 30, 2022
PR-URL: #45639
Refs: #45260
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
danielleadams pushed a commit that referenced this issue Dec 30, 2022
PR-URL: #45639
Refs: #45260
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
targos pushed a commit that referenced this issue Jan 1, 2023
Fixes: #45260
Refs: #45599
Refs: #45260
PR-URL: #45621
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
danielleadams pushed a commit that referenced this issue Jan 3, 2023
PR-URL: #45639
Refs: #45260
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
danielleadams pushed a commit that referenced this issue Jan 4, 2023
PR-URL: #45639
Refs: #45260
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
danielleadams pushed a commit that referenced this issue Jan 5, 2023
PR-URL: #45639
Refs: #45260
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
RafaelGSS pushed a commit that referenced this issue Jan 5, 2023
Fixes: #45260
Refs: #45599
Refs: #45260
PR-URL: #45621
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
juanarbol pushed a commit that referenced this issue Jan 26, 2023
Fixes: #45260
Refs: #45599
Refs: #45260
PR-URL: #45621
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
nodejs-github-bot pushed a commit that referenced this issue May 14, 2023
src: add cjs_module_lexer_version base64_version
PR-URL: #45629
Refs: #45260
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
targos pushed a commit that referenced this issue May 14, 2023
src: add cjs_module_lexer_version base64_version
PR-URL: #45629
Refs: #45260
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
targos pushed a commit that referenced this issue May 15, 2023
src: add cjs_module_lexer_version base64_version
PR-URL: #45629
Refs: #45260
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
targos pushed a commit that referenced this issue Nov 10, 2023
src: add cjs_module_lexer_version base64_version
PR-URL: #45629
Refs: #45260
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
sercher added a commit to sercher/graaljs that referenced this issue Apr 25, 2024
src: add cjs_module_lexer_version base64_version
PR-URL: nodejs/node#45629
Refs: nodejs/node#45260
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
sercher added a commit to sercher/graaljs that referenced this issue Apr 25, 2024
src: add cjs_module_lexer_version base64_version
PR-URL: nodejs/node#45629
Refs: nodejs/node#45260
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Issues that are suitable for first-time contributors. process Issues and PRs related to the process subsystem.
Projects
None yet
Development

No branches or pull requests

10 participants