diff --git a/__tests__/commands/_helpers.js b/__tests__/commands/_helpers.js index fa1b21da73..68987908e1 100644 --- a/__tests__/commands/_helpers.js +++ b/__tests__/commands/_helpers.js @@ -65,7 +65,7 @@ export async function run( args: Array, flags: Object, name: string | { source: string, cwd: string }, - checkInstalled: ?(config: Config, reporter: R, install: T) => ?Promise, + checkInstalled: ?(config: Config, reporter: R, install: T, getStdout: () => string) => ?Promise, beforeInstall: ?(cwd: string) => ?Promise, ): Promise { let out = ''; @@ -131,7 +131,7 @@ export async function run( const install = await factory(args, flags, config, reporter, lockfile, () => out); if (checkInstalled) { - await checkInstalled(config, reporter, install); + await checkInstalled(config, reporter, install, () => out); } } catch (err) { throw new Error(`${err && err.stack} \nConsole output:\n ${out}`); diff --git a/__tests__/commands/check.js b/__tests__/commands/check.js index 657052993b..14ade7e5fe 100644 --- a/__tests__/commands/check.js +++ b/__tests__/commands/check.js @@ -21,7 +21,7 @@ const runCheck = buildRun.bind( }, ); -test.concurrent('--verify-tree should report wrong version ', async (): Promise => { +test.concurrent('--verify-tree should report wrong version', async (): Promise => { let thrown = false; try { await runCheck([], {verifyTree: true}, 'verify-tree-version-mismatch'); @@ -31,7 +31,7 @@ test.concurrent('--verify-tree should report wrong version ', async (): Promise< expect(thrown).toEqual(true); }); -test.concurrent('--verify-tree should report missing dependency ', async (): Promise => { +test.concurrent('--verify-tree should report missing dependency', async (): Promise => { let thrown = false; try { await runCheck([], {verifyTree: true}, 'verify-tree-not-found'); @@ -80,6 +80,36 @@ test.concurrent('--integrity should ignore comments and whitespaces in yarn.lock }); }); +test.concurrent('--integrity should fail if integrity file is missing', async (): Promise => { + await runInstall({}, path.join('..', 'check', 'integrity-lock-check'), async (config, reporter): Promise => { + await fs.unlink(path.join(config.cwd, 'node_modules', '.yarn-integrity')); + + let thrown = false; + try { + await checkCmd.run(config, reporter, {integrity: true}, []); + } catch (e) { + thrown = true; + } + expect(thrown).toEqual(true); + }); +}); + +test.concurrent('--integrity should fail if integrity file is not a json', async (): Promise => { + await runInstall({}, path.join('..', 'check', 'integrity-lock-check'), + async (config, reporter, install, getStdout): Promise => { + await fs.writeFile(path.join(config.cwd, 'node_modules', '.yarn-integrity'), 'not a json'); + + let thrown = false; + try { + await checkCmd.run(config, reporter, {integrity: true}, []); + } catch (e) { + thrown = true; + } + expect(thrown).toEqual(true); + expect(getStdout()).toContain('Integrity check: integrity file is not a json'); + }); +}); + test.concurrent('--integrity should fail if yarn.lock has patterns changed', async (): Promise => { await runInstall({}, path.join('..', 'check', 'integrity-lock-check'), async (config, reporter): Promise => { let lockfile = await fs.readFile(path.join(config.cwd, 'yarn.lock')); @@ -97,7 +127,8 @@ test.concurrent('--integrity should fail if yarn.lock has patterns changed', asy }); test.concurrent('--integrity should fail if yarn.lock has new pattern', async (): Promise => { - await runInstall({}, path.join('..', 'check', 'integrity-lock-check'), async (config, reporter): Promise => { + await runInstall({}, path.join('..', 'check', 'integrity-lock-check'), + async (config, reporter, install, getStdout): Promise => { let lockfile = await fs.readFile(path.join(config.cwd, 'yarn.lock')); lockfile += `\nxtend@^4.0.0: version "4.0.1" @@ -111,11 +142,13 @@ test.concurrent('--integrity should fail if yarn.lock has new pattern', async () thrown = true; } expect(thrown).toEqual(true); + expect(getStdout()).toContain('Integrity check: Lock files don\'t match'); }); }); test.concurrent('--integrity should fail if yarn.lock has resolved changed', async (): Promise => { - await runInstall({}, path.join('..', 'check', 'integrity-lock-check'), async (config, reporter): Promise => { + await runInstall({}, path.join('..', 'check', 'integrity-lock-check'), + async (config, reporter, install, getStdout): Promise => { let lockfile = await fs.readFile(path.join(config.cwd, 'yarn.lock')); lockfile = lockfile.replace('https://registry.npmjs.org/left-pad/-/left-pad-1.1.1.tgz', 'https://registry.yarnpkg.com/left-pad/-/left-pad-1.1.1.tgz'); @@ -128,13 +161,14 @@ test.concurrent('--integrity should fail if yarn.lock has resolved changed', asy thrown = true; } expect(thrown).toEqual(true); + expect(getStdout()).toContain('Integrity check: Lock files don\'t match'); }); }); test.concurrent('--integrity should fail if files are missing and --check-files is passed', async (): Promise => { await runInstall({checkFiles: true}, path.join('..', 'check', 'integrity-lock-check'), - async (config, reporter): Promise => { + async (config, reporter, install, getStdout): Promise => { await fs.unlink(path.join(config.cwd, 'node_modules', 'left-pad', 'index.js')); let thrown = false; @@ -144,22 +178,23 @@ async (): Promise => { thrown = true; } expect(thrown).toEqual(true); + expect(getStdout()).toContain('Integrity check: Files are missing'); }); }); -test.concurrent('--integrity should fail if --ignore-scripts is changed', - async (): Promise => { - await runInstall({ignoreScripts: true}, path.join('..', 'check', 'integrity-lock-check'), - async (config, reporter): Promise => { - let thrown = false; - try { - await checkCmd.run(config, reporter, {integrity: true, ignoreScripts: false}, []); - } catch (e) { - thrown = true; - } - expect(thrown).toEqual(true); - }); - }); +test.concurrent('--integrity should fail if --ignore-scripts is changed', async (): Promise => { + await runInstall({ignoreScripts: true}, path.join('..', 'check', 'integrity-lock-check'), + async (config, reporter, install, getStdout): Promise => { + let thrown = false; + try { + await checkCmd.run(config, reporter, {integrity: true, ignoreScripts: false}, []); + } catch (e) { + thrown = true; + } + expect(thrown).toEqual(true); + expect(getStdout()).toContain('Integrity check: Flags don\'t match'); + }); +}); test.concurrent('when switching to --check-files install should rebuild integrity file', async (): Promise => { @@ -204,3 +239,22 @@ async (): Promise => { }); }); + +test.concurrent('--integrity should fail if integrity file have different linkedModules', async (): Promise => { + await runInstall({}, path.join('..', 'check', 'integrity-lock-check'), + async (config, reporter, install, getStdout): Promise => { + const integrityFilePath = path.join(config.cwd, 'node_modules', '.yarn-integrity'); + const integrityFile = JSON.parse(await fs.readFile(integrityFilePath)); + integrityFile.linkedModules.push('aLinkedModule'); + await fs.writeFile(integrityFilePath, JSON.stringify(integrityFile, null, 2)); + + let thrown = false; + try { + await checkCmd.run(config, reporter, {integrity: true}, []); + } catch (e) { + thrown = true; + } + expect(thrown).toEqual(true); + expect(getStdout()).toContain('Integrity check: Linked modules don\'t match'); + }); +}); diff --git a/src/cli/commands/check.js b/src/cli/commands/check.js index c09593c750..47c9b46cf0 100644 --- a/src/cli/commands/check.js +++ b/src/cli/commands/check.js @@ -3,6 +3,7 @@ import type Config from '../../config.js'; import {MessageError} from '../../errors.js'; import InstallationIntegrityChecker from '../../integrity-checker.js'; +import {integrityErrors} from '../../integrity-checker.js'; import Lockfile from '../../lockfile/wrapper.js'; import type {Reporter} from '../../reporters/index.js'; import * as fs from '../../util/fs.js'; @@ -147,7 +148,7 @@ async function integrityHashCheck( reporter.error(reporter.lang(msg, ...vars)); errCount++; } - const integrityChecker = new InstallationIntegrityChecker(config, reporter); + const integrityChecker = new InstallationIntegrityChecker(config); const lockfile = await Lockfile.fromDirectory(config.cwd); const install = new Install(flags, config, reporter, lockfile); @@ -162,7 +163,8 @@ async function integrityHashCheck( if (match.integrityFileMissing) { reportError('noIntegrityFile'); } - if (!match.integrityMatches) { + if (match.integrityMatches === false) { + reporter.warn(reporter.lang(integrityErrors[match.integrityError])); reportError('integrityCheckFailed'); } diff --git a/src/cli/commands/install.js b/src/cli/commands/install.js index 32228cfd26..917ac9dd79 100644 --- a/src/cli/commands/install.js +++ b/src/cli/commands/install.js @@ -172,7 +172,7 @@ export class Install { this.resolver = new PackageResolver(config, lockfile); this.fetcher = new PackageFetcher(config, this.resolver); - this.integrityChecker = new InstallationIntegrityChecker(config, this.reporter); + this.integrityChecker = new InstallationIntegrityChecker(config); this.compatibility = new PackageCompatibility(config, this.resolver, this.flags.ignoreEngines); this.linker = new PackageLinker(config, this.resolver); this.scripts = new PackageInstallScripts(config, this.resolver, this.flags.force); diff --git a/src/integrity-checker.js b/src/integrity-checker.js index 848d645073..238c5df289 100644 --- a/src/integrity-checker.js +++ b/src/integrity-checker.js @@ -3,7 +3,6 @@ import type Config from './config.js'; import type {LockManifest} from './lockfile/wrapper.js'; import type {RegistryNames} from './registries/index.js'; -import type {Reporter} from './reporters/index.js'; import * as constants from './constants.js'; import {registryNames} from './registries/index.js'; import * as fs from './util/fs.js'; @@ -13,9 +12,20 @@ import type {InstallArtifacts} from './package-install-scripts.js'; const invariant = require('invariant'); const path = require('path'); +export const integrityErrors = { + EXPECTED_IS_NOT_A_JSON: 'integrityFailedExpectedIsNotAJSON', + FILES_MISSING: 'integrityFailedFilesMissing', + LOCKFILE_DONT_MATCH: 'integrityLockfilesDontMatch', + FLAGS_DONT_MATCH: 'integrityFlagsDontMatch', + LINKED_MODULES_DONT_MATCH: 'integrityCheckLinkedModulesDontMatch', +}; + +type IntegrityError = $Keys; + export type IntegrityCheckResult = { integrityFileMissing: boolean, integrityMatches?: boolean, + integrityError?: IntegrityError, missingPatterns: Array, }; @@ -47,15 +57,11 @@ type IntegrityFlags = { export default class InstallationIntegrityChecker { constructor( config: Config, - reporter: Reporter, ) { this.config = config; - this.reporter = reporter; } config: Config; - reporter: Reporter; - /** * Get the location of an existing integrity hash. If none exists then return the location where we should @@ -167,32 +173,58 @@ export default class InstallationIntegrityChecker { return result; } - _compareIntegrityFiles(actual: IntegrityFile, expected: IntegrityFile): boolean { - if (!compareSortedArrays(actual.linkedModules, expected.linkedModules)) { - this.reporter.warn(this.reporter.lang('integrityCheckLinkedModulesDontMatch')); - return false; + async _getIntegrityFile(locationPath: string): Promise { + const expectedRaw = await fs.readFile(locationPath); + try { + return JSON.parse(expectedRaw); + } catch (e) { + // ignore JSON parsing for legacy text integrity files compatibility + } + return null; + } + + async _compareIntegrityFiles( + actual: IntegrityFile, + expected: ?IntegrityFile, + checkFiles: boolean, + locationFolder: string): Promise<'OK' | IntegrityError> { + if (!expected) { + return 'EXPECTED_IS_NOT_A_JSON'; } - if (!compareSortedArrays(actual.topLevelPatters, expected.topLevelPatters)) { - this.reporter.warn(this.reporter.lang('integrityPatternsDontMatch')); - return false; + if (!compareSortedArrays(actual.linkedModules, expected.linkedModules)) { + return 'LINKED_MODULES_DONT_MATCH'; } if (!compareSortedArrays(actual.flags, expected.flags)) { - this.reporter.warn(this.reporter.lang('integrityFlagsDontMatch')); - return false; + return 'FLAGS_DONT_MATCH'; } for (const key of Object.keys(actual.lockfileEntries)) { if (actual.lockfileEntries[key] !== expected.lockfileEntries[key]) { - this.reporter.warn(this.reporter.lang('integrityLockfilesDontMatch')); - return false; + return 'LOCKFILE_DONT_MATCH'; } } for (const key of Object.keys(expected.lockfileEntries)) { if (actual.lockfileEntries[key] !== expected.lockfileEntries[key]) { - this.reporter.warn(this.reporter.lang('integrityLockfilesDontMatch')); - return false; + return 'LOCKFILE_DONT_MATCH'; + } + } + if (checkFiles) { + if (expected.files.length === 0) { + // edge case handling - --check-fies is passed but .yarn-integrity does not contain any files + // check and fail if there are file in node_modules after all. + const actualFiles = await this._getFilesDeep(locationFolder); + if (actualFiles.length > 0) { + return 'FILES_MISSING'; + } + } else { + // TODO we may want to optimise this check by checking only for package.json files on very large trees + for (const file of expected.files) { + if (!await fs.exists(path.join(locationFolder, file))) { + return 'FILES_MISSING'; + } + } } } - return true; + return 'OK'; } async check( @@ -214,41 +246,13 @@ export default class InstallationIntegrityChecker { patterns, Object.assign({}, {checkFiles: false}, flags), // don't generate files when checking, we check the files below loc.locationFolder); - const expectedRaw = await fs.readFile(loc.locationPath); - let expected: ?IntegrityFile; - try { - expected = JSON.parse(expectedRaw); - } catch (e) { - // ignore JSON parsing for legacy text integrity files compatibility - } - let integrityMatches; - if (expected) { - integrityMatches = this._compareIntegrityFiles(actual, expected); - if (flags.checkFiles && expected.files.length === 0) { - // edge case handling - --check-fies is passed but .yarn-integrity does not contain any files - // check and fail if there are file in node_modules after all. - const actualFiles = await this._getFilesDeep(loc.locationFolder); - if (actualFiles.length > 0) { - this.reporter.warn(this.reporter.lang('integrityFailedFilesMissing')); - integrityMatches = false; - } - } else if (flags.checkFiles && expected.files.length > 0) { - // TODO we may want to optimise this check by checking only for package.json files on very large trees - for (const file of expected.files) { - if (!await fs.exists(path.join(loc.locationFolder, file))) { - this.reporter.warn(this.reporter.lang('integrityFailedFilesMissing')); - integrityMatches = false; - break; - } - } - } - } else { - integrityMatches = false; - } + const expected = await this._getIntegrityFile(loc.locationPath); + const integrityMatches = await this._compareIntegrityFiles(actual, expected, flags.checkFiles, loc.locationFolder); return { integrityFileMissing: false, - integrityMatches, + integrityMatches: integrityMatches === 'OK', + integrityError: integrityMatches === 'OK' ? undefined : integrityMatches, missingPatterns, }; } @@ -295,5 +299,4 @@ export default class InstallationIntegrityChecker { await fs.unlink(loc.locationPath); } } - } diff --git a/src/reporters/lang/en.js b/src/reporters/lang/en.js index b0fbe904de..d8274679c9 100644 --- a/src/reporters/lang/en.js +++ b/src/reporters/lang/en.js @@ -254,8 +254,8 @@ const messages = { lockfileNotContainPattern: 'Lockfile does not contain pattern: $0', integrityCheckFailed: 'Integrity check failed', noIntegrityFile: 'Couldn\'t find an integrity file', + integrityFailedExpectedIsNotAJSON: 'Integrity check: integrity file is not a json', integrityCheckLinkedModulesDontMatch: 'Integrity check: Linked modules don\'t match', - integrityPatternsDontMatch: 'Integrity check: Patterns don\'t match', integrityFlagsDontMatch: 'Integrity check: Flags don\'t match', integrityLockfilesDontMatch: 'Integrity check: Lock files don\'t match', integrityFailedFilesMissing: 'Integrity check: Files are missing',