From 343961b070df16c40f04152988b7860bfa737fc5 Mon Sep 17 00:00:00 2001 From: mtraynham Date: Tue, 7 Mar 2017 12:17:45 -0500 Subject: [PATCH 1/3] file-resolver should invalidate cache with a new hash everytime --- package.json | 1 + src/cli/commands/import.js | 3 ++- src/resolvers/exotics/file-resolver.js | 5 +++-- yarn.lock | 2 +- 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index e190e7f940..e7d3ea7ddc 100644 --- a/package.json +++ b/package.json @@ -36,6 +36,7 @@ "strip-bom": "^3.0.0", "tar-fs": "^1.15.1", "tar-stream": "^1.5.2", + "uuid": "^3.0.1", "v8-compile-cache": "^1.1.0", "validate-npm-package-license": "^3.0.1" }, diff --git a/src/cli/commands/import.js b/src/cli/commands/import.js index 998cd857fa..0c8c4eba07 100644 --- a/src/cli/commands/import.js +++ b/src/cli/commands/import.js @@ -25,6 +25,7 @@ const NPM_REGISTRY = /http[s]:\/\/registry.npmjs.org/g; const invariant = require('invariant'); const path = require('path'); +const uuid = require('uuid'); export const noArguments = true; @@ -98,7 +99,7 @@ class ImportResolver extends BaseResolver { info._remote = { type: 'copy', registry: this.registry, - hash: null, + hash: `${uuid.v4()}-${new Date().getTime()}`, reference: loc, }; return info; diff --git a/src/resolvers/exotics/file-resolver.js b/src/resolvers/exotics/file-resolver.js index e0e63a3b21..c22ab3557f 100644 --- a/src/resolvers/exotics/file-resolver.js +++ b/src/resolvers/exotics/file-resolver.js @@ -9,6 +9,7 @@ import * as fs from '../../util/fs.js'; const invariant = require('invariant'); const path = require('path'); +const uuid = require('uuid'); type Dependencies = { [key: string]: string @@ -40,7 +41,7 @@ export default class FileResolver extends ExoticResolver { manifest._remote = { type: 'copy', registry, - hash: null, + hash: `${uuid.v4()}-${new Date().getTime()}`, reference: loc, }; @@ -49,7 +50,7 @@ export default class FileResolver extends ExoticResolver { // Normalize relative paths; if anything changes, make a copy of the manifest const dependencies = this.normalizeDependencyPaths(manifest.dependencies, loc); const optionalDependencies = this.normalizeDependencyPaths(manifest.optionalDependencies, loc); - + if (dependencies !== manifest.dependencies || optionalDependencies !== manifest.optionalDependencies) { const _manifest = Object.assign({}, manifest); if (dependencies != null) { diff --git a/yarn.lock b/yarn.lock index 81fa74be14..2c843b0217 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4519,7 +4519,7 @@ util@0.10.3, util@^0.10.3: dependencies: inherits "2.0.1" -uuid@^3.0.0: +uuid@^3.0.0, uuid@^3.0.1: version "3.0.1" resolved "https://registry.yarnpkg.com/uuid/-/uuid-3.0.1.tgz#6544bba2dfda8c1cf17e629a3a305e2bb1fee6c1" From 93bb137a7ccc3adfbe7162c4072a7050946d6ebd Mon Sep 17 00:00:00 2001 From: Matt Traynham Date: Sun, 12 Mar 2017 09:15:17 -0400 Subject: [PATCH 2/3] Add/update tests for file protocol cache busting with force flag --- __tests__/commands/install/integration.js | 30 ++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/__tests__/commands/install/integration.js b/__tests__/commands/install/integration.js index 5fbc469f89..4900098547 100644 --- a/__tests__/commands/install/integration.js +++ b/__tests__/commands/install/integration.js @@ -268,7 +268,7 @@ test.concurrent('install file: protocol with relative paths', (): Promise }); }); -test.concurrent('install file: protocol without cache', async (): Promise => { +test.concurrent('install file: protocol without force retains installed package', async (): Promise => { const fixturesLoc = path.join(__dirname, '..', '..', 'fixtures', 'install'); const compLoc = path.join(fixturesLoc, 'install-file-without-cache', 'comp', 'index.js'); @@ -280,12 +280,11 @@ test.concurrent('install file: protocol without cache', async (): Promise 'foo\n', ); - await fs.writeFile(compLoc, 'bar\n'); + await fs.writeFile(path.join(config.cwd, 'comp', 'index.js'), 'bar\n'); const reinstall = new Install({}, config, reporter, await Lockfile.fromDirectory(config.cwd)); await reinstall.init(); - // TODO: This should actually be equal. See https://github.com/yarnpkg/yarn/pull/2443. expect( await fs.readFile(path.join(config.cwd, 'node_modules', 'comp', 'index.js')), ).not.toEqual( @@ -294,6 +293,31 @@ test.concurrent('install file: protocol without cache', async (): Promise }); }); +test.concurrent('install file: protocol with force re-installs local package', async (): Promise => { + const fixturesLoc = path.join(__dirname, '..', '..', 'fixtures', 'install'); + const compLoc = path.join(fixturesLoc, 'install-file-without-cache', 'comp', 'index.js'); + + await fs.writeFile(compLoc, 'foo\n'); + await runInstall({}, 'install-file-without-cache', async (config, reporter) => { + expect( + await fs.readFile(path.join(config.cwd, 'node_modules', 'comp', 'index.js')), + ).toEqual( + 'foo\n', + ); + + await fs.writeFile(path.join(config.cwd, 'comp', 'index.js'), 'bar\n'); + + const reinstall = new Install({force: true}, config, reporter, await Lockfile.fromDirectory(config.cwd)); + await reinstall.init(); + + expect( + await fs.readFile(path.join(config.cwd, 'node_modules', 'comp', 'index.js')), + ).toEqual( + 'bar\n', + ); + }); +}); + test.concurrent('install file: local packages with local dependencies', async (): Promise => { await runInstall({}, 'install-file-local-dependency', async (config, reporter) => { const reinstall = new Install({}, config, reporter, await Lockfile.fromDirectory(config.cwd)); From 645315b5d32f8444450ba231d8fa264bf52e0246 Mon Sep 17 00:00:00 2001 From: mtraynham Date: Mon, 24 Apr 2017 15:10:28 -0400 Subject: [PATCH 3/3] Fixing cache test and adding comment on size of cache directory. --- __tests__/commands/cache.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/__tests__/commands/cache.js b/__tests__/commands/cache.js index 433ecf9052..fbdfa66a8e 100644 --- a/__tests__/commands/cache.js +++ b/__tests__/commands/cache.js @@ -51,7 +51,14 @@ test('dir', async (): Promise => { test('clean', async (): Promise => { await runInstall({}, 'artifacts-finds-and-saves', async (config): Promise => { let files = await fs.readdir(config.cacheFolder); - expect(files.length).toEqual(2); // we need to add one .tmp folder + // Asserting cache size is 1... + // we need to add one for the .tmp folder + // + // Per #2860, file: protocol installs may add the same package to the cache + // multiple times if it is installed with a force flag or has an install script. + // We'll add another for a total of 3 because this particular fixture has + // an install script. + expect(files.length).toEqual(3); const out = new stream.PassThrough(); const reporter = new reporters.JSONReporter({stdout: out});