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

build: ./node built with --node-builtin-modules-path runs into an error recently #36512

Closed
RaisinTen opened this issue Dec 14, 2020 · 5 comments
Labels
build Issues and PRs related to build files or the CI.

Comments

@RaisinTen
Copy link
Contributor

RaisinTen commented Dec 14, 2020

  • Version: v16.0.0-pre
  • Platform: Linux hp 4.15.0-128-generic lib: reintroduce v8 module #131-Ubuntu SMP Wed Dec 9 06:53:22 UTC 2020 i686 i686 i686 GNU/Linux
  • Subsystem: build

What steps will reproduce the bug?

Build node with the --node-builtin-modules-path option and run ./node.

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

I had built ./node a while back and the problem started after syncing my local copy of the repo with the main branch 3 days back.

What is the expected behavior?

No errors.

What do you see instead?

$ ./node
node:internal/bootstrap/loaders:311
  if (!mod) throw new TypeError(`Missing internal module '${id}'`);
            ^

TypeError: Missing internal module 'internal/streams/add-abort-signal'
    at nativeModuleRequire (node:internal/bootstrap/loaders:311:19)
    at node:internal/streams/readable:46:5
    at NativeModule.compileForInternalLoader (node:internal/bootstrap/loaders:283:7)
    at nativeModuleRequire (node:internal/bootstrap/loaders:312:14)
    at node:stream:40:19
    at NativeModule.compileForInternalLoader (node:internal/bootstrap/loaders:283:7)
    at nativeModuleRequire (node:internal/bootstrap/loaders:312:14)
    at node:internal/worker/io:39:32
    at NativeModule.compileForInternalLoader (node:internal/bootstrap/loaders:283:7)
    at nativeModuleRequire (node:internal/bootstrap/loaders:312:14)

Additional information

To my knowledge, the generated executable is flexible enough to adapt to any js changes without another rebuild. However, ./node seems to crash with the same error message no matter which commit I check out.

@targos
Copy link
Member

targos commented Dec 14, 2020

We cannot guarantee that a binary built previously with --node-builtin-modules-path is always going to work. There are many possible changes that require it to be rebuilt.

@joyeecheung
Copy link
Member

joyeecheung commented Dec 14, 2020

Your local JS file expects internal/streams/add-abort-signal to be present, so when you switch to a copy that changes enough so that your binary is not yet built with knowing internal/streams/add-abort-signal is a module, there would be an error. This option only works when the JS file listed in library_files in node.gyp is untouched when you switch between different versions of Node.js.

@RaisinTen
Copy link
Contributor Author

Is there any way we can make --node-builtin-modules-path to build node so that this works irrespective of library_files by loading the files completely at runtime?

@RaisinTen RaisinTen added the build Issues and PRs related to build files or the CI. label Dec 15, 2020
@devsnek
Copy link
Member

devsnek commented Dec 15, 2020

no, it specifically only patches the part of the codebase where the source of the module is looked up. using it should not modify node's module system logic which is already quite complex and prone to bugs.

@Trott
Copy link
Member

Trott commented Dec 22, 2020

I believe this can be closed, but if there's more to talk about, feel free to comment and/or re-open.

@Trott Trott closed this as completed Dec 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI.
Projects
None yet
Development

No branches or pull requests

5 participants