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

Root binaries get removed after installing a Workspace dependency #4706

Closed
ovidiuch opened this issue Oct 13, 2017 · 8 comments · Fixed by #4730
Closed

Root binaries get removed after installing a Workspace dependency #4706

ovidiuch opened this issue Oct 13, 2017 · 8 comments · Fixed by #4730

Comments

@ovidiuch
Copy link

Reproducing on Yarn 1.20 and 1.21.

Minimal reproducible demo at skidding/yarn-bin-missing

Steps to reproduce:

Step 1: Jest is installed in workspace root and binary is present after running yarn

> yarn
> ls node_modules/.bin/jest
# node_modules/.bin/jest

Step 2: Go to foo workspace and install Lodash

> cd cd packages/foo
> yarn add lodash

Step 3: Go to back to root and 💨 node_modules/.bin is gone.

> cd -
> ls node_modules/.bin/jest
ls: node_modules/.bin/jest: No such file or directory
@milesj
Copy link

milesj commented Oct 13, 2017

Also encountering this. Running yarn install in the root temporarily fixes the issue.

@ovidiuch
Copy link
Author

Running yarn install in the root temporarily fixes the issue.

Hmm. It doesn't for me. I have to remove node_modules and install from scratch or add a new root dep to get the binaries back.

@panrafal
Copy link
Contributor

Same here.
I will add, that it also happens if yarn add (or yarn install for that matter) are called inside the workspace.
The bin files are actually removed from the root, but show up (all of them) in the said workspace.

@skidding Try runnin yarn install --force

@ovidiuch
Copy link
Author

@skidding Try runnin yarn install --force

Thanks. I remember trying this to not avail.

What works for me:

  • rm -rf node_modules && yarn
  • yarn add fooPackageIDontReallyNeed && yarn remove fooPackageIDontReallyNeed

Hope somebody finds the root cause for this though 🙏

@edmorley
Copy link
Contributor

The log output for the yarn add lodash step when using --verbose includes many lines of form:

verbose 2.385 Removing extraneous file "C:\\Users\\Ed\\src\\yarn-bin-missing\\node_modules\\.bin".
verbose 2.402 Removing extraneous file "C:\\Users\\Ed\\src\\yarn-bin-missing\\node_modules\\acorn-globals\\node_modules".
verbose 2.404 Removing extraneous file "C:\\Users\\Ed\\src\\yarn-bin-missing\\node_modules\\babel-core\\node_modules".
...
verbose 2.439 Removing extraneous file "C:\\Users\\Ed\\src\\yarn-bin-missing\\node_modules\\jest\\node_modules\\.bin".

Looking at the Git log shows #4630 made some changes to the extraneous file removal code:
https://github.com/yarnpkg/yarn/pull/4630/files#diff-306a2caea89d886b11134fb770d1ebf6

@arcanis I don't suppose you might know what's happening in this case? :-)
(Also CC @jgoz)

@edmorley
Copy link
Contributor

I wonder if this is related to:

  1. it installs all other dependencies of my-workspace to /my-workspace/node_modules

(From #4513)

In this case, the workspace package has zero dependencies, so if it was overwriting the root node_modules I guess that would explain why the .bin was seen as extraneous.

There's also #4359 about issues with binary linking.

@jgoz
Copy link
Contributor

jgoz commented Oct 17, 2017

I wonder if the root cause is that the bin links are being incorrectly determined after extraneous files are being removed (i.e., starting here: https://github.com/arcanis/yarn/blob/8ba49660fd96c4155af29e784a4891e2c263ce03/src/package-linker.js#L393).

Removing the bin links shouldn't be a problem as long as they are re-created properly, which isn't happening.

@bestander
Copy link
Member

@jgoz you are on the right path.
I think this code has an error when determining the right set of top level binaries.
A PR is welcome

@BYK BYK closed this as completed in #4730 Oct 19, 2017
BYK pushed a commit that referenced this issue Oct 19, 2017
**Summary**

Fixes #4706, fixes #4359, refs #4513. `this.config.cwd` was being used as the root for bin link paths, rather than `this.config.lockfileFolder`.

**Test plan**

- Added tests for `add` and `remove` commands (#4706)
- Added test for `install` command (#4359)
joaolucasl pushed a commit to joaolucasl/yarn that referenced this issue Oct 27, 2017
**Summary**

Fixes yarnpkg#4706, fixes yarnpkg#4359, refs yarnpkg#4513. `this.config.cwd` was being used as the root for bin link paths, rather than `this.config.lockfileFolder`.

**Test plan**

- Added tests for `add` and `remove` commands (yarnpkg#4706)
- Added test for `install` command (yarnpkg#4359)
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 a pull request may close this issue.

6 participants