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

--preserve-symlinks-main doesn't work #41000

Closed
pauldraper opened this issue Nov 28, 2021 · 3 comments · Fixed by #51312
Closed

--preserve-symlinks-main doesn't work #41000

pauldraper opened this issue Nov 28, 2021 · 3 comments · Fixed by #51312
Labels
good first issue Issues that are suitable for first-time contributors.

Comments

@pauldraper
Copy link

pauldraper commented Nov 28, 2021

Version

v17.0.1

Platform

Linux paul 5.4.0-90-generic #101-Ubuntu SMP Fri Oct 15 20:00:55 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

No response

What steps will reproduce the bug?

Create a package with a file and a symlink to a file in a different directory.

echo 'require("./lib.js")' > file.js
mkdir package
> package/lib.js
ln -rs file.js package/index.js

Run Node.js with the package as the entrypoint.

node --preserve-symlinks --preserve-symlinks-main ./package

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

The feature --preserve-symlinks-main works with some file structures and doesn't work with others (like the example). I haven't yet figured out the general rule.

What is the expected behavior?

Node.js runs without error.

Namely, the entry point /home/paul/example/package/index.js resolves ./lib.js to /home/paul/example/package/lib.js

What do you see instead?

Node.js errs:

node:internal/modules/cjs/loader:936
  throw err;
  ^

Error: Cannot find module './lib.js'
Require stack:
- /home/paul/example/file.js
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:933:15)
    at Function.Module._load (node:internal/modules/cjs/loader:778:27)
    at Module.require (node:internal/modules/cjs/loader:999:19)
    at require (node:internal/modules/cjs/helpers:102:18)
    at Object.<anonymous> (/home/paul/dev/example/file.js:1:1)
    at Module._compile (node:internal/modules/cjs/loader:1095:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1147:10)
    at Module.load (node:internal/modules/cjs/loader:975:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [ '/home/paul/example/file.js' ]
}

Notice that Node.js has resolved /home/paul/example/package/index.js to its realpath (/home/paul/example/file.js), causing module resolution to fail.

Additional information

The validity of the example can be confirmed by replacing the symlink with a hardlink (ln file.js package/index.js), after which module resolution works.

@marco-ippolito marco-ippolito added confirmed-bug Issues with confirmed bugs. doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors. and removed confirmed-bug Issues with confirmed bugs. doc Issues and PRs related to the documentations. labels Dec 16, 2023
@dgoldstein0
Copy link
Contributor

hm, the one thing that comes to mind is that I've only ever used --preserve-symlinks-main with full file paths. Does it repro with

node --preserve-symlinks --preserve-symlinks-main ./package/index.js

Also it was implemented before ESM was supported by default in node, so could be a commonjs vs ESM module loading issue.

per4uk added a commit to per4uk/node that referenced this issue Dec 29, 2023
Fixed resolving relative imports when symlink to main file called without file extension

Fixies: nodejs#41000
@mrgrain
Copy link

mrgrain commented Jan 3, 2024

I can easily reproduce this with npm:

$ node --preserve-symlinks --preserve-symlinks-main ./path/to/global/installs/node_modules/bin/npm --version

node:internal/modules/cjs/loader:1078
  throw err;
  ^

Error: Cannot find module '../lib/cli.js'

mrgrain added a commit to aws/jsii that referenced this issue Jan 3, 2024
This was previously attempted to fix in #4324
While the above fix resolves issues with dependencies, it causes failures when the binary is shelling out to other node processes. This is due to the intrusive and indiscriminate overloading of NODE_OPTIONS, which will forcibly apply to any child processes as well. While in theory adding the symlink flags should not be an issue, this seems to trigger a bug in node: nodejs/node#41000

tl;dr this all sucks very much and we are now just disabling the runtime cache for binaries.
mergify bot pushed a commit to aws/jsii that referenced this issue Jan 3, 2024
This was previously attempted to fix in #4324 While the above fix resolves issues with dependencies, it causes failures when the binary is shelling out to other node processes. This is due to the intrusive and indiscriminate overloading of NODE_OPTIONS, which will forcibly apply to any child processes as well. While in theory adding the symlink flags should not be an issue, this seems to trigger a bug in node: nodejs/node#41000

tl;dr this all sucks very much and we are now just disabling the runtime cache for binaries.

---

By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
@dgoldstein0
Copy link
Contributor

not sure I entirely understand your npm example but in general I would not expect --preserve-symlinks-main to be usable with a node_modules/.bin/thing main argument - thing there would be a symlink into an entirely different folder, probably node_modules/thing/cli.js or similar, and then require()s or imports in cli.js should be resolved relative to node_modules/thing/cli.js, not the node_modules/.bin/thing. So I don't think your npm example is a valid reproduction.

aduh95 pushed a commit that referenced this issue Jan 10, 2024
Fixes resolving main module when the `argv[1]` was pointing to a symlink
without its file extension.

PR-URL: #51312
Fixes: #41000
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
marco-ippolito pushed a commit to marco-ippolito/node that referenced this issue Jan 12, 2024
Fixes resolving main module when the `argv[1]` was pointing to a symlink
without its file extension.

PR-URL: nodejs#51312
Fixes: nodejs#41000
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Medhansh404 pushed a commit to Medhansh404/node that referenced this issue Jan 19, 2024
Fixes resolving main module when the `argv[1]` was pointing to a symlink
without its file extension.

PR-URL: nodejs#51312
Fixes: nodejs#41000
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
targos pushed a commit that referenced this issue Feb 15, 2024
Fixes resolving main module when the `argv[1]` was pointing to a symlink
without its file extension.

PR-URL: #51312
Fixes: #41000
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
richardlau pushed a commit that referenced this issue Mar 25, 2024
Fixes resolving main module when the `argv[1]` was pointing to a symlink
without its file extension.

PR-URL: #51312
Fixes: #41000
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
richardlau pushed a commit that referenced this issue Mar 25, 2024
Fixes resolving main module when the `argv[1]` was pointing to a symlink
without its file extension.

PR-URL: #51312
Fixes: #41000
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.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants