fixes hardlink collitions #3691
Conversation
src/util/fs.js
Outdated
| @@ -450,6 +450,8 @@ async function buildActionsForHardlink( | |||
|
|
|||
| // push all files to queue | |||
| invariant(srcFiles, 'src files not initialised'); | |||
| // hardlinking is per package, not going into sub packages | |||
| srcFiles = srcFiles.filter(f => f !== 'node_modules'); | |||
There was a problem hiding this comment.
Why is there paths from the node_modules here? Shouldn't they be handled outside of this function (not completely sure how works the linking pipeline yet)? + will it work with bundledDependencies?
src/fetchers/tarball-fetcher.js
Outdated
| @@ -175,13 +176,16 @@ export default class TarballFetcher extends BaseFetcher { | |||
| } | |||
|
|
|||
| async _fetch(): Promise<FetchedOverride> { | |||
| const isFilePath = this.reference.startsWith('file:'); | |||
There was a problem hiding this comment.
copied from #3709 for bundledDependencies case
|
@arcanis, you were right, this is trickier but I found a way around and added more tests |
|
Review please |
| @@ -360,6 +360,11 @@ async function buildActionsForHardlink( | |||
| const {src, dest} = data; | |||
| const onFresh = data.onFresh || noop; | |||
| const onDone = data.onDone || noop; | |||
| if (files.has(dest)) { | |||
There was a problem hiding this comment.
What happens if an hardlinked file needs to change? Will it be removed in an earlier pass? Unless it is, maybe we could try comparing the inodes as well
There was a problem hiding this comment.
Oh it does compare inodes for sure, this check just prevents assessing the same path twice, a folder in particular.
We have A -> B structure that we want to hardlink to A1 -> B1, package-linker passes that modules A1 and B1 need to be hardlinked independently but part of hardlinking is that it is recursive, so files in B1 will be scheduled to be linked twice.
I could not find a more elegant way of doing this especially with bundledDependencies because they don't participate in hoisting.
There was a problem hiding this comment.
I'll add this comment to the code for clarity
Summary
Fixes #2734
The problem in hardlinking code trying to create a hardlink per package twice.
That might happen when the same there were a few packages hardlinked in a hierarchy.
Test plan
Added test from the issue.