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.

Sign up for GitHub

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

Jump to bottom

Linked modules do not get added to the built-in modules #42302

Closed
LeonMrBonnie opened this issue Mar 11, 2022 · 2 comments · Fixed by #48522
Closed

Linked modules do not get added to the built-in modules #42302

LeonMrBonnie opened this issue Mar 11, 2022 · 2 comments · Fixed by #48522
Labels
build Issues and PRs related to build files or the CI. confirmed-bug Issues with confirmed bugs.

Comments

@LeonMrBonnie
Copy link

Version

v17.7.0

Platform

Windows

Subsystem

No response

What steps will reproduce the bug?

Compile NodeJS and add a linked module by e.g. using this command:

vcbuild.bat x64 dll link-module ./my-extra-module.js

How often does it reproduce? Is there a required condition?

No response

What is the expected behavior?

The module should be available as a built-in module when using the resulting binary.

What do you see instead?

The file is missing from the file generated by the js2c script, presumably by faulty refactoring done in the past. (It still worked with NodeJS v14)

Additional information

No response
@ritikBhandari
Copy link

Hi,
Can I have a try on this issue?

@Trott Trott added the windows Issues and PRs related to the Windows platform. label Mar 17, 2022
@richardlau richardlau added confirmed-bug Issues with confirmed bugs. build Issues and PRs related to build files or the CI. and removed windows Issues and PRs related to the Windows platform. labels Jun 22, 2023
@richardlau richardlau mentioned this issue Jun 22, 2023
Merged
@richardlau
Copy link
Member

This affects all platforms, not just Windows. It's an oversight in #39069.

I've opened #48522 to fix.

@richardlau richardlau added lts-watch-v16.x lts-watch-v18.x PRs that may need to be released in v18.x. and removed lts-watch-v16.x lts-watch-v18.x PRs that may need to be released in v18.x. labels Jun 22, 2023
nodejs-github-bot pushed a commit that referenced this issue Jun 24, 2023
@richardlau
build: fix configure --link-module
ae9f919 
Add the list of linked modules to the arguments for `js2c.py`. These
were unintentionally omitted when the build was previously refactored
to avoid command line length limits on Windows.

PR-URL: #48522
Fixes: #42302
Refs: #39069
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
RafaelGSS pushed a commit that referenced this issue Jul 3, 2023
@richardlau @RafaelGSS
build: fix configure --link-module
6551538 
Add the list of linked modules to the arguments for `js2c.py`. These
were unintentionally omitted when the build was previously refactored
to avoid command line length limits on Windows.

PR-URL: #48522
Fixes: #42302
Refs: #39069
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Ceres6 pushed a commit to Ceres6/node that referenced this issue Aug 14, 2023
@richardlau @Ceres6
build: fix configure --link-module
651f37a 
Add the list of linked modules to the arguments for `js2c.py`. These
were unintentionally omitted when the build was previously refactored
to avoid command line length limits on Windows.

PR-URL: nodejs#48522
Fixes: nodejs#42302
Refs: nodejs#39069
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Ceres6 pushed a commit to Ceres6/node that referenced this issue Aug 14, 2023
@richardlau @Ceres6
build: fix configure --link-module
b5d6683 
Add the list of linked modules to the arguments for `js2c.py`. These
were unintentionally omitted when the build was previously refactored
to avoid command line length limits on Windows.

PR-URL: nodejs#48522
Fixes: nodejs#42302
Refs: nodejs#39069
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
targos pushed a commit that referenced this issue Oct 28, 2023
@richardlau @targos
build: fix configure --link-module
f2eccb7 
Add the list of linked modules to the arguments for `js2c.py`. These
were unintentionally omitted when the build was previously refactored
to avoid command line length limits on Windows.

PR-URL: #48522
Fixes: #42302
Refs: #39069
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
sercher added a commit to sercher/graaljs that referenced this issue Apr 25, 2024
@sercher
build: fix configure --link-module
bca04ed 
Add the list of linked modules to the arguments for `js2c.py`. These
were unintentionally omitted when the build was previously refactored
to avoid command line length limits on Windows.

PR-URL: nodejs/node#48522
Fixes: nodejs/node#42302
Refs: nodejs/node#39069
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
sercher added a commit to sercher/graaljs that referenced this issue Apr 25, 2024
@sercher
build: fix configure --link-module
37d4ec7 
Add the list of linked modules to the arguments for `js2c.py`. These
were unintentionally omitted when the build was previously refactored
to avoid command line length limits on Windows.

PR-URL: nodejs/node#48522
Fixes: nodejs/node#42302
Refs: nodejs/node#39069
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
build Issues and PRs related to build files or the CI. confirmed-bug Issues with confirmed bugs.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

build: fix configure --link-module richardlau/node-1
4 participants
@Trott @richardlau @LeonMrBonnie @ritikBhandari