yarnpkg / yarn

The 1.x line is frozen - features and bugfixes now happen on https://github.com/yarnpkg/berry
https://classic.yarnpkg.com
Other
41.44k stars 2.73k forks source link

Sub dependency .bin symlinks can override top level dependency .bin symlink #4937

Closed anders-advisa closed 6 years ago

anders-advisa commented 6 years ago

Do you want to request a feature or report a bug?

Bug

What is the current behavior? Sub dependencies .bin symlinks override top level dependencies .bin symlinks in some cases (seems to be depending on which top level package is first when sorted alphabetically). Likely introduced in https://github.com/yarnpkg/yarn/pull/3310

If the current behavior is a bug, please provide the steps to reproduce.

Example: https://github.com/anders-advisa/yarn-symlink-bug

What is the expected behavior? Dependencies explicitly entered into package.json should have precedence over sub dependencies.

Please mention your node.js, yarn and operating system version. node: 8.9.1 yarn: 1.3.2

anders-advisa commented 6 years ago

The problem I encountered was with LiveScript and livescript which is two different packages what both specifiy a binary called lsc. With npm we get the correct binary linked in .bin, with yarn we sometimes get the binary from the wrong package (as it is a sub dependency) which blocks us from switching to yarn.

torifat commented 6 years ago

we sometimes get the binary from the wrong package

Can you explain it a bit? Did you mean that if you yarn install without anything it creates the symlink randomly? Or it changes when you change version of yarn?

dbushong commented 6 years ago

This is a recurring bug that keeps being marked fix and never quite is. Simple repro:

https://github.com/dbushong/yarn-bug

anders-advisa commented 6 years ago

@torifat This code tries to decide which top level bin links should be created. The problem is that it goes by package name, not the link name. And since packages can have bin links (even multiple ones) with whatever names they chose it will do the wrong thing in a lot of cases where differently named packages have bin links with the same name, such as in my reproduction above.

torifat commented 6 years ago

@anders-advisa since you have figured out the root of the problem. Do you want to submit a PR? I would happy to assist you 😃

anders-advisa commented 6 years ago

@torifat haha, I actually spent an hour on two on that (which is why I know what the problem is), but I couldn't get the test suite working properly (took very long time and I got a few failing tests in master). But I'll probably have time to look at it again next week or so.