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 windows hard links #99

Merged
merged 1 commit into from
Jun 25, 2024
Merged

fix windows hard links #99

merged 1 commit into from
Jun 25, 2024

Conversation

patricklx
Copy link
Contributor

@patricklx patricklx commented Jun 22, 2024

Windows has a limit of 1024 hard links per file . So this can fail quickly. Considering that pnpm also uses hard links. Apparently it can also fail for unknown reasons 🤷‍♂️.
We never encountered it before, because on windows in github actions tmp is always on C drive, while the CWD is on D drive...
and hard links are not permitted cross device.

this changes it to the same behaviour pnpm has https://github.com/pnpm/pnpm/blob/6e031e7428b3e46fc093f47a5702ac8510703a91/fs/indexed-pkg-importer/src/index.ts#L190

@mansona
Copy link
Contributor

mansona commented Jun 24, 2024

So this isn't quite the exact same as what PNPM is doing 🤔 this is opting out of hard-linking for all following files once we get one error, whereas pnpm is allowing a fallback for one file. I wonder if we just added this as a fallback and removed the concept of this.usingHardLinks if the code would be better?

I agree that something around this line has to change, when I'm debugging things on Windows I invariably have to remove this throw and let it copy the file(s)

@patricklx
Copy link
Contributor Author

So this isn't quite the exact same as what PNPM is doing 🤔 this is opting out of hard-linking for all following files once we get one error, whereas pnpm is allowing a fallback for one file. I wonder if we just added this as a fallback and removed the concept of this.usingHardLinks if the code would be better?

I agree that something around this line has to change, when I'm debugging things on Windows I invariably have to remove this throw and let it copy the file(s)

That was the behaviour before. With this change it only opts out if the error is exdev, cross device. Which will never work for any other file then.

@ef4 ef4 merged commit 9a4a99c into stefanpenner:master Jun 25, 2024
6 checks passed
@ef4 ef4 added the bug Something isn't working label Jun 25, 2024
@patricklx patricklx deleted the patch-1 branch June 25, 2024 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants