From 7bd72dacbda9192dfd7e49de4616fb352547db1c Mon Sep 17 00:00:00 2001 From: Alexander Marks Date: Sun, 30 Oct 2022 10:59:04 -0700 Subject: [PATCH 1/5] Rename isDirectlyInvoked to isPersistent --- src/analyzer.ts | 12 ++++++------ src/config.ts | 2 +- src/execution/service.ts | 12 ++++++------ src/executor.ts | 28 ++++++++++++++-------------- src/test/analysis.test.ts | 6 +++--- src/test/service.test.ts | 12 ++++++------ 6 files changed, 36 insertions(+), 36 deletions(-) diff --git a/src/analyzer.ts b/src/analyzer.ts index feae56208..2dba711a2 100644 --- a/src/analyzer.ts +++ b/src/analyzer.ts @@ -946,7 +946,7 @@ export class Analyzer { private _checkForCyclesAndSortDependencies( config: LocallyValidScriptConfig | ScriptConfig | InvalidScriptConfig, trail: Set, - isDirectlyInvoked: boolean + isPersistent: boolean ): Result { if (config.state === 'valid') { // Already validated. @@ -1069,10 +1069,10 @@ export class Analyzer { dependency.config, trail, // Walk through no-command scripts when determining if something is - // being directly invoked (e.g. if the top-level script has no command - // and simply delegates to one or more other scripts, then those - // dependencies are effectively being directly invoked). - isDirectlyInvoked && config.command === undefined + // persistent (e.g. if the top-level script has no command and + // simply delegates to one or more other scripts, then those + // dependencies are effectively persistent). + isPersistent && config.command === undefined ); if (!validDependencyConfigResult.ok) { return { @@ -1128,7 +1128,7 @@ export class Analyzer { // Unfortunately TypeScript doesn't narrow the ...config spread, so we // have to assign explicitly. command: config.command, - isDirectlyInvoked, + isPersistent, serviceConsumers: [], }; } else { diff --git a/src/config.ts b/src/config.ts index 2e5aa988e..3db3ed761 100644 --- a/src/config.ts +++ b/src/config.ts @@ -84,7 +84,7 @@ export interface ServiceScriptConfig /** * Whether this service is being invoked directly (e.g. `npm run serve`). */ - isDirectlyInvoked: boolean; + isPersistent: boolean; /** * Scripts that depend on this service. diff --git a/src/execution/service.ts b/src/execution/service.ts index 964e2db91..99d210c24 100644 --- a/src/execution/service.ts +++ b/src/execution/service.ts @@ -125,9 +125,9 @@ function unexpectedState(state: ServiceState) { * │ │ │ │ * │ ├─────◄──────────────╯ │ * │ │ │ - * ▼ ╔══════════▼═══════════╗ │ - * │ ║ is directly invoked? ╟── yes ──╮ │ - * │ ╚══════════╤═══════════╝ │ │ + * ▼ ╔═══════▼════════╗ │ + * │ ║ is persistent? ╟───── yes ──╮ │ + * │ ╚═══════╤════════╝ │ │ * │ │ │ │ * │ no │ │ * │ │ │ │ @@ -298,7 +298,7 @@ export class ServiceScriptExecution extends BaseExecutionWithCommand { @@ -439,7 +439,7 @@ export class ServiceScriptExecution extends BaseExecutionWithCommand(); - private readonly _directlyInvokedServices: ServiceMap = new Map(); - private readonly _indirectlyInvokedServices: ServiceScriptExecution[] = []; + private readonly _persistentServices: ServiceMap = new Map(); + private readonly _ephemeralServices: ServiceScriptExecution[] = []; private readonly _previousIterationServices: ServiceMap | undefined; private readonly _logger: Logger; private readonly _workerPool: WorkerPool; @@ -130,16 +130,16 @@ export class Executor { this._previousIterationServices.size > 0 ) { // If any services were removed from the graph entirely, or used to be - // directly invoked but are no longer, then stop them now. - const currentDirectlyInvokedServices = new Set(); + // persistent but are no longer, then stop them now. + const currentPersistentServices = new Set(); for (const script of findAllScripts(this._rootConfig)) { - if (script.service && script.isDirectlyInvoked) { - currentDirectlyInvokedServices.add(scriptReferenceToString(script)); + if (script.service && script.isPersistent) { + currentPersistentServices.add(scriptReferenceToString(script)); } } const stopPromises = []; for (const [key, service] of this._previousIterationServices) { - if (!currentDirectlyInvokedServices.has(key)) { + if (!currentPersistentServices.has(key)) { const child = service.detach(); if (child !== undefined) { child.kill(); @@ -158,10 +158,10 @@ export class Executor { if (!rootExecutionResult.ok) { errors.push(...rootExecutionResult.error); } - const indirectlyInvokedServiceResults = await Promise.all( - this._indirectlyInvokedServices.map((service) => service.terminated) + const ephemeralServiceResults = await Promise.all( + this._ephemeralServices.map((service) => service.terminated) ); - for (const result of indirectlyInvokedServiceResults) { + for (const result of ephemeralServiceResults) { if (!result.ok) { errors.push(result.error); } @@ -169,7 +169,7 @@ export class Executor { if (errors.length > 0) { return {ok: false, error: errors}; } - return {ok: true, value: this._directlyInvokedServices}; + return {ok: true, value: this._persistentServices}; } /** @@ -215,10 +215,10 @@ export class Executor { this._stopServices.promise, this._previousIterationServices?.get(key) ); - if (config.isDirectlyInvoked) { - this._directlyInvokedServices.set(key, execution); + if (config.isPersistent) { + this._persistentServices.set(key, execution); } else { - this._indirectlyInvokedServices.push(execution); + this._ephemeralServices.push(execution); } } else { execution = new StandardScriptExecution( diff --git a/src/test/analysis.test.ts b/src/test/analysis.test.ts index 9962177c7..9b122dbab 100644 --- a/src/test/analysis.test.ts +++ b/src/test/analysis.test.ts @@ -99,7 +99,7 @@ test('analyzes services', async ({rig}) => { } assert.equal(b.serviceConsumers.length, 1); assert.equal(b.serviceConsumers[0].name, 'd'); - assert.equal(b.isDirectlyInvoked, true); + assert.equal(b.isPersistent, true); // c const c = a.dependencies[1].config; @@ -107,7 +107,7 @@ test('analyzes services', async ({rig}) => { if (!c.service) { throw new Error('Expected service'); } - assert.equal(c.isDirectlyInvoked, true); + assert.equal(c.isPersistent, true); assert.equal(c.serviceConsumers.length, 0); assert.equal(c.services.length, 0); @@ -124,7 +124,7 @@ test('analyzes services', async ({rig}) => { if (!e.service) { throw new Error('Expected service'); } - assert.equal(e.isDirectlyInvoked, false); + assert.equal(e.isPersistent, false); assert.equal(e.serviceConsumers.length, 1); }); diff --git a/src/test/service.test.ts b/src/test/service.test.ts index 6f31d1ecd..a581218d7 100644 --- a/src/test/service.test.ts +++ b/src/test/service.test.ts @@ -378,7 +378,7 @@ test( ); test( - 'directly invoked service and dependency starts and runs until SIGINT', + 'persistent service and dependency starts and runs until SIGINT', // service1 // | // v @@ -449,11 +449,11 @@ test( ); for (const failureMode of ['continue', 'no-new', 'kill']) { - // Even directly invoked services which don't have an error in their branch - // should stop when an error occurs elsewhere, regardless of the error mode. + // Even persistent services which don't have an error in their branch should + // stop when an error occurs elsewhere, regardless of the error mode. // Otherwise wireit won't always exit on failures. test( - `directly invoked service and dependency stop on error ` + + `persistent service and dependency stop on error ` + `with failure mode ${failureMode}`, // entrypoint // / \ @@ -546,7 +546,7 @@ for (const failureMode of ['continue', 'no-new', 'kill']) { } test( - 'indirectly invoked service shuts down between watch iterations', + 'ephemeral service shuts down between watch iterations', timeout(async ({rig}) => { // consumer // | @@ -606,7 +606,7 @@ test( ); test( - 'directly invoked service is preserved across watch iterations', + 'persistent service is preserved across watch iterations', timeout(async ({rig}) => { // entrypoint // / \ From 7c6e303ee0e4f20a50f9acc1e2d6f56feba2828f Mon Sep 17 00:00:00 2001 From: Alexander Marks Date: Sun, 30 Oct 2022 11:00:55 -0700 Subject: [PATCH 2/5] Expand documentation for isPersistent --- src/config.ts | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/src/config.ts b/src/config.ts index 3db3ed761..0c2dd3d6b 100644 --- a/src/config.ts +++ b/src/config.ts @@ -82,7 +82,34 @@ export interface ServiceScriptConfig service: true; /** - * Whether this service is being invoked directly (e.g. `npm run serve`). + * Whether this service persists beyond the initial execution phase. + * + * When true, this service will keep running until the user exits wireit, or + * until its fingerprint changes in watch mode, requiring a restart. + * + * When false, this service will start only if it is needed by a standard + * script, and will stop when that dependent is done. We call these scripts + * "ephemeral". + * + * So, this is true when there is a path from the entrypoint script to the + * service, which does not pass through a standard script. + * + * Example: + * + * start + * (no-command) + * / \ + * ▼ ▼ + * serve:api serve:static + * (persistent service) (persistent service) + * | | + * ▼ ▼ + * serve:db build:assets + * (persistent service) (standard) + * | + * ▼ + * serve:playwright + * (ephemeral service) */ isPersistent: boolean; From 763beb6edda3d899e0d1a3c8ec1485e8b09d36be Mon Sep 17 00:00:00 2001 From: Alexander Marks Date: Sun, 30 Oct 2022 11:02:01 -0700 Subject: [PATCH 3/5] Implement new definition of isPersistent --- src/analyzer.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/analyzer.ts b/src/analyzer.ts index 2dba711a2..add05c20d 100644 --- a/src/analyzer.ts +++ b/src/analyzer.ts @@ -1068,11 +1068,9 @@ export class Analyzer { this._checkForCyclesAndSortDependencies( dependency.config, trail, - // Walk through no-command scripts when determining if something is - // persistent (e.g. if the top-level script has no command and - // simply delegates to one or more other scripts, then those - // dependencies are effectively persistent). - isPersistent && config.command === undefined + // Walk through no-command scripts and services when determining if + // something is persistent. + isPersistent && (config.command === undefined || config.service) ); if (!validDependencyConfigResult.ok) { return { From bd8e0bc9c4858cd83a7f1dea4542b531b371425a Mon Sep 17 00:00:00 2001 From: Alexander Marks Date: Sun, 30 Oct 2022 11:02:32 -0700 Subject: [PATCH 4/5] Allow start to be called multiple times --- src/execution/service.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/execution/service.ts b/src/execution/service.ts index 99d210c24..b2eecfd57 100644 --- a/src/execution/service.ts +++ b/src/execution/service.ts @@ -525,6 +525,12 @@ export class ServiceScriptExecution extends BaseExecutionWithCommand Date: Sun, 30 Oct 2022 11:02:49 -0700 Subject: [PATCH 5/5] Expand persistence test to cover services of services --- src/test/service.test.ts | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/src/test/service.test.ts b/src/test/service.test.ts index a581218d7..d084ae93c 100644 --- a/src/test/service.test.ts +++ b/src/test/service.test.ts @@ -606,28 +606,38 @@ test( ); test( - 'persistent service is preserved across watch iterations', + 'persistent services are preserved across watch iterations', timeout(async ({rig}) => { // entrypoint // / \ // v v - // service standard + // service1 standard + // | + // v + // service2 - const service = await rig.newCommand(); + const service1 = await rig.newCommand(); + const service2 = await rig.newCommand(); const standard = await rig.newCommand(); await rig.writeAtomic({ 'package.json': { scripts: { entrypoint: 'wireit', - service: 'wireit', + service1: 'wireit', + service2: 'wireit', standard: 'wireit', }, wireit: { entrypoint: { - dependencies: ['service', 'standard'], + dependencies: ['service1', 'standard'], }, - service: { - command: service.command, + service1: { + command: service1.command, + dependencies: ['service2'], + service: true, + }, + service2: { + command: service2.command, service: true, }, standard: { @@ -643,10 +653,12 @@ test( // Iteration 1 { - await service.nextInvocation(); + await service2.nextInvocation(); + await service1.nextInvocation(); const standardInv = await standard.nextInvocation(); standardInv.exit(0); await standardInv.closed; + await wireit.waitForLog(/Watching for file changes/); } await rig.write('input', '1'); @@ -656,11 +668,13 @@ test( const standardInv = await standard.nextInvocation(); standardInv.exit(0); await standardInv.closed; + await wireit.waitForLog(/Watching for file changes/); } wireit.kill(); await wireit.exit; - assert.equal(service.numInvocations, 1); + assert.equal(service1.numInvocations, 1); + assert.equal(service2.numInvocations, 1); assert.equal(standard.numInvocations, 2); }) );