Use alternate method of generating global bin shims #6959
Conversation
|
| @@ -159,18 +160,38 @@ async function initUpdateBins(config: Config, reporter: Reporter, flags: Object) | |||
| } | |||
| } | |||
|
|
|||
| // add new bins | |||
| for (const src of afterBins) { | |||
There was a problem hiding this comment.
Wouldn't it be fixed simply but filtering afterBins here to make sure we don't iterate over .ps1 and .cmd files?
There was a problem hiding this comment.
It would still be creating shims of shims. It would fix the .ps1.cmd/.cmd.ps1/etc issue, but it would not fix the problem of there being shims pointing to shims pointing to binaries. cmd-shim does not work correctly when such a condition occurs, at least not with PowerShell.
There was a problem hiding this comment.
Longer explanation:
The cmd-shim package looks at the shebang of the program it's targeting to figure out what to write into the shim. This works fine for bash and cmd, because for bash it will always be either node or sh and for cmd it will always be either node.exe or cmd.exe. PowerShell breaks because the shebang on the PowerShell shim ends up referencing pwsh.exe, which is the binary for PowerShell Core, even when PowerShell Core is not installed. When a shim is created to a PowerShell shim, it automatically points to pwsh from the shebang, which causes problems if PowerShell Core isn't installed on the system (the version of PS that comes with Windows is powershell.exe, not pwsh.exe, and PowerShell Core is a separate installation).
I suppose it might be worth it to file an issue with cmd-shim to write the correct shebang to PowerShell shims, but I also would have to argue that it's still best to have one layer of shims if possible instead of shims-pointing-to-shims.
|
Is there any traction on this? |
…ointing to the global node_modules location
ArcanoxDragon
force-pushed
the
issue_6958
branch
from
a8c42a1 to
b3e4d3f
Compare
|
Rebased to fix conflicts due to my other PR; should be a clean merge now for if this is approved at some point |
|
Seems reasonable to me... @arcanis what do you think? |
Summary
PR #6954 is a band-aid to a deeper problem with generating global bin shims. Currently, Yarn essentially makes a "shim shim" on Windows due to older versions of Node not supporting symbolic links. This worked fine with the
shandcmdshims, but the new@zkochan/cmd-shim@3.0.0PowerShell shims are written a bit differently and broke when a "shim of a shim" was created for one.The old method of Yarn involved traversing Yarn's "Data/global/node_modules/.bin" directory after a global install operation, and for each shim file that was found,
cmd-shimwas called to create a "shim shim" in Yarn's "bin" folder. This had an unfortunate side effect of creatingshim,shim.cmd, andshim.ps1for each global shim on Windows, meaning one would end up with every permutation of extensions, such asshim.ps1.ps1, andshim.ps1would actually contain the extensionless bash shim. Previous versions of Yarn worked around this by copyingshim.cmd.cmdtoshim.cmdbut this was a band-aid at best.The new method implemented by this PR fixes both #6902 and #6958 by changing the way global shims are generated. Instead of creating shims in
<yarn prefix>/binwhich point to<yarn prefix>/Data/global/node_modules/.bin, Yarn will now parse the global lockfile after every global operation and find the top-level binaries that would be added to<yarn prefix>/Data/global/node_modules/.binand create a new top-level shim in<yarn prefix>/binpointing directly to the actual binary path in the providing package'snode_modulesfolder.I have tested this on Windows 10 using the version of PowerShell included with Windows, and the issue originally documented in #6902 is fixed by the changes made in this PR.
Test plan
A test was added to
__tests__/commands/global.jsto ensure the proper binaries are created on Windows. The existing global tests should cover regression testing. I have tested this PR locally and successfully used a global binary installed with my local compiled version of Yarn without issue.