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

bug: Yarn uses old cached local file version of a package #2649

Closed
Timer opened this issue Feb 6, 2017 · 17 comments

Comments

Projects
None yet
5 participants
@Timer
Copy link

commented Feb 6, 2017

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

What is the current behavior?
Yarn uses old cached version of a file package instead of newer one.

If the current behavior is a bug, please provide the steps to reproduce.
Install a package on filesystem, update contents (without changing version), reinstall and see old version.

What is the expected behavior?
To correctly detected the cached version is invalid (hash) and use the new one.


package.json

...

  "devDependencies": {
    "react-scripts": "/Users/joe/Desktop/tinker/create-react-app/packages/react-scripts/react-scripts-0.8.5.tgz"
  },

...

yarn.lock

...

react-scripts@/Users/joe/Desktop/tinker/create-react-app/packages/react-scripts/react-scripts-0.8.5.tgz:
  version "0.8.5"
  resolved "/Users/joe/Desktop/tinker/create-react-app/packages/react-scripts/react-scripts-0.8.5.tgz#670169dc60993eea9014fb7729890df31c267233"
  dependencies:
    autoprefixer "6.5.1"
    babel-core "6.17.0"
    babel-eslint "7.1.1"
    babel-jest "18.0.0"
    babel-loader "6.2.10"
    babel-preset-react-app "file:/Users/joe/Desktop/tinker/create-react-app/packages/babel-preset-react-app"
    babel-runtime "^6.20.0"
    case-sensitive-paths-webpack-plugin "1.1.4"
    chalk "1.1.3"
    connect-history-api-fallback "1.3.0"
    cross-spawn "4.0.2"
    css-loader "0.26.1"
    detect-port "1.0.1"
    dotenv "2.0.0"
    eslint "3.8.1"
    eslint-config-react-app "file:/Users/joe/Desktop/tinker/create-react-app/packages/eslint-config-react-app"
    eslint-loader "1.6.1"
    eslint-plugin-flowtype "2.21.0"
    eslint-plugin-import "2.0.1"
    eslint-plugin-jsx-a11y "2.2.3"
    eslint-plugin-react "6.4.1"
    extract-text-webpack-plugin "2.0.0-rc.2"
    file-loader "0.10.0"
    filesize "3.3.0"
    fs-extra "0.30.0"
    gzip-size "3.0.0"
    html-webpack-plugin "2.28.0"
    http-proxy-middleware "0.17.2"
    jest "18.1.0"
    object-assign "4.1.0"
    postcss-loader "1.2.2"
    promise "7.1.1"
    react-dev-utils "file:/Users/joe/Desktop/tinker/create-react-app/packages/react-dev-utils"
    recursive-readdir "2.1.0"
    strip-ansi "3.0.1"
    style-loader "0.13.1"
    url-loader "0.5.7"
    webpack "2.2.1"
    webpack-dev-server "2.2.1"
    webpack-manifest-plugin "1.1.0"
    whatwg-fetch "1.0.0"
  optionalDependencies:
    fsevents "1.0.14"

...

Expected:

$ mkdir inspect
$ tar -xzf react-scripts-0.8.5.tgz -C inspect/
$ sed -n 107,113p inspect/package/config/webpack.config.prod.js 
  resolveLoader: {
    modules: [
      paths.ownNodeModules,
      // Lerna hoists everything, so we need to look in our app directory
      paths.appNodeModules
    ]
  },

Actual (installed by yarn):

$ sed -n 107,113p node_modules/react-scripts/config/webpack.config.prod.js 
  resolveLoader: {
    modules: [
      paths.ownNodeModules
    ]
  },
  // @remove-on-eject-end
  module: {

This works as expected with npm.

yarn cache clean fixes this (once) but it is still a huge annoyance to clean my yarn cache every time I want to test a local package.

@Timer Timer referenced this issue Feb 6, 2017

Closed

Yarn + Create React App Umbrella #1189

7 of 12 tasks complete

@Timer Timer changed the title bug: Yarn uses old cached local file version bug: Yarn uses old cached local file version of a package Feb 6, 2017

@bestander

This comment has been minimized.

Copy link
Member

commented Feb 7, 2017

There should be an open issue for that and I think a PR is in the works

@Timer

This comment has been minimized.

Copy link
Author

commented Feb 7, 2017

Alright, thanks! I just ran into it so I just wanted to make sure it was being taken care of. 😄

Thanks for all your hard work!

@bestander bestander self-assigned this Feb 7, 2017

@da70

This comment has been minimized.

Copy link

commented Feb 8, 2017

I believe that I've come across the bug described in this ticket. I was going to submit a bug report and PR, but since this issue is open already will just add a comment (couldn't find the open issue that @bestander was referring to two comments up).

I found that changing this code in lib/package-fetcher.js:

EDIT: had originally pasted in code from src/package-fetcher.js by accident. Replaced with generated @code.

if (yield _this2.config.isValidModuleDest(dest)) {
    return _this2.fetchCache(dest, fetcher);
}

...to:

if (yield _this2.config.isValidModuleDest(dest)) {
    const metadata = yield _this2.config.readPackageMetadata(dest);

    if (metadata.remote.resolved === ref.remote.resolved) {
        return _this2.fetchCache(dest, fetcher);
    }
}

...fixed the problem. Note that I did this the package-fetcher.js created by yarn run build, not the similarly named src/package-fetcher.js, which appears to be different from the resulting /lib/package-fetcher.js. I don't currently know anything about the build process so don't know where this fix could be tested in the actual source files.

EDIT: This ticket is closed and the bug I was going to report no longer occurs in current master. For edification of anyone reading this, though, just providing the change for src/package-fetcher.js, which is babel-fied into lib/package-fetcher.js:

if (await this.config.isValidModuleDest(dest)) {
  const metadata = await this.config.readPackageMetadata(dest);

  if (metadata.remote.resolved === ref.remote.resolved) {
    return this.fetchCache(dest, fetcher);
  }
}

Just thought I'd post these findings here in case it's of any help to the efforts already underway.

Steps for bug reproduction:

$ ls
package.json.09fd3b55a61cb4288a479a1c956de4975ce1e92d    package.json.b2ff01deeaef7bde62cee7539aad143a8df2a2be

$ cat package.json.09fd3b55a61cb4288a479a1c956de4975ce1e92d
{
  "name": "bug",
  "version": "0.0.1",
  "description": "bug",
  "dependencies": {
    "urijs": "medialize/URI.js#09fd3b55a61cb4288a479a1c956de4975ce1e92d"
  }
}

$ cat package.json.b2ff01deeaef7bde62cee7539aad143a8df2a2be
{
  "name": "bug",
  "version": "0.0.1",
  "description": "bug",
  "dependencies": {
    "urijs": "medialize/URI.js#b2ff01deeaef7bde62cee7539aad143a8df2a2be"
  }
}

$ ln -s package.json.b2ff01deeaef7bde62cee7539aad143a8df2a2be package.json

$ yarn cache clean
yarn cache v0.18.1
success Cleared cache.
✨  Done in 0.09s.

$ yarn_cache_dir=$( yarn cache dir | head -2 | tail -1 )

$ yarn
yarn install v0.18.1
info No lockfile found.
warning bug@0.0.1: No license field
[1/4] □□  Resolving packages...
[2/4] □□  Fetching packages...
[3/4] □□  Linking dependencies...
[4/4] □□  Building fresh packages...
success Saved lockfile.
✨  Done in 0.77s.

$ cat yarn.lock
# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
# yarn lockfile v1


urijs@medialize/URI.js#b2ff01deeaef7bde62cee7539aad143a8df2a2be:
  version "1.18.4"
  resolved "https://codeload.github.com/medialize/URI.js/tar.gz/b2ff01deeaef7bde62cee7539aad143a8df2a2be"

$ yarn cache ls
yarn cache v0.18.1
Name  Version Registry Resolved                                                                                    
urijs 1.18.4  npm      https://codeload.github.com/medialize/URI.js/tar.gz/b2ff01deeaef7bde62cee7539aad143a8df2a2be
✨  Done in 0.06s.

$ cp -pr $yarn_cache_dir/npm-urijs-1.18.4/ ./urijs-b2ff01deeaef7bde62cee7539aad143a8df2a2be

$ rm -fr node_modules/

$ mv yarn.lock yarn.lock.b2ff01deeaef7bde62cee7539aad143a8df2a2be

$ yarn cache clean
yarn cache v0.18.1
success Cleared cache.
✨  Done in 0.07s.

$ rm package.json

$ ln -s package.json.09fd3b55a61cb4288a479a1c956de4975ce1e92d package.json

$ yarn
yarn install v0.18.1
info No lockfile found.
warning bug@0.0.1: No license field
[1/4] □□  Resolving packages...
[2/4] □□  Fetching packages...
[3/4] □□  Linking dependencies...
[4/4] □□  Building fresh packages...
success Saved lockfile.
✨  Done in 0.73s.

$ cat yarn.lock
# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
# yarn lockfile v1


urijs@medialize/URI.js#09fd3b55a61cb4288a479a1c956de4975ce1e92d:
  version "1.18.4"
  resolved "https://codeload.github.com/medialize/URI.js/tar.gz/09fd3b55a61cb4288a479a1c956de4975ce1e92d"

$ yarn cache ls
yarn cache v0.18.1
Name  Version Registry Resolved                                                                                    
urijs 1.18.4  npm      https://codeload.github.com/medialize/URI.js/tar.gz/09fd3b55a61cb4288a479a1c956de4975ce1e92d
✨  Done in 0.06s.

$ cp -pr $yarn_cache_dir/npm-urijs-1.18.4/ ./urijs-09fd3b55a61cb4288a479a1c956de4975ce1e92d

$ rm yarn.lock

$ rm -fr node_modules/

$ rm package.json

$ ln -s package.json.b2ff01deeaef7bde62cee7539aad143a8df2a2be package.json

$ cp -p yarn.lock.b2ff01deeaef7bde62cee7539aad143a8df2a2be yarn.lock

$ yarn
yarn install v0.18.1
warning bug@0.0.1: No license field
[1/4] □□  Resolving packages...
[2/4] □□  Fetching packages...
[3/4] □□  Linking dependencies...
[4/4] □□  Building fresh packages...
✨  Done in 0.41s.

$ cat yarn.lock

# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
# yarn lockfile v1


urijs@medialize/URI.js#b2ff01deeaef7bde62cee7539aad143a8df2a2be:
  version "1.18.4"
  resolved "https://codeload.github.com/medialize/URI.js/tar.gz/b2ff01deeaef7bde62cee7539aad143a8df2a2be"

$ diff -r node_modules/urijs/ urijs-b2ff01deeaef7bde62cee7539aad143a8df2a2be/
Only in urijs-b2ff01deeaef7bde62cee7539aad143a8df2a2be/: .yarn-metadata.json
Only in urijs-b2ff01deeaef7bde62cee7539aad143a8df2a2be/: .yarn-tarball.tgz
diff -r node_modules/urijs/CHANGELOG.md urijs-b2ff01deeaef7bde62cee7539aad143a8df2a2be/CHANGELOG.md
5,8d4
< ### master
< 
< * prevent `new URI(null)` from blowing up - [PR #321](https://github.com/medialize/URI.js/issues/321)
< 
diff -r node_modules/urijs/src/URI.js urijs-b2ff01deeaef7bde62cee7539aad143a8df2a2be/src/URI.js
64,69d63
<     if (url === null) {
<       if (_urlSupplied) {
<         throw new TypeError('null is not a valid argument for URI');
<       }
<     }
< 
diff -r node_modules/urijs/test/test.js urijs-b2ff01deeaef7bde62cee7539aad143a8df2a2be/test/test.js
22,26d21
<   test('URI(null)', function() {
<     raises(function() {
<       URI(null);
<     }, TypeError, 'Failing undefined input');
<   });

$ diff -r node_modules/urijs/ urijs-09fd3b55a61cb4288a479a1c956de4975ce1e92d/
Only in urijs-09fd3b55a61cb4288a479a1c956de4975ce1e92d/: .yarn-metadata.json
Only in urijs-09fd3b55a61cb4288a479a1c956de4975ce1e92d/: .yarn-tarball.tgz
@akloeber

This comment has been minimized.

Copy link

commented Feb 8, 2017

@da70 I think the issue @bestander is referring to is #2165.

@bestander

This comment has been minimized.

Copy link
Member

commented Feb 8, 2017

Thanks, @akloeber.
I'll close this one

@bestander bestander closed this Feb 8, 2017

@da70

This comment has been minimized.

Copy link

commented Feb 8, 2017

@akloeber Thanks. I didn't read the ticket closely enough...it appears to apply only to local dependencies. The bug I ran into is different. I'll open a new ticket if I don't find an existing one addressing it.

@da70

This comment has been minimized.

Copy link

commented Feb 8, 2017

Can't reproduce the bug in version 0.21.0-0 (current master) using the steps above. I notice now that in yarn cache the commit is appended to the end of the directory.
The above steps were a contrived case using specific commit IDs because that was the only way to reliably reproduce the bug. Originally when I ran into this problem was just using version numbers, but reproducibility of that case was dependent on URI.js staying the same. Will see if the bug case I originally had is still an issue.

@da70

This comment has been minimized.

Copy link

commented Feb 15, 2017

In case anyone using an older version of yarn stumbles across this ticket while looking for answers about caching irregularities, the bug I was trying to describe above was resolved by Add hash to cache path for non-NPM packages (#2074).
This fix is in release v0.20.3.

@akloeber

This comment has been minimized.

Copy link

commented Feb 16, 2017

@da70 Are you sure this fix is already contained in v0.20.3? In my cache folder the hashes are still missing for local file dependencies (npm-<LOCAL_MODULE_NAME>-<LOCAL_MODULE_VERSION>) and updates are only triggered if version number is incremented.

@akloeber

This comment has been minimized.

Copy link

commented Feb 16, 2017

I checked the sources of the yarn version that is installed on my machine and the changes from #2074 were present there. I have no idea why the hashing does still not occur.

This can be reproduced with:

├── local
│   ├── index.js
│   └── package.json
└── package.json

package.json:

{
  "name": "test",
  "version": "1.0.0",
  "license": "MIT",
  "dependencies": {
    "local": "file:./local"
  }
}

local/package.json:

{
  "name": "local",
  "version": "1.0.0",
  "license": "MIT"
}

local/index.js:

module.exports = 'foo';

This produces ~/Library/Caches/Yarn/npm-local-1.0.0 folder after running yarn:

$ yarn
yarn install v0.20.3
[1/4] 🔍  Resolving packages...
[2/4] 🚚  Fetching packages...
[3/4] 🔗  Linking dependencies...
[4/4] 📃  Building fresh packages...
✨  Done in 0.25s.

Subsequent changes to in local/index.js do not have any effect:

module.exports = 'bar';
$ yarn
yarn install v0.20.3
[1/4] 🔍  Resolving packages...
success Already up-to-date.
✨  Done in 0.17s.
$ cat node_modules/local/index.js
module.exports = 'foo';
@da70

This comment has been minimized.

Copy link

commented Feb 16, 2017

@akloeber The reason you are seeing this behavior is because in the v0.20.3 code, the directory name only includes a hash if the package's .remote.hash is set:

src/config.js:

    let name = pkg.name;
    let uid = pkg.uid;
    if (pkg.registry) {
      name = `${pkg.registry}-${name}`;
      uid = pkg.version || uid;
    }

    const hash = pkg.remote.hash;

    if (hash) {
      uid += `-${hash}`;
    }

For Github dependencies, pkg.remote.hash is the git commit id hash. For local dependencies, however, such as your example bug case, pkg.remote.hash is null. Here is the code where the info that ends up in the pkg object above is set:

src/resolvers/exotics/file-resolver.js:resolve():

    manifest._remote = {
      type: 'copy',
      registry,
      hash: null,
      reference: loc,
    };

I don't think the hash in this case is meant to be thought of as a hash of package contents, but rather as an identifier provided by the remote registry.

@akloeber

This comment has been minimized.

Copy link

commented Feb 17, 2017

@da70 Thanks for the clarification, that makes sense and solves this issue for deployment. For development I'm using linklocal anyway.

Only the prefix npm- (${pkg.registry}) is kind of missleading. In this case local-, git-, scm- or something else would me more appropriate as this package is not coming from the npm registry.

@da70

This comment has been minimized.

Copy link

commented Feb 17, 2017

@akloeber Yes from the point of view of someone looking at the cache it would probably be easier to have a greater variety of prefixes. I think yarn is considering registry to be a registry format tied to a specific tool. The list of registries is loaded in src/registries/index.js, and there appear to be only two: NPM and yarn.

The fetch request determines the registry for the dependency here in cli/commands/install.js by matching against registry filename, which in the use case you describe is "package.json", which matches the NpmRegistry class.

Maybe in the future there will be a clearer differentiation between package registry and package origin/location.

@JedI-O

This comment has been minimized.

Copy link

commented Jan 10, 2018

11 months later, for me, using yarn 1.3.2 the problem is still there.

Defining a dependency in package.json like so:

{
  "dependencies": {
    "some-dependency": "https://github.com/JedI-O/some-arbitrary-repository.git#mySpecialBranch",
  }
}

Note that there is no versioning!

Running yarn install.
Committing something to the remote repository to branch mySpecialBranch
Running yarn install again - expect that my new commit is being fetched - but it isn't. An old version is used instead.

@bestander

This comment has been minimized.

Copy link
Member

commented Jan 10, 2018

@JedI-O, this is by design.
We don't want Yarn to clone git repos every time one runs yarn install.
So the first time your run it the git repo gets cached and won't update unless you clean cache.

I recommend using npm for fast cacheable updates or use version tags in git if you want to force update.

@JedI-O

This comment has been minimized.

Copy link

commented Jan 10, 2018

Well, I don't want yarn to clone the repo each time I run yarn installeither. But I want yarn to clone the repo each time it has changed. I had assumed that yarn would (or should) compare the latest commits and check whether the latest commit on the remote repo is newer than the latest commit locally installed. If yes: clone again, if not: do nothing. In my opinion, that should at least be an optional behavior.

@bestander

This comment has been minimized.

Copy link
Member

commented Jan 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.