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

The "bin" field: yarn should not shim it if the developer has a launcher script #7529

Open
Artoria2e5 opened this issue Sep 6, 2019 · 2 comments

Comments

@Artoria2e5
Copy link

Artoria2e5 commented Sep 6, 2019

This is a bug that affects yarn itself. See #4497 (comment).

Current behavior

When handling the bin field, yarn only follows the filenames given exactly by the developer. This is all good and stuff, untill someone (in our case, yarn itself) finds out that some extra handling is needed and decides to write a special wrapper.

Steps to reproduce

I know y'all hate this, but it's literally yarn global add yarn with another copy of yarn (I used npm's). After that view these files with notepad.exe:

  1. %AppData\Local\Yarn\bin\yarn (launches file 2)
  2. %AppData\Local\Yarn\Data\Global\node_modules\.bin\yarn (launches the js directly)

Note: using the same yarn causes the unlink (deletion) to fail for obvious Windows reasons.

Expected behavior

File (2) should be launching the special-soup shell script "$basedir/../yarn/bin/yarn" instead of trying to let node run the js directly. The cmd version should also be launching the extra special "%~dp0\..\yarn\bin\yarn.cmd instead.

Other considerations

  • Will a silent change in behavior break anything? Should this be made an explicit extension in the bin key syntax instead?
  • If yarn was to handle this, it will need two separate calls to cmd-shim so both the sh and the cmd shim are satisfied. It would be cleaner at the API level to let cmd-shim to decide the mix, but on the other hand it would make the "smart" detection a windows-only feature. And special sh scripts are not just written for Cygwin only...
  • Why is the special soup needed in the first place? Why isn't cmd-shim doing the winpty decision?

yarn 1.17.3, node 12.5.0, windows 10.

@BYK
Copy link
Member

BYK commented Sep 6, 2019

Why is the special soup needed in the first place? Why isn't cmd-shim doing the winpty decision?

I don't recall the reason for this but I think we can safely remove all winpty stuff as it already became obsolete with never versions of Windows (both new and better terminals and the introduction of conpty).

What do you think?

@Artoria2e5
Copy link
Author

Artoria2e5 commented Sep 7, 2019

Well, winpty will always be needed as long as people run yarn from a place other than Windows's own console system ("conhost"). There is still a lot of reasons and cases for people to do something like that, mainly because they use something based on Cygwin or MSYS. (Most notably, Git Bash. And ssh sessions, which for some reason have become a thing on Windows.)

The question is more about why this shimming is not being moved to something that create the whole shell script shim in the first place. Using a script instead of a symlink is basically a Windows-only thing, and winpty (with some console detection) is a general-enough solution for that environment. We shouldn't make it a yarn-only thing; every node text UI app should have a working console by default.


Edit: wait, the current bin/yarn.js on master seems to run fine without any intervention: both colors and console interaction work in mintty's unix-style pty. I wonder what made it run differently. Gotta wrap it with something to try it out. (The shell script works as expected too, but the .cmd file is bad. Maybe some command resolution thing on the shell is not acting properly?)

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

No branches or pull requests

2 participants