From 95da1563aba9f92defd69b78b6e89b5d5840db25 Mon Sep 17 00:00:00 2001 From: Alexander Marks Date: Fri, 8 Apr 2022 12:13:27 -0700 Subject: [PATCH 1/9] Rename CacheKey to ScriptState --- src/caching/cache.ts | 6 +-- src/caching/local-cache.ts | 8 ++-- src/executor.ts | 93 ++++++++++++++++++-------------------- src/script.ts | 12 ++--- 4 files changed, 58 insertions(+), 61 deletions(-) diff --git a/src/caching/cache.ts b/src/caching/cache.ts index 254c458a5..620200232 100644 --- a/src/caching/cache.ts +++ b/src/caching/cache.ts @@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import type {CacheKeyString, ScriptReference} from '../script.js'; +import type {ScriptStateString, ScriptReference} from '../script.js'; /** * Saves and restores output files to some cache store (e.g. local disk or @@ -23,7 +23,7 @@ export interface Cache { */ get( script: ScriptReference, - cacheKey: CacheKeyString + cacheKey: ScriptStateString ): Promise; /** @@ -37,7 +37,7 @@ export interface Cache { */ set( script: ScriptReference, - cacheKey: CacheKeyString, + cacheKey: ScriptStateString, relativeFilePaths: string[] ): Promise; } diff --git a/src/caching/local-cache.ts b/src/caching/local-cache.ts index 4a886b092..168b63624 100644 --- a/src/caching/local-cache.ts +++ b/src/caching/local-cache.ts @@ -11,7 +11,7 @@ import {getScriptDataDir} from '../util/script-data-dir.js'; import {optimizeCopies, optimizeMkdirs} from '../util/optimize-fs-ops.js'; import type {Cache, CacheHit} from './cache.js'; -import type {ScriptReference, CacheKeyString} from '../script.js'; +import type {ScriptReference, ScriptStateString} from '../script.js'; /** * Caches script output to each package's @@ -20,7 +20,7 @@ import type {ScriptReference, CacheKeyString} from '../script.js'; export class LocalCache implements Cache { async get( script: ScriptReference, - cacheKey: CacheKeyString + cacheKey: ScriptStateString ): Promise { const cacheDir = this.#getCacheDir(script, cacheKey); try { @@ -36,7 +36,7 @@ export class LocalCache implements Cache { async set( script: ScriptReference, - cacheKey: CacheKeyString, + cacheKey: ScriptStateString, relativeFiles: string[] ): Promise { // TODO(aomarks) A script's cache directory currently just grows forever. @@ -86,7 +86,7 @@ export class LocalCache implements Cache { ); } - #getCacheDir(script: ScriptReference, cacheKey: CacheKeyString): string { + #getCacheDir(script: ScriptReference, cacheKey: ScriptStateString): string { return pathlib.join( getScriptDataDir(script), 'cache', diff --git a/src/executor.ts b/src/executor.ts index 0e1228c4f..e15909128 100644 --- a/src/executor.ts +++ b/src/executor.ts @@ -20,8 +20,8 @@ import type { ScriptConfig, ScriptReference, ScriptReferenceString, - CacheKey, - CacheKeyString, + ScriptState, + ScriptStateString, Sha256HexDigest, } from './script.js'; import type {Logger} from './logging/logger.js'; @@ -65,7 +65,7 @@ const IS_WINDOWS = process.platform === 'win32'; export class Executor { readonly #executions = new Map< string, - Promise + Promise >(); readonly #logger: Logger; readonly #workerPool: WorkerPool; @@ -81,9 +81,11 @@ export class Executor { this.#cache = cache; } - async execute(script: ScriptConfig): Promise { - const cacheKey = scriptReferenceToString(script); - let promise = this.#executions.get(cacheKey); + async execute( + script: ScriptConfig + ): Promise { + const executionKey = scriptReferenceToString(script); + let promise = this.#executions.get(executionKey); if (promise === undefined) { promise = ScriptExecution.execute( script, @@ -92,7 +94,7 @@ export class Executor { this.#cache, this.#logger ); - this.#executions.set(cacheKey, promise); + this.#executions.set(executionKey, promise); } return promise; } @@ -108,7 +110,7 @@ class ScriptExecution { workerPool: WorkerPool, cache: Cache | undefined, logger: Logger - ): Promise { + ): Promise { return new ScriptExecution( script, executor, @@ -138,20 +140,17 @@ class ScriptExecution { this.#logger = logger; } - async #execute(): Promise { - const dependencyCacheKeys = await this.#executeDependencies(); + async #execute(): Promise { + const dependencyStates = await this.#executeDependencies(); // Note we must wait for dependencies to finish before generating the cache // key, because a dependency could create or modify an input file to this // script, which would affect the key. - const cacheKey = await this.#getCacheKey(dependencyCacheKeys); - let cacheKeyStr: CacheKeyString | typeof UNCACHEABLE; - if (cacheKey !== UNCACHEABLE) { - cacheKeyStr = JSON.stringify(cacheKey) as CacheKeyString; - const previousCacheKeyStr = await this.#readStateFile(); - if ( - previousCacheKeyStr !== undefined && - cacheKeyStr === previousCacheKeyStr - ) { + const state = await this.#computeState(dependencyStates); + let stateStr: ScriptStateString | typeof UNCACHEABLE; + if (state !== UNCACHEABLE) { + stateStr = JSON.stringify(state) as ScriptStateString; + const prevStateStr = await this.#readPreviousState(); + if (prevStateStr !== undefined && stateStr === prevStateStr) { // TODO(aomarks) Does not preserve original order of stdout vs stderr // chunks. See https://github.com/google/wireit/issues/74. await Promise.all([ @@ -163,10 +162,10 @@ class ScriptExecution { type: 'success', reason: 'fresh', }); - return cacheKey; + return state; } } else { - cacheKeyStr = UNCACHEABLE; + stateStr = UNCACHEABLE; } // It's important that we delete any previous state before running the @@ -175,8 +174,8 @@ class ScriptExecution { await this.#prepareDataDir(); const cacheHit = - cacheKeyStr !== UNCACHEABLE - ? await this.#cache?.get(this.#script, cacheKeyStr) + stateStr !== UNCACHEABLE + ? await this.#cache?.get(this.#script, stateStr) : undefined; // The "clean" setting controls whether we delete output before execution. @@ -212,21 +211,21 @@ class ScriptExecution { await this.#executeCommandIfNeeded(); } - if (cacheKeyStr !== UNCACHEABLE) { + if (stateStr !== UNCACHEABLE) { // TODO(aomarks) We don't technically need to wait for these to finish to // return, we only need to wait in the top-level call to execute. The same // will go for saving output to the cache. - await this.#writeStateFile(cacheKeyStr); + await this.#writeStateFile(stateStr); if (cacheHit === undefined) { - await this.#saveToCacheIfPossible(cacheKeyStr); + await this.#saveToCacheIfPossible(stateStr); } } - return cacheKey; + return state; } async #executeDependencies(): Promise< - Array<[ScriptReference, CacheKey | typeof UNCACHEABLE]> + Array<[ScriptReference, ScriptState | typeof UNCACHEABLE]> > { // Randomize the order we execute dependencies to make it less likely for a // user to inadvertently depend on any specific order, which could indicate @@ -240,7 +239,8 @@ class ScriptExecution { ) ); const errors: unknown[] = []; - const results: Array<[ScriptReference, CacheKey | typeof UNCACHEABLE]> = []; + const results: Array<[ScriptReference, ScriptState | typeof UNCACHEABLE]> = + []; for (let i = 0; i < dependencyResults.length; i++) { const result = dependencyResults[i]; if (result.status === 'rejected') { @@ -419,10 +419,10 @@ class ScriptExecution { * Save the current output files to the configured cache if possible. */ async #saveToCacheIfPossible( - cacheKeyStr: CacheKeyString | typeof UNCACHEABLE + stateStr: ScriptStateString | typeof UNCACHEABLE ): Promise { if ( - cacheKeyStr === UNCACHEABLE || + stateStr === UNCACHEABLE || this.#cache === undefined || this.#script.output === undefined ) { @@ -430,7 +430,7 @@ class ScriptExecution { } await this.#cache.set( this.#script, - cacheKeyStr, + stateStr, await this.#glob( [ ...this.#script.output, @@ -471,9 +471,9 @@ class ScriptExecution { * Returns the sentinel value {@link UNCACHEABLE} if this script, or any of * this script's transitive dependencies, have undefined input files. */ - async #getCacheKey( - dependencyCacheKeys: Array<[ScriptReference, CacheKey | typeof UNCACHEABLE]> - ): Promise { + async #computeState( + dependencyStates: Array<[ScriptReference, ScriptState | typeof UNCACHEABLE]> + ): Promise { if ( this.#script.files === undefined && this.#script.command !== undefined @@ -489,19 +489,16 @@ class ScriptExecution { return UNCACHEABLE; } - const filteredDependencyCacheKeys: Array< - [ScriptReferenceString, CacheKey] + const filteredDependencyStates: Array< + [ScriptReferenceString, ScriptState] > = []; - for (const [dep, depCacheKey] of dependencyCacheKeys) { - if (depCacheKey === UNCACHEABLE) { + for (const [dep, depState] of dependencyStates) { + if (depState === UNCACHEABLE) { // If one of our dependencies is uncacheable, then we're uncacheable // too, because that dependency could have an effect on our output. return UNCACHEABLE; } - filteredDependencyCacheKeys.push([ - scriptReferenceToString(dep), - depCacheKey, - ]); + filteredDependencyStates.push([scriptReferenceToString(dep), depState]); } let fileHashes: Array<[string, Sha256HexDigest]>; @@ -539,7 +536,7 @@ class ScriptExecution { ), output: this.#script.output ?? [], dependencies: Object.fromEntries( - filteredDependencyCacheKeys.sort(([aRef], [bRef]) => + filteredDependencyStates.sort(([aRef], [bRef]) => aRef.localeCompare(bRef) ) ), @@ -577,9 +574,9 @@ class ScriptExecution { /** * Read this script's ".wireit//state" file. */ - async #readStateFile(): Promise { + async #readPreviousState(): Promise { try { - return (await fs.readFile(this.#statePath, 'utf8')) as CacheKeyString; + return (await fs.readFile(this.#statePath, 'utf8')) as ScriptStateString; } catch (error) { if ((error as {code?: string}).code === 'ENOENT') { return undefined; @@ -591,9 +588,9 @@ class ScriptExecution { /** * Write this script's ".wireit//state" file. */ - async #writeStateFile(cacheKeyStr: CacheKeyString): Promise { + async #writeStateFile(stateStr: ScriptStateString): Promise { await fs.mkdir(this.#dataDir, {recursive: true}); - await fs.writeFile(this.#statePath, cacheKeyStr, 'utf8'); + await fs.writeFile(this.#statePath, stateStr, 'utf8'); } /** diff --git a/src/script.ts b/src/script.ts index 3c82f0e6f..6ee0dea2a 100644 --- a/src/script.ts +++ b/src/script.ts @@ -94,7 +94,7 @@ export type ScriptReferenceString = string & { * All meaningful input state of a script. Used for determining if a script is * fresh, and as the key for storing cached output. */ -export interface CacheKey { +export interface ScriptState { command: string | undefined; /** @@ -123,19 +123,19 @@ export interface CacheKey { output: string[]; // Must be sorted. - dependencies: {[dependency: ScriptReferenceString]: CacheKey}; + dependencies: {[dependency: ScriptReferenceString]: ScriptState}; } /** - * String serialization of a {@link CacheKey}. + * String serialization of a {@link ScriptState}. */ -export type CacheKeyString = string & { - __CacheKeyStringBrand__: never; +export type ScriptStateString = string & { + __ScriptStateStringBrand__: never; }; /** * SHA256 hash hexadecimal digest of a file's content. */ export type Sha256HexDigest = string & { - __Sha256HexDigest__: never; + __Sha256HexDigestBrand__: never; }; From 5f5c3e21a77cef941cda9b21bc8fdc92a69a405d Mon Sep 17 00:00:00 2001 From: Alexander Marks Date: Fri, 8 Apr 2022 12:36:05 -0700 Subject: [PATCH 2/9] Replace UNCACHEABLE sentinel with uncacheable state bit --- src/executor.ts | 106 ++++++++++++++++++------------------------------ src/script.ts | 11 +++++ 2 files changed, 50 insertions(+), 67 deletions(-) diff --git a/src/executor.ts b/src/executor.ts index e15909128..284f36c78 100644 --- a/src/executor.ts +++ b/src/executor.ts @@ -28,13 +28,6 @@ import type {Logger} from './logging/logger.js'; import type {WriteStream} from 'fs'; import type {Cache} from './caching/cache.js'; -/** - * Unique symbol to represent a script that isn't safe to be cached, because its - * input files, or the input files of one of its transitive dependencies, are - * undefined. - */ -const UNCACHEABLE = Symbol(); - /** * The PATH environment variable of this process, minus all of the leading * "node_modules/.bin" entries that the incoming "npm run" command already set. @@ -63,10 +56,7 @@ const IS_WINDOWS = process.platform === 'win32'; * Executes a script that has been analyzed and validated by the Analyzer. */ export class Executor { - readonly #executions = new Map< - string, - Promise - >(); + readonly #executions = new Map>(); readonly #logger: Logger; readonly #workerPool: WorkerPool; readonly #cache?: Cache; @@ -81,9 +71,7 @@ export class Executor { this.#cache = cache; } - async execute( - script: ScriptConfig - ): Promise { + async execute(script: ScriptConfig): Promise { const executionKey = scriptReferenceToString(script); let promise = this.#executions.get(executionKey); if (promise === undefined) { @@ -110,7 +98,7 @@ class ScriptExecution { workerPool: WorkerPool, cache: Cache | undefined, logger: Logger - ): Promise { + ): Promise { return new ScriptExecution( script, executor, @@ -140,15 +128,14 @@ class ScriptExecution { this.#logger = logger; } - async #execute(): Promise { + async #execute(): Promise { const dependencyStates = await this.#executeDependencies(); // Note we must wait for dependencies to finish before generating the cache // key, because a dependency could create or modify an input file to this // script, which would affect the key. const state = await this.#computeState(dependencyStates); - let stateStr: ScriptStateString | typeof UNCACHEABLE; - if (state !== UNCACHEABLE) { - stateStr = JSON.stringify(state) as ScriptStateString; + const stateStr = JSON.stringify(state) as ScriptStateString; + if (state.cacheable) { const prevStateStr = await this.#readPreviousState(); if (prevStateStr !== undefined && stateStr === prevStateStr) { // TODO(aomarks) Does not preserve original order of stdout vs stderr @@ -164,8 +151,6 @@ class ScriptExecution { }); return state; } - } else { - stateStr = UNCACHEABLE; } // It's important that we delete any previous state before running the @@ -173,10 +158,9 @@ class ScriptExecution { // don't want to think that the previous state is still valid. await this.#prepareDataDir(); - const cacheHit = - stateStr !== UNCACHEABLE - ? await this.#cache?.get(this.#script, stateStr) - : undefined; + const cacheHit = state.cacheable + ? await this.#cache?.get(this.#script, stateStr) + : undefined; // The "clean" setting controls whether we delete output before execution. // @@ -211,22 +195,18 @@ class ScriptExecution { await this.#executeCommandIfNeeded(); } - if (stateStr !== UNCACHEABLE) { - // TODO(aomarks) We don't technically need to wait for these to finish to - // return, we only need to wait in the top-level call to execute. The same - // will go for saving output to the cache. - await this.#writeStateFile(stateStr); - if (cacheHit === undefined) { - await this.#saveToCacheIfPossible(stateStr); - } + // TODO(aomarks) We don't technically need to wait for these to finish to + // return, we only need to wait in the top-level call to execute. The same + // will go for saving output to the cache. + await this.#writeStateFile(stateStr); + if (cacheHit === undefined && state.cacheable) { + await this.#saveToCacheIfPossible(stateStr); } return state; } - async #executeDependencies(): Promise< - Array<[ScriptReference, ScriptState | typeof UNCACHEABLE]> - > { + async #executeDependencies(): Promise> { // Randomize the order we execute dependencies to make it less likely for a // user to inadvertently depend on any specific order, which could indicate // a missing edge in the dependency graph. @@ -239,8 +219,7 @@ class ScriptExecution { ) ); const errors: unknown[] = []; - const results: Array<[ScriptReference, ScriptState | typeof UNCACHEABLE]> = - []; + const results: Array<[ScriptReference, ScriptState]> = []; for (let i = 0; i < dependencyResults.length; i++) { const result = dependencyResults[i]; if (result.status === 'rejected') { @@ -418,14 +397,8 @@ class ScriptExecution { /** * Save the current output files to the configured cache if possible. */ - async #saveToCacheIfPossible( - stateStr: ScriptStateString | typeof UNCACHEABLE - ): Promise { - if ( - stateStr === UNCACHEABLE || - this.#cache === undefined || - this.#script.output === undefined - ) { + async #saveToCacheIfPossible(stateStr: ScriptStateString): Promise { + if (this.#cache === undefined || this.#script.output === undefined) { return; } await this.#cache.set( @@ -472,31 +445,15 @@ class ScriptExecution { * this script's transitive dependencies, have undefined input files. */ async #computeState( - dependencyStates: Array<[ScriptReference, ScriptState | typeof UNCACHEABLE]> - ): Promise { - if ( - this.#script.files === undefined && - this.#script.command !== undefined - ) { - // If files are undefined, then it's never safe for us to be cached, - // because we don't know what the inputs are, so we can't know if the - // output of this script could change. - // - // However, if the command is also undefined, then it actually _is_ safe - // to be cached, because the script isn't itself going to do anything - // anyway. In that case, the cache keys will be purely the cache keys of - // the dependencies. - return UNCACHEABLE; - } - + dependencyStates: Array<[ScriptReference, ScriptState]> + ): Promise { + let allDependenciesAreCacheable = true; const filteredDependencyStates: Array< [ScriptReferenceString, ScriptState] > = []; for (const [dep, depState] of dependencyStates) { - if (depState === UNCACHEABLE) { - // If one of our dependencies is uncacheable, then we're uncacheable - // too, because that dependency could have an effect on our output. - return UNCACHEABLE; + if (!depState.cacheable) { + allDependenciesAreCacheable = false; } filteredDependencyStates.push([scriptReferenceToString(dep), depState]); } @@ -528,7 +485,22 @@ class ScriptExecution { fileHashes = []; } + const cacheable = + // If command is undefined, then it's always safe to be cached, because + // the script isn't going to do anything anyway. In these cases, the cache + // keys are essentially just the cache keys of the dependencies. + this.#script.command === undefined || + // Otherwise, If files are undefined, then it's not safe to be cached, + // because we don't know what the inputs are, so we can't know if the + // output of this script could change. + (this.#script.files !== undefined && + // Similarly, if any of our dependencies are uncacheable, then we're + // uncacheable too, because that dependency could also have an effect on + // our output. + allDependenciesAreCacheable); + return { + cacheable, command: this.#script.command, clean: this.#script.clean, files: Object.fromEntries( diff --git a/src/script.ts b/src/script.ts index 6ee0dea2a..66f384711 100644 --- a/src/script.ts +++ b/src/script.ts @@ -95,6 +95,17 @@ export type ScriptReferenceString = string & { * fresh, and as the key for storing cached output. */ export interface ScriptState { + /** + * Whether the output for this script can be fresh or cached. + * + * True only if the "files" array was defined for this script, and for all of + * this script's transitive dependencies. + */ + cacheable: boolean; + + /** + * The shell command from the Wireit config. + */ command: string | undefined; /** From 916e34e91f24548c739069950043b237d782bf6f Mon Sep 17 00:00:00 2001 From: Alexander Marks Date: Fri, 8 Apr 2022 08:34:52 -0700 Subject: [PATCH 3/9] Support clean:if-file-deleted in Analyzer --- src/analyzer.ts | 32 ++++++++++++-------------------- src/script.ts | 12 ++++++++---- src/test/errors-analysis.test.ts | 4 ++-- 3 files changed, 22 insertions(+), 26 deletions(-) diff --git a/src/analyzer.ts b/src/analyzer.ts index d31a83d01..88a8e5bdd 100644 --- a/src/analyzer.ts +++ b/src/analyzer.ts @@ -234,8 +234,18 @@ export class Analyzer { } } - if (wireitConfig?.clean !== undefined) { - assertBoolean(placeholder, wireitConfig.clean, 'clean'); + if ( + wireitConfig?.clean !== undefined && + wireitConfig.clean !== true && + wireitConfig.clean !== false && + wireitConfig.clean !== 'if-file-deleted' + ) { + throw new WireitError({ + script: placeholder, + type: 'failure', + reason: 'invalid-config-syntax', + message: `clean must be true, false, or "if-file-deleted"`, + }); } if (wireitConfig?.packageLocks !== undefined) { @@ -426,24 +436,6 @@ const assertString = ( } }; -/** - * Throw an error if the given value is not a boolean. - */ -const assertBoolean = ( - script: ScriptReference, - value: unknown, - name: string -) => { - if (typeof value !== 'boolean') { - throw new WireitError({ - type: 'failure', - reason: 'invalid-config-syntax', - script, - message: `${name} is not a boolean`, - }); - } -}; - /** * Throw an error if the given value is not an Array. */ diff --git a/src/script.ts b/src/script.ts index 66f384711..760568ca8 100644 --- a/src/script.ts +++ b/src/script.ts @@ -56,10 +56,14 @@ export interface ScriptConfig extends ScriptReference { output: string[] | undefined; /** - * Whether all files matching the output glob patterns should be deleted - * before the script executes. + * When to clean output: + * + * - true: Before the script executes, and before restoring from cache. + * - false: Before restoring from cache. + * - "if-file-deleted": If an input file has been deleted, and before restoring from + * cache. */ - clean: boolean; + clean: boolean | 'if-file-deleted'; } /** @@ -115,7 +119,7 @@ export interface ScriptState { * could produce different output, so a re-run should be triggered even if * nothing else changed. */ - clean: boolean; + clean: boolean | 'if-file-deleted'; // Must be sorted. files: {[packageDirRelativeFilename: string]: Sha256HexDigest}; diff --git a/src/test/errors-analysis.test.ts b/src/test/errors-analysis.test.ts index 9582b10af..3b5160123 100644 --- a/src/test/errors-analysis.test.ts +++ b/src/test/errors-analysis.test.ts @@ -268,7 +268,7 @@ test( ); test( - 'clean is not a boolean', + 'clean is not a boolean or "if-file-deleted"', timeout(async ({rig}) => { await rig.write({ 'package.json': { @@ -289,7 +289,7 @@ test( assert.equal( done.stderr.trim(), ` -❌ [a] Invalid config: clean is not a boolean`.trim() +❌ [a] Invalid config: clean must be true, false, or "if-file-deleted"`.trim() ); }) ); From bcaa9cb7d3fe7b4f674b0c3159b938399bcf91e2 Mon Sep 17 00:00:00 2001 From: Alexander Marks Date: Fri, 8 Apr 2022 08:35:04 -0700 Subject: [PATCH 4/9] Support clean:if-file-deleted in Executor --- src/executor.ts | 109 ++++++++++++++++------ src/test/clean.test.ts | 201 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 283 insertions(+), 27 deletions(-) diff --git a/src/executor.ts b/src/executor.ts index 284f36c78..fc8ee7136 100644 --- a/src/executor.ts +++ b/src/executor.ts @@ -15,6 +15,7 @@ import {scriptReferenceToString} from './script.js'; import {shuffle} from './util/shuffle.js'; import {WorkerPool} from './util/worker-pool.js'; import {getScriptDataDir} from './util/script-data-dir.js'; +import {unreachable} from './util/unreachable.js'; import type { ScriptConfig, @@ -135,22 +136,24 @@ class ScriptExecution { // script, which would affect the key. const state = await this.#computeState(dependencyStates); const stateStr = JSON.stringify(state) as ScriptStateString; - if (state.cacheable) { - const prevStateStr = await this.#readPreviousState(); - if (prevStateStr !== undefined && stateStr === prevStateStr) { - // TODO(aomarks) Does not preserve original order of stdout vs stderr - // chunks. See https://github.com/google/wireit/issues/74. - await Promise.all([ - this.#replayStdoutIfPresent(), - this.#replayStderrIfPresent(), - ]); - this.#logger.log({ - script: this.#script, - type: 'success', - reason: 'fresh', - }); - return state; - } + const prevStateStr = await this.#readPreviousState(); + if ( + state.cacheable && + prevStateStr !== undefined && + prevStateStr === stateStr + ) { + // TODO(aomarks) Does not preserve original order of stdout vs stderr + // chunks. See https://github.com/google/wireit/issues/74. + await Promise.all([ + this.#replayStdoutIfPresent(), + this.#replayStderrIfPresent(), + ]); + this.#logger.log({ + script: this.#script, + type: 'success', + reason: 'fresh', + }); + return state; } // It's important that we delete any previous state before running the @@ -162,17 +165,47 @@ class ScriptExecution { ? await this.#cache?.get(this.#script, stateStr) : undefined; - // The "clean" setting controls whether we delete output before execution. - // - // However, if we are restoring from cache, we should always delete existing - // output, regardless of the "clean" setting. The purpose of the "clean" - // setting is to allow tools that are smart about cleaning up their own - // previous output to work more efficiently, but that only applies when the - // tool is able to observe each incremental change to the input files. When - // we restore from cache, we are directly replacing the output files, and - // not invoking the tool at all, so there is no way for the tool to do any - // cleanup. - if (this.#script.clean || cacheHit !== undefined) { + const shouldClean = (() => { + if (cacheHit !== undefined) { + // If we are restoring from cache, we should always delete existing + // output. The purpose of "clean:false" and "clean:if-file-deleted" is to + // allow tools with incremental build (like tsc --build) to work. + // + // However, this only applies when the tool is able to observe each + // incremental change to the input files. When we restore from cache, we + // are directly replacing the output files, and not invoking the tool at + // all, so there is no way for the tool to do any cleanup. + return true; + } + switch (this.#script.clean) { + case true: { + return true; + } + case false: { + return false; + } + case 'if-file-deleted': { + if (prevStateStr === undefined) { + // If we don't know the previous state, then we can't know whether + // any input files were removed. It's safer to err on the side of + // cleaning. + return true; + } + return this.#anyInputFilesDeletedSinceLastRun( + state, + JSON.parse(prevStateStr) as ScriptState + ); + } + default: { + throw new Error( + `Unhandled clean setting: ${ + unreachable(this.#script.clean) as string + }` + ); + } + } + })(); + if (shouldClean) { await this.#cleanOutput(); } @@ -206,6 +239,28 @@ class ScriptExecution { return state; } + /** + * Compares the current set of input file names to the previous set of input + * file names, and returns whether any files have been removed. + */ + #anyInputFilesDeletedSinceLastRun( + curState: ScriptState, + prevState: ScriptState + ): boolean { + const curFiles = Object.keys(curState.files); + const prevFiles = Object.keys(prevState.files); + if (curFiles.length < prevFiles.length) { + return true; + } + const newFilesSet = new Set(curFiles); + for (const oldFile of prevFiles) { + if (!newFilesSet.has(oldFile)) { + return true; + } + } + return false; + } + async #executeDependencies(): Promise> { // Randomize the order we execute dependencies to make it less likely for a // user to inadvertently depend on any specific order, which could indicate diff --git a/src/test/clean.test.ts b/src/test/clean.test.ts index 3ad43e26e..e76bfcdef 100644 --- a/src/test/clean.test.ts +++ b/src/test/clean.test.ts @@ -277,4 +277,205 @@ test( }) ); +test( + '"if-file-deleted" cleans only when input file deleted', + timeout(async ({rig}) => { + const cmdA = await rig.newCommand(); + await rig.write({ + 'package.json': { + scripts: { + a: 'wireit', + }, + wireit: { + a: { + command: cmdA.command, + files: ['input/**'], + output: ['output/**'], + clean: 'if-file-deleted', + }, + }, + }, + }); + + // Initial run creates output A. + { + await rig.write({'input/a': 'v0'}); + + const exec = rig.exec('npm run a'); + const inv = await cmdA.nextInvocation(); + + // No outputs have been written yet. + assert.not(await rig.exists('output/a')); + assert.not(await rig.exists('output/b')); + assert.not(await rig.exists('output/c')); + + // Write output A. + await rig.write({'output/a': 'v0'}); + + inv.exit(0); + const res = await exec.exit; + assert.equal(res.code, 0); + } + + // Add new input file. Don't clean. Creates output/b. + { + await rig.write({'input/b': 'v0'}); + + const exec = rig.exec('npm run a'); + const inv = await cmdA.nextInvocation(); + + // Output A should still exist. + assert.equal(await rig.read('output/a'), 'v0'); + assert.not(await rig.exists('output/b')); + assert.not(await rig.exists('output/c')); + + // Write outputs A and B. + await rig.write({'output/a': 'v1'}); + await rig.write({'output/b': 'v1'}); + + inv.exit(0); + const res = await exec.exit; + assert.equal(res.code, 0); + } + + // Modify input file. Don't clean. + { + await rig.write({'input/a': 'v1'}); + + const exec = rig.exec('npm run a'); + const inv = await cmdA.nextInvocation(); + + // Outputs A and B should still exist. + assert.equal(await rig.read('output/a'), 'v1'); + assert.equal(await rig.read('output/b'), 'v1'); + assert.not(await rig.exists('output/c')); + + // Write outputs A and B + await rig.write({'output/a': 'v2'}); + await rig.write({'output/b': 'v2'}); + + inv.exit(0); + const res = await exec.exit; + assert.equal(res.code, 0); + assert.equal(cmdA.numInvocations, 3); + } + + // Delete input file. Clean. (This covers the case where the number of input + // files is lower). + { + await rig.delete('input/a'); + + const exec = rig.exec('npm run a'); + const inv = await cmdA.nextInvocation(); + + // Outputs A and B should have been cleaned. + assert.not(await rig.exists('output/a')); + assert.not(await rig.exists('output/b')); + assert.not(await rig.exists('output/c')); + + // Write output B. + await rig.write({'output/b': 'v3'}); + + inv.exit(0); + const res = await exec.exit; + assert.equal(res.code, 0); + } + + // Delete an input file, and also add an input file. Clean. (This covers the + // case where the number of input files are the same, but they are + // different.) + { + await rig.delete('input/b'); + await rig.write({'input/c': 'v0'}); + + const exec = rig.exec('npm run a'); + const inv = await cmdA.nextInvocation(); + + // Output B should have been cleaned. + assert.not(await rig.exists('output/a')); + assert.not(await rig.exists('output/b')); + assert.not(await rig.exists('output/c')); + + // Write output C. + await rig.write({'output/c': 'v0'}); + + inv.exit(0); + const res = await exec.exit; + assert.equal(res.code, 0); + } + + assert.equal(cmdA.numInvocations, 5); + }) +); + +test( + '"if-file-deleted" cleans only when input file deleted when dependency has no input files', + timeout(async ({rig}) => { + const cmdA = await rig.newCommand(); + await rig.write({ + 'package.json': { + scripts: { + a: 'wireit', + b: 'wireit', + }, + wireit: { + a: { + command: cmdA.command, + files: ['input/**'], + output: ['output/**'], + clean: 'if-file-deleted', + dependencies: ['b'], + }, + b: { + command: 'true', + }, + }, + }, + }); + + // Initial run creates output A. + { + await rig.write({'input/a': 'v0'}); + + const exec = rig.exec('npm run a'); + const inv = await cmdA.nextInvocation(); + + // No outputs have been written yet. + assert.not(await rig.exists('output/a')); + assert.not(await rig.exists('output/b')); + assert.not(await rig.exists('output/c')); + + // Write output A. + await rig.write({'output/a': 'v0'}); + + inv.exit(0); + const res = await exec.exit; + assert.equal(res.code, 0); + } + + // Add new input file. Don't clean. Creates output/b. + { + await rig.write({'input/b': 'v0'}); + + const exec = rig.exec('npm run a'); + const inv = await cmdA.nextInvocation(); + + // Output A should still exist. + assert.equal(await rig.read('output/a'), 'v0'); + assert.not(await rig.exists('output/b')); + assert.not(await rig.exists('output/c')); + + // Write outputs A and B. + await rig.write({'output/a': 'v1'}); + await rig.write({'output/b': 'v1'}); + + inv.exit(0); + const res = await exec.exit; + assert.equal(res.code, 0); + } + + assert.equal(cmdA.numInvocations, 2); + }) +); + test.run(); From aa27ee304ea462eb4bd0b9a2ba35bc44f829d486 Mon Sep 17 00:00:00 2001 From: Alexander Marks Date: Fri, 8 Apr 2022 08:35:18 -0700 Subject: [PATCH 5/9] Document clean:if-file-deleted --- CHANGELOG.md | 4 ++++ README.md | 30 ++++++++++++++++-------------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 05762e0e5..6c245963f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,10 @@ Versioning](https://semver.org/spec/v2.0.0.html). - Added `WIREIT_CACHE` environment variable, which controls caching behavior. Can be `local` or `none` to disable. +- Added `if-file-deleted` option to the `clean` settings. In this mode, + `output` files are deleted if any of the input files have been deleted since + the last run. + ### Changed - In watch mode, the terminal is now cleared at the start of each run, making it diff --git a/README.md b/README.md index 85c814cc8..d26ccaf49 100644 --- a/README.md +++ b/README.md @@ -291,13 +291,15 @@ a script. This is helpful for ensuring that every build is clean and free from outdated files created in previous runs from source files that have since been removed. -To enable output cleaning, configure the output files for each script by -specifying the `wireit.