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

[Timer]

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.

[bestander]

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

[Timer]

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!

[da70]

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]

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

[bestander]

Thanks, @akloeber.
I'll close this one

[da70]

@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]

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]

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]

@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]

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]

@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.