feat: completion plugin

docs/PLUGIN-COMPLETION-DETECTION.md

@@ -0,0 +1,36 @@
+## Configure how `simple-git` determines the `git` tasks to be complete.
+
+Up until version `2.46.0`, `simple-git` used both `close` and `exit` events from the `git` child process to determine
+whether the task was complete as follows:
+
+- the close / exit event fires
+- if there is data on either `stderr` or `stdout`, or if the child process as a whole has thrown an exception (for
+  example when `git` is not installed) then the task is complete.
+- otherwise wait `50ms` and then treat the task as complete.
+
+From version `2.46.0` onwards, you can configure this behaviour by using the
+`completion` plugin:
+
+```typescript
+import simpleGit, { SimpleGit } from 'simple-git';
+
+const git: SimpleGit = simpleGit({
+   completion: {
+      onExit: 50,
+      onClose: true,
+   },
+});
+```
+
+The `completion` plugin accepts two properties `onClose` and `onExit` that can be either:
+
+- `false` to ignore this event from the child process
+- `true` to treat the task as complete as soon as the event has fired
+- `number` to wait an arbitrary number of `ms` after the event has fired before treating the task as complete.
+
+To ensure backward compatibility, version 2.x of `simple-git` uses a default of
+`onClose = true, onExit = 50`.
+
+From version 3.x of `simple-git` the default will change to `onClose = true, onExit = false`,
+it should only be necessary to handle the `exit` event when the child processes are
+configured to not close (ie: with keep-alive).

readme.md

@@ -79,6 +79,9 @@ await git.pull();
 
 ## Configuring Plugins
 
+- [Completion Detection](./docs/PLUGIN-COMPLETION-DETECTION.md)
+  Customise how `simple-git` detects the end of a `git` process.
+
 - [Error Detection](./docs/PLUGIN-ERRORS.md)
   Customise the detection of errors from the underlying `git` process.
 

src/lib/git-factory.ts

@@ -3,6 +3,7 @@ import { SimpleGitFactory } from '../../typings';
 import api from './api';
 import {
    commandConfigPrefixingPlugin,
+   completionDetectionPlugin,
    errorDetectionHandler,
    errorDetectionPlugin,
    PluginStore,
@@ -52,6 +53,7 @@ export function gitInstanceFactory(baseDir?: string | Partial<SimpleGitOptions>,
       plugins.add(commandConfigPrefixingPlugin(config.config));
    }
 
+   plugins.add(completionDetectionPlugin(config.completion));
    config.progress && plugins.add(progressMonitorPlugin(config.progress));
    config.timeout && plugins.add(timeoutPlugin(config.timeout));
    config.spawnOptions && plugins.add(spawnOptionsPlugin(config.spawnOptions));

src/lib/plugins/completion-detection.plugin.ts

@@ -0,0 +1,81 @@
+import deferred, { DeferredPromise } from '@kwsites/promise-deferred';
+import { SimpleGitPluginConfig } from '../types';
+import { delay } from '../utils';
+import { SimpleGitPlugin } from './simple-git-plugin';
+
+const never = deferred().promise;
+
+export function completionDetectionPlugin({
+                                             onClose = true,
+                                             onExit = 50
+                                          }: SimpleGitPluginConfig['completion'] = {}): SimpleGitPlugin<'spawn.after'> {
+
+   function createEvents() {
+      let exitCode = -1;
+      const events = {
+         close: deferred(),
+         closeTimeout: deferred(),
+         exit: deferred(),
+         exitTimeout: deferred(),
+      };
+
+      const result = Promise.race([
+         onClose === false ? never : events.closeTimeout.promise,
+         onExit === false ? never : events.exitTimeout.promise,
+      ]);
+
+      configureTimeout(onClose, events.close, events.closeTimeout);
+      configureTimeout(onExit, events.exit, events.exitTimeout);
+
+      return {
+         close(code: number) {
+            exitCode = code;
+            events.close.done();
+         },
+         exit(code: number) {
+            exitCode = code;
+            events.exit.done();
+         },
+         get exitCode() {
+            return exitCode;
+         },
+         result,
+      };
+   }
+
+   function configureTimeout(flag: boolean | number, event: DeferredPromise<void>, timeout: DeferredPromise<void>) {
+      if (flag === false) {
+         return;
+      }
+
+      (flag === true ? event.promise : event.promise.then(() => delay(flag))).then(timeout.done);
+   }
+
+   return {
+      type: 'spawn.after',
+      async action(_data, {spawned, close}) {
+         const events = createEvents();
+
+         let deferClose = true;
+         let quickClose = () => void (deferClose = false);
+
+         spawned.stdout?.on('data', quickClose);
+         spawned.stderr?.on('data', quickClose);
+         spawned.on('error', quickClose);
+
+         spawned.on('close', (code: number) => events.close(code));
+         spawned.on('exit', (code: number) => events.exit(code));
+
+         try{
+            await events.result;
+            if (deferClose) {
+               await delay(50);
+            }
+            close(events.exitCode);
+         }
+         catch (err) {
+            close(events.exitCode, err);
+         }
+      }
+   }
+}

src/lib/plugins/index.ts

@@ -1,4 +1,5 @@
 export * from './command-config-prefixing-plugin';
+export * from './completion-detection.plugin';
 export * from './error-detection.plugin';
 export * from './plugin-store';
 export * from './progress-monitor-plugin';

src/lib/plugins/simple-git-plugin.ts

@@ -19,6 +19,7 @@ export interface SimpleGitPluginTypes {
       data: void;
       context: SimpleGitTaskPluginContext & {
          spawned: ChildProcess;
+         close (exitCode: number, reason?: Error): void;
          kill (reason: Error): void;
       };
    },

src/lib/runners/git-executor-chain.ts

@@ -160,32 +160,8 @@ export class GitExecutorChain implements SimpleGitExecutor {
          const stdOut: Buffer[] = [];
          const stdErr: Buffer[] = [];
 
-         let attempted = false;
          let rejection: Maybe<Error>;
 
-         function attemptClose(exitCode: number, event: string = 'retry') {
-
-            // closing when there is content, terminate immediately
-            if (attempted || stdErr.length || stdOut.length) {
-               logger.info(`exitCode=%s event=%s rejection=%o`, exitCode, event, rejection);
-               done({
-                  stdOut,
-                  stdErr,
-                  exitCode,
-                  rejection,
-               });
-               attempted = true;
-            }
-
-            // first attempt at closing but no content yet, wait briefly for the close/exit that may follow
-            if (!attempted) {
-               attempted = true;
-               setTimeout(() => attemptClose(exitCode, 'deferred'), 50);
-               logger('received %s event before content on stdOut/stdErr', event)
-            }
-
-         }
-
          logger.info(`%s %o`, command, args);
          logger('%O', spawnOptions)
          const spawned = spawn(command, args, spawnOptions);
@@ -195,9 +171,6 @@ export class GitExecutorChain implements SimpleGitExecutor {
 
          spawned.on('error', onErrorReceived(stdErr, logger));
 
-         spawned.on('close', (code: number) => attemptClose(code, 'close'));
-         spawned.on('exit', (code: number) => attemptClose(code, 'exit'));
-
          if (outputHandler) {
             logger(`Passing child process stdOut/stdErr to custom outputHandler`);
             outputHandler(command, spawned.stdout!, spawned.stderr!, [...args]);
@@ -206,16 +179,23 @@ export class GitExecutorChain implements SimpleGitExecutor {
          this._plugins.exec('spawn.after', undefined, {
             ...pluginContext(task, args),
             spawned,
+            close(exitCode: number, reason?: Error) {
+               done({
+                  stdOut,
+                  stdErr,
+                  exitCode,
+                  rejection: rejection || reason,
+               });
+            },
             kill(reason: Error) {
                if (spawned.killed) {
                   return;
                }
 
                rejection = reason;
                spawned.kill('SIGINT');
-            }
+            },
          });
-
       });
    }
 

src/lib/types/index.ts

@@ -66,6 +66,23 @@ export interface GitExecutorResult {
 
 export interface SimpleGitPluginConfig {
 
+   /**
+    * Configures the events that should be used to determine when the unederlying child process has
+    * been terminated.
+    *
+    * Version 2 will default to use `onClose=true, onExit=50` to mean the `close` event will be
+    * used to immediately treat the child process as closed and start using the data from `stdOut`
+    * / `stdErr`, whereas the `exit` event will wait `50ms` before treating the child process
+    * as closed.
+    *
+    * This will be changed in version 3 to use `onClose=true, onExit=false` so that only the
+    * close event is used to determine the termination of the process.
+    */
+   completion: {
+      onClose?: boolean | number;
+      onExit?: boolean | number;
+   };
+
    /**
     * Configures the content of errors thrown by the `simple-git` instance for each task
     */

src/lib/utils/util.ts

@@ -138,13 +138,17 @@ export function prefixedArray<T>(input: T[], prefix: T): T[] {
    return output;
 }
 
-export function bufferToString (input: Buffer | Buffer[]): string {
+export function bufferToString(input: Buffer | Buffer[]): string {
    return (Array.isArray(input) ? Buffer.concat(input) : input).toString('utf-8');
 }
 
 /**
  * Get a new object from a source object with only the listed properties.
  */
-export function pick (source: Record<string, any>, properties: string[]) {
+export function pick(source: Record<string, any>, properties: string[]) {
    return Object.assign({}, ...properties.map((property) => property in source ? {[property]: source[property]} : {}));
 }
+
+export function delay(duration = 0): Promise<void> {
+   return new Promise(done => setTimeout(done, duration));
+}

test/integration/completion-plugin.spec.ts

@@ -0,0 +1,15 @@
+import { promiseError } from '@kwsites/promise-result';
+import { createTestContext, newSimpleGit, SimpleGitTestContext } from '../__fixtures__';
+
+describe('progress-monitor', () => {
+
+   let context: SimpleGitTestContext;
+
+   beforeEach(async () => context = await createTestContext());
+
+   it('detects successful completion', async () => {
+      const git = newSimpleGit(context.root);
+      expect(await promiseError(git.init())).toBeUndefined();
+   });
+
+});

test/unit/__fixtures__/child-processes.ts

@@ -90,4 +90,5 @@ async function exitChildProcess(proc: MockChildProcess, data: string | null, exi
 
    // exit/close events are bound to the process itself
    proc.$emit('exit', exitSignal);
+   proc.$emit('close', exitSignal);
 }

test/unit/completion-detection-plugin.spec.ts

@@ -0,0 +1,55 @@
+import { newSimpleGit, wait } from '../__fixtures__';
+import { theChildProcessMatching } from './__fixtures__';
+import { MockChildProcess } from './__mocks__/mock-child-process';
+
+describe('completionDetectionPlugin', () => {
+
+   function process(proc: MockChildProcess, data: string, close = false, exit = false) {
+      proc.stdout.$emit('data', Buffer.from(data));
+      close && proc.$emit('close', 1);
+      exit && proc.$emit('exit', 1);
+   }
+
+   it('can respond to just close events', async () => {
+      const git = newSimpleGit({
+         completion: {
+            onClose: true,
+            onExit: false,
+         },
+      });
+
+      const output = Promise.race([
+         git.raw('foo'),
+         git.raw('bar'),
+      ]);
+
+      await wait();
+
+      process(theChildProcessMatching(['foo']), 'foo', false, true);
+      process(theChildProcessMatching(['bar']), 'bar', true, false);
+
+      expect(await output).toBe('bar')
+   });
+
+   it('can respond to just exit events', async () => {
+      const git = newSimpleGit({
+         completion: {
+            onClose: false,
+            onExit: true,
+         },
+      });
+
+      const output = Promise.race([
+         git.raw('foo'),
+         git.raw('bar'),
+      ]);
+
+      await wait();
+
+      process(theChildProcessMatching(['foo']), 'foo', false, true);
+      process(theChildProcessMatching(['bar']), 'bar', true, false);
+
+      expect(await output).toBe('foo')
+   });
+
+});