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

Disable creation of cmdShim PowerShell shims because they don't work and cause problems #6954

Merged
merged 6 commits into from Mar 14, 2019

Conversation

ArcanoxDragon
Copy link
Contributor

Summary

Fixes #6902. The latest version of @zkochan/cmd-shim has built-in support for PowerShell shims, but they don't work correctly (even the correct .ps1 shims throw errors in PowerShell). In addition, due to the funky way that Yarn handles global binaries, every permutation of (no extension/cmd/ps1) gets created (e.g. .cmd.ps1, .cmd.cmd, etc) as a shim. Prior to the update to @zkochan/cmd-shim, Yarn suppressed this by renaming .cmd.cmd to just .cmd, which is not the most elegant solution, but it works. As a (temporary?) workaround to the broken PowerShell shims, this PR simply disables the creation of PowerShell shims altogether whenever Yarn calls @zkochan/cmd-shim.

Test plan

A new test was added in __tests__/commands/global.js (shouldn't create powershell shims) which ensures that no PowerShell shims are created, and that the duplicate-extension files do not exist.

@khs1994
Copy link

khs1994 commented Mar 14, 2019

Can create powershell file, not delete it?

Copy link
Contributor

@ExE-Boss ExE-Boss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is indeed correct for disabling PowerShell shim creation.

review?(@arcanis)

@arcanis
Copy link
Member

arcanis commented Mar 14, 2019

Will it introduce a regression? I'd be more enclined to merge this one than #6959, since the later seems quite a bit more complex.

cc @zkochan - are you aware of this issue?

@ExE-Boss
Copy link
Contributor

This will restore cmd shim creation behaviour to how it was before 1.13.

@arcanis
Copy link
Member

arcanis commented Mar 14, 2019

Sounds good then - I'll merge this and release it with the 1.15.1

@buildsize
Copy link

buildsize bot commented Mar 14, 2019

File name Previous Size New Size Change
yarn-[version].noarch.rpm 1.11 MB 1.11 MB -1.81 KB (0%)
yarn-[version].js 4.47 MB 4.47 MB -3.85 KB (0%)
yarn-legacy-[version].js 4.67 MB 4.66 MB -4.5 KB (0%)
yarn-v[version].tar.gz 1.12 MB 1.12 MB -140 bytes (0%)
yarn_[version]all.deb 816.45 KB 815.62 KB -850 bytes (0%)

@zkochan
Copy link

zkochan commented Mar 14, 2019

If @ExE-Boss has accepted this, then LGTM. He has more experience with it.

We don't change cmd-shim frequently. We can include @arcanis in any future changes.

@arcanis arcanis merged commit aff9d63 into yarnpkg:master Mar 14, 2019
@ArcanoxDragon ArcanoxDragon deleted the issue_6902 branch March 14, 2019 16:23
@arcanis arcanis mentioned this pull request Mar 15, 2019
r15ch13 pushed a commit to ScoopInstaller/Scoop that referenced this pull request Mar 18, 2019
- Yarn's global folder can be set via `yarn config set global-folder` (yarnpkg/yarn#7056) and thus can be persisted. 
- ~~Since yarn's global bin creating procedure is still problematic (yarnpkg/yarn#6902, **fixed by yarnpkg/yarn#6954 The `.bin` folder in `global\node_modules` is a better path to add to env, and this can avoid the annoying problem when you install scoop in some place except `C:` (that the shims in global bin have wrong relative path pointer).
- If you install yarn via `scoop install yarn`, the `Yarn` folder in `$env:LOCALAPPDATA` is useless, and when you uninstall `yarn`, the `.yarnrc` is unused, so the manifest add `uninstaller.script` to remove them when you uninstall.
- For reconfiguration, please use `scoop update yarn -f` instead of `scoop reset yarn`.

- Close #2969
@ghost
Copy link

ghost commented Mar 9, 2020

So does this mean I can't use yarn to global install packages?
Currently, global installing through yarn causes CLI's not to work

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.

Globally installed binaries do not work in PowerShell after 1.13.0
5 participants