diff --git a/packages/jest-runtime/package.json b/packages/jest-runtime/package.json index 99f2e6fa924d..ef281b1c2e0a 100644 --- a/packages/jest-runtime/package.json +++ b/packages/jest-runtime/package.json @@ -22,6 +22,7 @@ "json-stable-stringify": "^1.0.1", "micromatch": "^2.3.11", "strip-bom": "3.0.0", + "write-file-atomic": "^2.1.0", "yargs": "^7.0.2" }, "devDependencies": { diff --git a/packages/jest-runtime/src/__tests__/script_transformer.test.js b/packages/jest-runtime/src/__tests__/script_transformer.test.js index c1c1b5d56a20..7201e233e443 100644 --- a/packages/jest-runtime/src/__tests__/script_transformer.test.js +++ b/packages/jest-runtime/src/__tests__/script_transformer.test.js @@ -9,9 +9,11 @@ */ 'use strict'; +const crypto = require('crypto'); const slash = require('slash'); jest + .mock('fs') .mock('graceful-fs') .mock('jest-haste-map', () => ({ getCacheFilePath: (cacheDir, baseDir, version) => cacheDir + baseDir, @@ -100,6 +102,14 @@ let fs; let mockFs; let object; let vm; +let writeFileAtomic; + +jest.mock('write-file-atomic', () => ({ + sync: jest.fn().mockImplementation((filePath, data) => { + const normalizedPath = require('slash')(filePath); + mockFs[normalizedPath] = data; + }), +})); describe('ScriptTransformer', () => { const reset = () => { @@ -145,6 +155,8 @@ describe('ScriptTransformer', () => { fs.existsSync = jest.fn(path => !!mockFs[path]); + writeFileAtomic = require('write-file-atomic'); + config = { cache: true, cacheDirectory: '/cache/', @@ -290,11 +302,10 @@ describe('ScriptTransformer', () => { mapCoverage: true, }); expect(result.sourceMapPath).toEqual(expect.any(String)); - expect(fs.writeFileSync).toBeCalledWith( - result.sourceMapPath, - JSON.stringify(map), - 'utf8', - ); + const mapStr = JSON.stringify(map); + expect(writeFileAtomic.sync).toBeCalledWith(result.sourceMapPath, mapStr, { + encoding: 'utf8', + }); }); it('writes source map if preprocessor inlines it', () => { @@ -320,11 +331,9 @@ describe('ScriptTransformer', () => { mapCoverage: true, }); expect(result.sourceMapPath).toEqual(expect.any(String)); - expect(fs.writeFileSync).toBeCalledWith( - result.sourceMapPath, - sourceMap, - 'utf8', - ); + expect( + writeFileAtomic.sync, + ).toBeCalledWith(result.sourceMapPath, sourceMap, {encoding: 'utf8'}); }); it('does not write source map if mapCoverage option is false', () => { @@ -348,7 +357,7 @@ describe('ScriptTransformer', () => { mapCoverage: false, }); expect(result.sourceMapPath).toBeFalsy(); - expect(fs.writeFileSync).toHaveBeenCalledTimes(1); + expect(writeFileAtomic.sync).toHaveBeenCalledTimes(1); }); it('reads values from the cache', () => { @@ -359,8 +368,8 @@ describe('ScriptTransformer', () => { scriptTransformer.transform('/fruits/banana.js', {}); const cachePath = getCachePath(mockFs, config); - expect(fs.writeFileSync).toBeCalled(); - expect(fs.writeFileSync.mock.calls[0][0]).toBe(cachePath); + expect(writeFileAtomic.sync).toBeCalled(); + expect(writeFileAtomic.sync.mock.calls[0][0]).toBe(cachePath); // Cache the state in `mockFsCopy` const mockFsCopy = mockFs; @@ -375,7 +384,7 @@ describe('ScriptTransformer', () => { expect(fs.readFileSync.mock.calls.length).toBe(2); expect(fs.readFileSync).toBeCalledWith('/fruits/banana.js', 'utf8'); expect(fs.readFileSync).toBeCalledWith(cachePath, 'utf8'); - expect(fs.writeFileSync).not.toBeCalled(); + expect(writeFileAtomic.sync).not.toBeCalled(); // Don't read from the cache when `config.cache` is false. jest.resetModuleRegistry(); @@ -388,6 +397,6 @@ describe('ScriptTransformer', () => { expect(fs.readFileSync.mock.calls.length).toBe(1); expect(fs.readFileSync).toBeCalledWith('/fruits/banana.js', 'utf8'); expect(fs.readFileSync).not.toBeCalledWith(cachePath, 'utf8'); - expect(fs.writeFileSync).toBeCalled(); + expect(writeFileAtomic.sync).toBeCalled(); }); }); diff --git a/packages/jest-runtime/src/script_transformer.js b/packages/jest-runtime/src/script_transformer.js index f476df79fa97..da9b152ab6db 100644 --- a/packages/jest-runtime/src/script_transformer.js +++ b/packages/jest-runtime/src/script_transformer.js @@ -28,6 +28,7 @@ import stableStringify from 'json-stable-stringify'; import slash from 'slash'; import {version as VERSION} from '../package.json'; import shouldInstrument from './should_instrument'; +import writeFileAtomic from 'write-file-atomic'; export type Options = {| collectCoverage: boolean, @@ -42,6 +43,9 @@ const configToJsonMap = new Map(); // Cache regular expressions to test whether the file needs to be preprocessed const ignoreCache: WeakMap = new WeakMap(); +// To reset the cache for specific changesets (rather than package version). +const CACHE_VERSION = '1'; + class ScriptTransformer { static EVAL_RESULT_VARIABLE: string; _config: ProjectConfig; @@ -67,9 +71,15 @@ class ScriptTransformer { const transformer = this._getTransformer(filename); if (transformer && typeof transformer.getCacheKey === 'function') { - return transformer.getCacheKey(fileData, filename, configString, { - instrument, - }); + return crypto + .createHash('md5') + .update( + transformer.getCacheKey(fileData, filename, configString, { + instrument, + }), + ) + .update(CACHE_VERSION) + .digest('hex'); } else { return crypto .createHash('md5') @@ -77,6 +87,7 @@ class ScriptTransformer { .update(configString) .update(instrument ? 'instrument' : '') .update(mapCoverage ? 'mapCoverage' : '') + .update(CACHE_VERSION) .digest('hex'); } } @@ -184,11 +195,13 @@ class ScriptTransformer { ); let sourceMapPath = cacheFilePath + '.map'; // Ignore cache if `config.cache` is set (--no-cache) - let code = this._config.cache - ? readCacheFile(filename, cacheFilePath) - : null; + let code = this._config.cache ? readCodeCacheFile(cacheFilePath) : null; if (code) { + // This is broken: we return the code, and a path for the source map + // directly from the cache. But, nothing ensures the source map actually + // matches that source code. They could have gotten out-of-sync in case + // two separate processes write concurrently to the same cache files. return { code, sourceMapPath, @@ -248,7 +261,7 @@ class ScriptTransformer { sourceMapPath = null; } - writeCacheFile(cacheFilePath, code); + writeCodeCacheFile(cacheFilePath, code); return { code, @@ -343,9 +356,46 @@ const stripShebang = content => { } }; +/** + * This is like `writeCacheFile` but with an additional sanity checksum. We + * cannot use the same technique for source maps because we expose source map + * cache file paths directly to callsites, with the expectation they can read + * it right away. This is not a great system, because source map cache file + * could get corrupted, out-of-sync, etc. + */ +function writeCodeCacheFile(cachePath: Path, code: string) { + const checksum = crypto.createHash('md5').update(code).digest('hex'); + writeCacheFile(cachePath, checksum + '\n' + code); +} + +/** + * Read counterpart of `writeCodeCacheFile`. We verify that the content of the + * file matches the checksum, in case some kind of corruption happened. This + * could happen if an older version of `jest-runtime` writes non-atomically to + * the same cache, for example. + */ +function readCodeCacheFile(cachePath: Path): ?string { + const content = readCacheFile(cachePath); + if (content == null) { + return null; + } + const code = content.substr(33); + const checksum = crypto.createHash('md5').update(code).digest('hex'); + if (checksum === content.substr(0, 32)) { + return code; + } + return null; +} + +/** + * Writing to the cache atomically relies on 'rename' being atomic on most + * file systems. Doing atomic write reduces the risk of corruption by avoiding + * two processes to write to the same file at the same time. It also reduces + * the risk of reading a file that's being overwritten at the same time. + */ const writeCacheFile = (cachePath: Path, fileData: string) => { try { - fs.writeFileSync(cachePath, fileData, 'utf8'); + writeFileAtomic.sync(cachePath, fileData, {encoding: 'utf8'}); } catch (e) { e.message = 'jest: failed to cache transform results in: ' + @@ -357,7 +407,7 @@ const writeCacheFile = (cachePath: Path, fileData: string) => { } }; -const readCacheFile = (filename: Path, cachePath: Path): ?string => { +const readCacheFile = (cachePath: Path): ?string => { if (!fs.existsSync(cachePath)) { return null; } @@ -366,7 +416,11 @@ const readCacheFile = (filename: Path, cachePath: Path): ?string => { try { fileData = fs.readFileSync(cachePath, 'utf8'); } catch (e) { - e.message = 'jest: failed to read cache file: ' + cachePath; + e.message = + 'jest: failed to read cache file: ' + + cachePath + + '\nFailure message: ' + + e.message; removeFile(cachePath); throw e; }