search

boltpkg / bolt

⚡️ Super-powered JavaScript project management
MIT License
2.35k stars 82 forks source link

Fix windows compatibility Issue #207 #241

Closed bholloway closed 5 years ago

bholloway commented 5 years ago

Alternative fix for #207 instead of existing PR #215.

src/utils/fs.js

In the readLink method there is a comment here stating code was copied from npm cli [here]().

The copied code doesn't have a fallback when the file is not a symlink.

However when we look further in npm cli. The copied code is used in such a way here as there is a fallback similar to what @Bloodb0ne has proposed in #215.

In #215 the try/catch is duplicated in cmdShim whereas here I simply use the already safe readLink method.

~In this PR I also removed the unused path-is-inside dependency.~

src/utils/symlinkPackageDependencies.js

In my testing I also received... is not a symlink error in symlinkPackageDependencies for workspace packages that exposed bin scripts. In lerna these shims are not symlinks here and considered to be packages that were erroneously installed. They are removed here before relinking.

In bolt I suspect it is correct to read the shim as if it is a symlink and continue, similar to what is done in cmdShim (as discussed above). From what I can see bolt then tries to recreate the shim here which is presumably idempotent.

bholloway commented 5 years ago

Ping @lukebatchelor

bholloway commented 5 years ago

The CI is failing Node 4. Seems to be a known issue with yarn@latest.

lukebatchelor commented 5 years ago

LGTM! Thanks heaps @bholloway. Let me run this past Jamie quickly and we'll get this merged and released.

lukebatchelor commented 5 years ago

Ping @jamiebuilds . Looks G to me. You happy to merge?

lukebatchelor commented 5 years ago

Thanks heaps @bholloway

Released in 0.24.1