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

FileSystemInfo can not createSnapshot with symbol link #12246

Closed
killagu opened this issue Dec 21, 2020 · 14 comments · Fixed by #14019
Closed

FileSystemInfo can not createSnapshot with symbol link #12246

killagu opened this issue Dec 21, 2020 · 14 comments · Fixed by #14019

Comments

@killagu
Copy link

killagu commented Dec 21, 2020

Bug report

What is the current behavior?
I install deps with npminstall(it can install faster), but the file tree in node_modules is special bacause npminstall install pkgs with symbol link to download and write less files.

The file tree looks like this

app
├── index.js
└── node_modules
    ├── _a@1.0@a(real dir)
    |   ├── index.js
    |   └── node_modules
    |       └── b(symbol link to app/node_modules/_b@1.0@b)
    ├── _b@1.0@b(real dir)
    |   ├── index.js
    |   └── node_modules
    |       └── a(symbol link to app/node_modules/_a@1.0@a)
    ├── a(symbol link to app/node_modules/_a@1.0@a)
    └── b(symbol link to app/node_modules/_b@1.0@b)

Because of a,b is reference by each other, contextTimestampQueue and contextHashQueue in FileSystemInfo will never reach the end.

If the current behavior is a bug, please provide the steps to reproduce.

The minimal reproduction repo is here.
The reproduce step is

git clone git@github.com:killagu/npminstall_webpack.git
cd npminstall_webpack
npm i -g cnpm
# must use cnpm install
cnpm i
npm run build

Build snapshow will fail.

What is the expected behavior?

  • Use fs.lstat to get file info, and use target path to process.
  • Skip files if has been processed

Other relevant information:

webpack version: 5.11.0
Node.js version: 12.16.3
Operating System: macOS Big Sur 11.0.1 (20B29)

If it is ok, I'm happy to make a pr to fix it.

@alexander-akait
Copy link
Member

Yep bug

@killagu
Copy link
Author

killagu commented Dec 21, 2020

@alexander-akait Thanks for confirm, I can have a try to fix it.

killagu added a commit to killagu/webpack that referenced this issue Dec 22, 2020
Use fs.lstat instead of fs.stat to get file meta.
If file is symbol link use fs.realpath to get real path,
and use real path to do recursive call.

Refs:
- webpack#12246
killagu added a commit to killagu/webpack that referenced this issue Dec 22, 2020
Use fs.lstat instead of fs.stat to get file meta.
If file is symbol link use fs.realpath to get real path,
and use real path to do recursive call.

Refs:
- webpack#12246
killagu added a commit to killagu/webpack that referenced this issue Dec 23, 2020
Use fs.lstat instead of fs.stat to get file meta.
If file is symbol link use fs.realpath to get real path,
and use real path to do recursive call.

Refs:
- webpack#12246
@alexander-akait
Copy link
Member

alexander-akait commented Jan 25, 2021

Other example of the problem with symlinks #12503 (comment)

killagu added a commit to killagu/webpack that referenced this issue Jan 26, 2021
Use fs.lstat instead of fs.stat to get file meta.
If file is symbol link use fs.realpath to get real path,
and use real path to do recursive call.

Refs:
- webpack#12246
@alexander-akait
Copy link
Member

Another example #12810

@webpack-bot
Copy link
Contributor

This issue had no activity for at least three months.

It's subject to automatic issue closing if there is no activity in the next 15 days.

@alexander-akait
Copy link
Member

bump

@iperzic
Copy link

iperzic commented Jul 23, 2021

Do we have an ETA on when this might be fixed?

killagu added a commit to killagu/webpack that referenced this issue Aug 3, 2021
Use fs.lstat instead of fs.stat to get file meta.
If file is symbol link use fs.realpath to get real path,
and use real path to do recursive call.

Refs:
- webpack#12246
@alexander-akait
Copy link
Member

#14019

@qiqiboy
Copy link

qiqiboy commented Sep 9, 2021

Hi @sokra, I found that some specific libs will still cause the snapshot to fail in v5.52.0, such as video.js. Install video.js in the above reproduction repo, and it can be reproduced after running.

git clone git@github.com:killagu/npminstall_webpack.git
cd npminstall_webpack
npm i -g cnpm
# must use cnpm install
cnpm i
# NEW: install the video.js library
cnpm install video.js -S
npm run build

<--- JS stacktrace --->

FATAL ERROR: invalid array length Allocation failed - JavaScript heap out of memory

@alexander-akait
Copy link
Member

@qiqiboy update wepback to the latest stable version

@qiqiboy
Copy link

qiqiboy commented Sep 9, 2021

@qiqiboy update wepback to the latest stable version

Yes, I have tried, but still fail.

cnpm install video.js webpack@latest -S
npm run build

@alexander-akait
Copy link
Member

Can't reproduce the problem

@qiqiboy
Copy link

qiqiboy commented Sep 9, 2021

@alexander-akait my env info:

$ cnpm -v                                
cnpm@7.0.0 (/usr/local/lib/node_modules/cnpm/lib/parse_argv.js)
npm@6.14.15 (/usr/local/lib/node_modules/cnpm/node_modules/npm/lib/npm.js)
node@14.17.5 (/usr/local/bin/node)
npminstall@5.0.2 (/usr/local/lib/node_modules/cnpm/node_modules/npminstall/lib/index.js)
prefix=/usr/local 
darwin x64 20.3.0 
registry=https://registry.nlark.com

截屏2021-09-09 20 33 42

@alexander-akait
Copy link
Member

Increase your memory usage, file is so bug, not related to this problem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants