From 6bfaac3f1cb0b26e06110eeacee0ccbd7f924327 Mon Sep 17 00:00:00 2001 From: Jamie Date: Thu, 17 Aug 2017 22:12:46 +1200 Subject: [PATCH] Bugfix 3752 - Artifacts disappearing when missing integrity file (#4185) * Add method to update force property * Update install to force script installs when integrity file missing * Update add to force script installs when integrity file missing * Add test case for retaining artifacts after add when missing integrity * Add test case for retaining artifacts after install when missing integrity * Add test fixture data for missing integrity tests * Fix lint errors --- __tests__/commands/add.js | 31 +++++++++++++++++++ __tests__/commands/install/lockfiles.js | 18 +++++++++++ .../a/install.js | 1 + .../a/package.json | 7 +++++ .../b/package.json | 4 +++ .../package.json | 1 + .../a/install.js | 1 + .../a/package.json | 7 +++++ .../package.json | 5 +++ .../yarn.lock | 6 ++++ src/cli/commands/add.js | 18 +++++++++-- src/cli/commands/install.js | 6 ++++ src/package-install-scripts.js | 4 +++ 13 files changed, 107 insertions(+), 2 deletions(-) create mode 100644 __tests__/fixtures/add/retain-build-artifacts-missing-integrity/a/install.js create mode 100644 __tests__/fixtures/add/retain-build-artifacts-missing-integrity/a/package.json create mode 100644 __tests__/fixtures/add/retain-build-artifacts-missing-integrity/b/package.json create mode 100644 __tests__/fixtures/add/retain-build-artifacts-missing-integrity/package.json create mode 100644 __tests__/fixtures/install/install-should-retain-artifacts-when-missing-integrity/a/install.js create mode 100644 __tests__/fixtures/install/install-should-retain-artifacts-when-missing-integrity/a/package.json create mode 100644 __tests__/fixtures/install/install-should-retain-artifacts-when-missing-integrity/package.json create mode 100644 __tests__/fixtures/install/install-should-retain-artifacts-when-missing-integrity/yarn.lock diff --git a/__tests__/commands/add.js b/__tests__/commands/add.js index 824c0df58e..59e552ee9e 100644 --- a/__tests__/commands/add.js +++ b/__tests__/commands/add.js @@ -848,6 +848,37 @@ test.concurrent('should only refer to root to satisfy peer dependency', (): Prom ); }); +test.concurrent('should retain build artifacts after add when missing integrity file', (): Promise => { + return buildRun( + reporters.BufferReporter, + fixturesLoc, + async (args, flags, config, reporter): Promise => { + const lockfile = await createLockfile(config.cwd); + + const addA = new Add(args, flags, config, reporter, lockfile); + await addA.init(); + + const expectedArtifacts = ['foo.txt']; + const integrityLoc = path.join(config.cwd, 'node_modules', constants.INTEGRITY_FILENAME); + + const beforeIntegrity = await fs.readJson(integrityLoc); + expect(beforeIntegrity.artifacts['a@0.0.0']).toEqual(expectedArtifacts); + + await fs.unlink(integrityLoc); + + const lockfileAfterPreviousAdd = await Lockfile.fromDirectory(config.cwd); + const addB = new Add(['file:b'], flags, config, reporter, lockfileAfterPreviousAdd); + await addB.init(); + + const afterIntegrity = await fs.readJson(integrityLoc); + expect(afterIntegrity.artifacts['a@0.0.0']).toEqual(expectedArtifacts); + }, + ['file:a'], + {}, + 'retain-build-artifacts-missing-integrity', + ); +}); + test.concurrent('should retain build artifacts after add', (): Promise => { return buildRun( reporters.BufferReporter, diff --git a/__tests__/commands/install/lockfiles.js b/__tests__/commands/install/lockfiles.js index 7302560cce..80ac964196 100644 --- a/__tests__/commands/install/lockfiles.js +++ b/__tests__/commands/install/lockfiles.js @@ -195,6 +195,24 @@ test.concurrent('install should write and read integrity file based on lockfile }); }); +test.concurrent('install should retain artifacts when missing integrity file', (): Promise => { + return runInstall({}, 'install-should-retain-artifacts-when-missing-integrity', async (config, reporter) => { + const expectedArtifacts = ['foo.txt']; + const integrityLoc = path.join(config.cwd, 'node_modules', constants.INTEGRITY_FILENAME); + + const beforeIntegrity = await fs.readJson(integrityLoc); + expect(beforeIntegrity.artifacts['a@0.0.0']).toEqual(expectedArtifacts); + + await fs.unlink(integrityLoc); + + const reinstall = new Install({}, config, reporter, await Lockfile.fromDirectory(config.cwd)); + await reinstall.init(); + + const afterIntegrity = await fs.readJson(integrityLoc); + expect(afterIntegrity.artifacts['a@0.0.0']).toEqual(expectedArtifacts); + }); +}); + test.concurrent('install should not continue if integrity check passes', (): Promise => { return runInstall({}, 'lockfile-stability', async (config, reporter) => { await fs.writeFile(path.join(config.cwd, 'node_modules', 'yarn.test'), 'YARN TEST'); diff --git a/__tests__/fixtures/add/retain-build-artifacts-missing-integrity/a/install.js b/__tests__/fixtures/add/retain-build-artifacts-missing-integrity/a/install.js new file mode 100644 index 0000000000..899265a0eb --- /dev/null +++ b/__tests__/fixtures/add/retain-build-artifacts-missing-integrity/a/install.js @@ -0,0 +1 @@ +require('fs').writeFileSync('foo.txt', 'foobar'); diff --git a/__tests__/fixtures/add/retain-build-artifacts-missing-integrity/a/package.json b/__tests__/fixtures/add/retain-build-artifacts-missing-integrity/a/package.json new file mode 100644 index 0000000000..fcfc10796f --- /dev/null +++ b/__tests__/fixtures/add/retain-build-artifacts-missing-integrity/a/package.json @@ -0,0 +1,7 @@ +{ + "name": "a", + "version": "0.0.0", + "scripts": { + "postinstall": "node install.js" + } +} diff --git a/__tests__/fixtures/add/retain-build-artifacts-missing-integrity/b/package.json b/__tests__/fixtures/add/retain-build-artifacts-missing-integrity/b/package.json new file mode 100644 index 0000000000..1d02201279 --- /dev/null +++ b/__tests__/fixtures/add/retain-build-artifacts-missing-integrity/b/package.json @@ -0,0 +1,4 @@ +{ + "name": "b", + "version": "0.0.0" +} diff --git a/__tests__/fixtures/add/retain-build-artifacts-missing-integrity/package.json b/__tests__/fixtures/add/retain-build-artifacts-missing-integrity/package.json new file mode 100644 index 0000000000..0967ef424b --- /dev/null +++ b/__tests__/fixtures/add/retain-build-artifacts-missing-integrity/package.json @@ -0,0 +1 @@ +{} diff --git a/__tests__/fixtures/install/install-should-retain-artifacts-when-missing-integrity/a/install.js b/__tests__/fixtures/install/install-should-retain-artifacts-when-missing-integrity/a/install.js new file mode 100644 index 0000000000..899265a0eb --- /dev/null +++ b/__tests__/fixtures/install/install-should-retain-artifacts-when-missing-integrity/a/install.js @@ -0,0 +1 @@ +require('fs').writeFileSync('foo.txt', 'foobar'); diff --git a/__tests__/fixtures/install/install-should-retain-artifacts-when-missing-integrity/a/package.json b/__tests__/fixtures/install/install-should-retain-artifacts-when-missing-integrity/a/package.json new file mode 100644 index 0000000000..fcfc10796f --- /dev/null +++ b/__tests__/fixtures/install/install-should-retain-artifacts-when-missing-integrity/a/package.json @@ -0,0 +1,7 @@ +{ + "name": "a", + "version": "0.0.0", + "scripts": { + "postinstall": "node install.js" + } +} diff --git a/__tests__/fixtures/install/install-should-retain-artifacts-when-missing-integrity/package.json b/__tests__/fixtures/install/install-should-retain-artifacts-when-missing-integrity/package.json new file mode 100644 index 0000000000..4e3bb4400b --- /dev/null +++ b/__tests__/fixtures/install/install-should-retain-artifacts-when-missing-integrity/package.json @@ -0,0 +1,5 @@ +{ + "dependencies": { + "a": "file:a" + } +} diff --git a/__tests__/fixtures/install/install-should-retain-artifacts-when-missing-integrity/yarn.lock b/__tests__/fixtures/install/install-should-retain-artifacts-when-missing-integrity/yarn.lock new file mode 100644 index 0000000000..1bba7514a3 --- /dev/null +++ b/__tests__/fixtures/install/install-should-retain-artifacts-when-missing-integrity/yarn.lock @@ -0,0 +1,6 @@ +# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. +# yarn lockfile v1 + + +"a@file:a": + version "0.0.0" diff --git a/src/cli/commands/add.js b/src/cli/commands/add.js index 8b854098ac..754781eb37 100644 --- a/src/cli/commands/add.js +++ b/src/cli/commands/add.js @@ -7,12 +7,16 @@ import type Config from '../../config.js'; import type {ListOptions} from './list.js'; import Lockfile from '../../lockfile/wrapper.js'; import PackageRequest from '../../package-request.js'; +import WorkspaceLayout from '../../workspace-layout.js'; import {getExoticResolver} from '../../resolvers/index.js'; import {buildTree} from './list.js'; import {wrapLifecycle, Install} from './install.js'; import {MessageError} from '../../errors.js'; +import * as constants from '../../constants.js'; +import * as fs from '../../util/fs.js'; import invariant from 'invariant'; +import path from 'path'; import semver from 'semver'; export class Add extends Install { @@ -97,8 +101,18 @@ export class Add extends Install { return preparedPatterns; } - bailout(patterns: Array): Promise { - return Promise.resolve(false); + async bailout(patterns: Array, workspaceLayout: ?WorkspaceLayout): Promise { + const lockfileCache = this.lockfile.cache; + if (!lockfileCache) { + return false; + } + const match = await this.integrityChecker.check(patterns, lockfileCache, this.flags, workspaceLayout); + const haveLockfile = await fs.exists(path.join(this.config.lockfileFolder, constants.LOCKFILE_FILENAME)); + if (match.integrityFileMissing && haveLockfile) { + // Integrity file missing, force script installations + this.scripts.setForce(true); + } + return false; } /** diff --git a/src/cli/commands/install.js b/src/cli/commands/install.js index 53dc1cd935..4558cfae70 100644 --- a/src/cli/commands/install.js +++ b/src/cli/commands/install.js @@ -359,6 +359,12 @@ export class Install { return true; } + if (match.integrityFileMissing && haveLockfile) { + // Integrity file missing, force script installations + this.scripts.setForce(true); + return false; + } + if (!patterns.length && !match.integrityFileMissing) { this.reporter.success(this.reporter.lang('nothingToInstall')); await this.createEmptyManifestFolders(); diff --git a/src/package-install-scripts.js b/src/package-install-scripts.js index 948e920783..7b98d841be 100644 --- a/src/package-install-scripts.js +++ b/src/package-install-scripts.js @@ -34,6 +34,10 @@ export default class PackageInstallScripts { force: boolean; artifacts: InstallArtifacts; + setForce(force: boolean) { + this.force = force; + } + setArtifacts(artifacts: InstallArtifacts) { this.artifacts = artifacts; }