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

Deps that add files outside of yarn add (e.g., selenium-standalone) have these files purged #1955

Closed
msegado opened this issue Nov 19, 2016 · 9 comments

Comments

@msegado
Copy link

msegado commented Nov 19, 2016

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

...bug? (It's unclear whether this should be yarn's responsibility or the installed package's)

What is the current behavior?

Yarn normally purges unexpected files found in node_modules as described in #231 (comment). To avoid purging build artifacts, yarn keeps a list of files created during the install process and doesn't remove those (see 3f765c3 for the latest implementation).

Some packages, however, add files to their installation directory outside of the regular lifecycle hooks; selenium-standalone for example provides an install CLI command which downloads the selenium .jar and user-specified driver binaries to [project]/node_modules/selenium-standalone/.selenium. Since these are not in yarn's list of artifacts, they're eventually purged.

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

echo "{}" > package.json
yarn add --dev selenium-standalone
node_modules/.bin/selenium-standalone install
node_modules/.bin/selenium-standalone start  # Works

yarn add left-pad
node_modules/.bin/selenium-standalone start  # Error: Missing [...]/.selenium/chromedriver/2.25-x64-chromedriver

What is the expected behavior?

Before seeing #231, my expectation was that dependencies' folders would not be touched by yarn after they had been installed. I see a few ways forward:

  • If that this isn't a supported use case for yarn,

    • leave the purging behavior as is,
    • add a note about it to the docs, and
    • open issues with packages that modify their install folders to have them provide an alternative.
  • If this is a supported use case for yarn, modify the purging implementation to permit it.

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

yarn 0.17.6
Node 6.9.1
OS X 10.11.6

...Thanks for the great tool! 📦

@lucasconstantino
Copy link

This is really anoying. I use Nightwatch for e2e tests, and before switching to Yarn I only needed to install that packaged and everything would just work.

My workarround: add npm install chromedriver as the "install" script of my package. Yarn will execute it after the install process and everything should work.

@msegado
Copy link
Author

msegado commented Nov 28, 2016

I'm increasingly thinking this should be addressed by the other packages (e.g. selenium-standalone) rather than Yarn. The core issue is that by providing separate install commands they're essentially implementing their own package management system rather than using the existing NPM/Yarn package lifecycle hooks... I don't think it should be Yarn's responsibility to deal with such external mutations to node_modules and will consider opening bugs on those repos.

That said, it would be nice to make Yarn's behavior less surprising to new users, either by...

  1. printing a warning when Yarn detects and purges unexpected files from node_modules, or
  2. making sure that commands which appear limited to a specific package (e.g. yarn add left-pad in my example above, but not yarn install) don't touch any other packages's folders, and/or
  3. prompting the user for confirmation any time Yarn is about to purge unexpected files.

This may also help avoid accidental data loss... it seems unlikely, but I could imagine myself experimenting with making changes in-place to one of my deps (with the goal of eventually cloning its repo and opening a PR) only to have a new file containing some of my work unexpectedly rm -rf-ed when I ran a seemingly-unrelated yarn command...

@kajmagnus
Copy link

I'm having this issue too, with Yarn removing the Selenium files. In addition to 1,2,3 above, perhaps:

4. A command line flag, e.g. --dont-delete-unexpected-files

@CrabDude
Copy link
Contributor

CrabDude commented Apr 12, 2017

This is occurring in master HEAD (#977497a71d57ed499d154d9a025d7d559f81a54c) presently where native packages are having their build artifacts deleted on the 2nd run of yarn install, effectively breaking yarn install with native dependencies.

EDIT: This is on nodejs@4 since I've noticed yarn isn't fully considering v4 breakages. E.g., yarn add global xxxx is broken due to the use of the unavailable array.includes.

EDIT 2: It only happens when using --check-files, so for me it's unrelated to the original issue.

EDIT 3: I happens without --check-files due to a build artifact not getting removed from the extraneous files list. I'm not sure why it's either not removed or not getting added to the artifacts list (to be removed). As luck would have it, once I was able to reliably reproduce, as I was about to debug it, it stopped occurring. I was able to discern that it was triggered via a yarn.lock update (without a package.json update) and the consequent Integrity check: Lock files don't match.

IOW, what I'm seeing might be related to the OP. It was very difficult to reproduce, and wasn't able to debug before it inexplicably went away.

@bestander
Copy link
Member

Great explanation, @msegado.
Indeed we look at Yarn as the guard of node_modules and if something happens outside of Yarn's lifecycle scripts Yarn would undo it.

Does anyone (@msegado?) want to help the project with the direction in this area?
I think any change here needs to be done via RFC so that we could agree on the approach.

A few random thoughts:

  • keeping a list of packages that abuse node_modules does not seem to scale well
  • yarn could prompt user for a choice when unexpected files are removed
  • maybe a project could opt in to ignore changes in some of the folders after the prompt

@bestander
Copy link
Member

A follow up on this.
After #3224 .yarn-integrity file contains list of artifacts that were modified at scripts execution phase, that should address a few bugs related to this issue but it does not really address this one.

If selenium-standalone had install: install in package.json then built/downloaded artifacts would be recorded by Yarn as non-extraneous files.

But selenium-standalone does not do this, so whatever gets downloaded into node_modules after installation will be cleaned up.

I see 3 ways to solve this:

  • Make sure packages self install into node_modules during installation phase like node-sass for example, i.e. follow up with package authors
  • Add a config to Yarn that would re-record artifacts when running commands, e.g. yarn run selenium-standalone install --record-artifacts
  • Make Yarn users add selenium-standalone install to projects' install phases and make sure Yarn keeps track of changed files after install/postinstall scripts

@okonet
Copy link

okonet commented Sep 5, 2017

Same issue with node-sass package when every time I use yarn add ... it remove the binary node-sass and running the project build results in the following error:

     [exec] >> Error: Missing binding /Users/okonet/Projects/feedly/client/node_modules/node-sass/vendor/darwin-x64-57/binding.node
     [exec] >> Node Sass could not find a binding for your current environment: OS X 64-bit with Node.js 8.x
     [exec] >> 
     [exec] >> Found bindings for the following environments:
     [exec] >>   - OS X 64-bit with Node.js 6.x
     [exec] >> 
     [exec] >> This usually happens because your environment has changed since running `npm install`.
     [exec] >> Run `npm rebuild node-sass --force` to build the binding for your current environment.

Running yarn install --force fixes it but it takes much longer than just running yarn and kind'a defeats the purpose of using yarn in the first place.

@CrabDude
Copy link
Contributor

CrabDude commented Sep 7, 2017

I've experienced many manifestations of this bug. It seems to be fixed in 1.0.0 for me, as it seems yarn no longer prunes files not in node_modules/.yarn-integrity, which was an issue for <1.0.0. Specifically, prior to 1.0.0, the following occurred:

yarn init
yarn add postcss
yarn add leftpad
# Manually edit out leftpad from package.json & yarn.lock to simulate a git rebase
yarn install --check-files

Previously, the above would cause postcss#caniuse-api/features.js to be pruned, but no longer occurs. Tracking down this bug has been a major headache for us, so a huge thanks to all the yarn developers that've contribute to making yarn better. This was the last major annoyance in our adoptance of yarn.

That said, caniuse-api/features.js is still not in the .yarn-integrity as expected, but it's there when yarn install completes, so it seems like the audit that creates the list in .yarn-integrity could occur a little later to ensure possibly asynchronously created files or postInstall created files are included. I expect it would address this issue.

@BYK
Copy link
Member

BYK commented Sep 8, 2017

I've experienced many manifestations of this bug. It seems to be fixed in 1.0.0 for me, as it seems yarn no longer prunes files not in node_modules/.yarn-integrity, which was an issue for <1.0.0. Specifically, prior to 1.0.0, the following occurred:

So the core issue is resolved! 🎉

That said, caniuse-api/features.js is still not in the .yarn-integrity as expected, but it's there when yarn install completes, so it seems like the audit that creates the list in .yarn-integrity could occur a little later to ensure possibly asynchronously created files or postInstall created files are included. I expect it would address this issue.

This is an interesting idea. Mind creating a separate issue for tracking it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants