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

fix(plugin-pnpm) Pnpm linker avoids symlink loops on the file system #4542

Merged
merged 6 commits into from
Jul 18, 2022

Conversation

larixer
Copy link
Member

@larixer larixer commented Jun 9, 2022

What's the problem this PR addresses?

Fixes #4525

How did you fix it?

Symlinks for dependencies are created higher in the directory structure. Instead of
<project_root>/node_modules/.store/webpack-cli-virtual-66f7aceb77/node_modules/webpack-cli/node_modules/commander,
the symlink is created at:
<project_root>/node_modules/.store/webpack-cli-virtual-66f7aceb77/node_modules/commander
This is the same way as pnpm does. This avoids the symlink loop on the file system. It works, because Node.js fallbacks to realpath and parent directories look up when it fails to find a module without applying realpath.

Checklist

  • I have set the packages that need to be released for my changes to be effective.
  • I will check that all automated PR checks pass before the PR gets reviewed.

e-im added a commit to PaperMC/docs that referenced this pull request Jun 26, 2022
Manually merge yarnpkg/berry#4542 to fix issue with the dev server.
@arcanis arcanis dismissed a stale review via efee3ee June 30, 2022 18:26
@larixer larixer requested a review from arcanis July 15, 2022 12:28
@merceyz merceyz added the infra: pending update A bot will merge master into this PR label Jul 15, 2022
@github-actions github-actions bot removed the infra: pending update A bot will merge master into this PR label Jul 15, 2022
merceyz
merceyz previously approved these changes Jul 15, 2022
@merceyz merceyz force-pushed the larixer/pnpm-symlink-loops branch from 2f430a4 to 9febb09 Compare July 15, 2022 13:32
@merceyz
Copy link
Member

merceyz commented Jul 15, 2022

Downside to having the bot update the branch, the CI wont run.

@arcanis arcanis merged commit 73b3fab into master Jul 18, 2022
@arcanis arcanis deleted the larixer/pnpm-symlink-loops branch July 18, 2022 15:01
jakutis pushed a commit to jakutis/berry that referenced this pull request Aug 11, 2022
…arnpkg#4542)

* Pnpm linker avoids symlink loops on the file system

* symlinkDst -> symlinkDstPath for uniformity with nearby code

* Avoids self-require trap

* Store paths rather than linkType, and remove now-irrelevant cleanup code

* Adds test

* Don't create self-references for workspaces, to avoid infinite loops

Co-authored-by: Maël Nison <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug?]: pnpm strategy breaks webpack cache
3 participants