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

[in docker] yarn (linking) touches module's files without need #4764

Open wclr opened 7 years ago

wclr commented 7 years ago

Well this is probably also some docker related issue, but the thing is that it didn't happen on older version of yarn.

Env: Windows 10 Docker for windows

Compare results of following steps in container:

docker run --rm -it mhart/alpine-node:<version> sh
mkdir app
cd app
yarn init
yarn add bson
stat node_modules/bson/index.js
yarn --force //or other command: yarn add ...
stat node_modules/bson/index.js

For yarn 1.2.0 (alpine-node:8.7.0): docker run --rm -it mhart/alpine-node:8.7.0 sh image

and older yarn 0.24.6 (alpine-node:8.1.0) docker run --rm -it mhart/alpine-node:8.1.0 sh

image

This leads for example to unexpected result that apps in watch mode (which are watching for all required files including node_modules) are restarted without need while yarn --force.

Also this seem not to be the case for Yarn 1.2 running not in docker.

edmorley commented 7 years ago

Hi!

I can repro using the STR above, and have a reduced regression range of yarn 1.1.0 (from the mhart/alpine-node:8.6.0 image) as the first affected, and yarn 1.0.2 (from mhart/alpine-node:8.5.0) being the last good. (I can also repro using the stock node:8.8.0-alpine and node:8.8.0 images too fwiw).

In addition to causing apps in watch mode to unnecessarily reload, this presumably has Docker image/container size implications, given copy on write (for cases where the initial yarn install was in the image, and the yarn --force is run in the container).

wclr commented 7 years ago

And this is the really annoying bug because now any installation that doesn't touch any package related to service triggers a restart of all services that use anything from node_modules.

wclr commented 6 years ago

Checked with latest alpine-node:9.2.0 with yarn 1.3.2, the problem persists. @bestander any ideas what can be the cause of such behavior?

bestander commented 6 years ago

yarn --force is quite an overkill, it ignores everything in node_modules and reinstalls all packages. It may skip file copy operations if files did not change but it most likely rewrites the .bin folder

wclr commented 6 years ago

@bestander Well, couple of notice about yarn --force: 1) fresh install take much longer than yarn --force with full node_modules. 2) I have to use yarn --force to enforce installing of changed file/link deps (which are not checked by yarn command), and also to purge yarn.lock.

And the case is that on native OS yarn works ok it doesn't change files, the problem is seen in docker (at least alpine-node not sure about other os/images).

It may skip file copy operations if files did not change but it most likely rewrites the .bin folder

But as you may see from this simple repro sample (above):

docker run --rm -it mhart/alpine-node:9.2.0 sh
mkdir app
cd app
yarn init
yarn add bson
stat node_modules/bson/index.js
yarn --force
stat node_modules/bson/index.js

everything is touched not only .bin folder.

bestander commented 6 years ago

I think force should not be used in regular day to day use, it is a nuclear option when things went wrong. In your case Yarn needs to support reinstall file/link dependencies. Maybe a new flag would be ok here.

Would you want to send a PR?

wclr commented 6 years ago

I'll think about it, I also seem to have other problems (not only updating local deps), which make it necessary to use --force in a container based automated workflow to keep things consistent.

Still, I think this issue should be investigated.

wclr commented 6 years ago

@bestander I've updated the issue it is not --force that causes this issue, but any command that will lead to linking.

wclr commented 6 years ago

The problem is here: from 1.0.2 native fs.copyFile is used.

This causes the problem in docker container, it seem to do something wrong to file time stats. On windows natively it works ok. Need to test it on docker on other OS.

Seems there are not any options for copyFile that could fix it. Any ideas what the problem can be and how it can be fixed? @BYK

@edmorley Temporal fix for affected containers: sed -i -e 's/.copyFile ?/\._copyFile ?/g' /usr/local/share/yarn/lib/cli.js

bestander commented 6 years ago

Oh, that is a good find, thanks @whitecolor.

pinging @BYK who did https://github.com/yarnpkg/yarn/pull/4486 and native fs.copy and @Daniel15 who is our cross-platform expert

wclr commented 5 years ago

Hey, guys still not solved.