Back out "progress: backout D65769371 and D65138587"

Summary:
Originally D65138587 was backed out because when using a release version on Windows (as opposed as running it on dev mode), running new commands would spawn a new *visible* terminal Window on Windows. D65769371 also had to be backed out because it depended on D65138587.

Because the issue has been fixed already in D66483471, we can back out the back-outs.

Reviewed By: muirdm

Differential Revision: D66483917

fbshipit-source-id: 32a56cf3f304e80eaae250fc6810f24231f1525f

addons/isl-server/src/Repository.ts

@@ -10,7 +10,6 @@ import type {KindOfChange, PollKind} from './WatchForChanges';
 import type {TrackEventName} from './analytics/eventNames';
 import type {ConfigLevel, ResolveCommandConflictOutput} from './commands';
 import type {RepositoryContext} from './serverTypes';
-import type {ExecaError} from 'execa';
 import type {
   CommitInfo,
   Disposable,
@@ -41,6 +40,7 @@ import type {
   CwdInfo,
 } from 'isl/src/types';
 import type {Comparison} from 'shared/Comparison';
+import type {EjecaChildProcess, EjecaOptions, EjecaError} from 'shared/ejeca';
 
 import {Internal} from './Internal';
 import {OperationQueue} from './OperationQueue';
@@ -76,10 +76,9 @@ import {
 import {
   findPublicAncestor,
   handleAbortSignalOnProcess,
-  isExecaError,
+  isEjecaError,
   serializeAsyncCall,
 } from './utils';
-import execa from 'execa';
 import {
   settableConfigNames,
   allConfigNames,
@@ -92,6 +91,7 @@ import {revsetArgsForComparison} from 'shared/Comparison';
 import {LRU} from 'shared/LRU';
 import {RateLimiter} from 'shared/RateLimiter';
 import {TypedEventEmitter} from 'shared/TypedEventEmitter';
+import {ejeca} from 'shared/ejeca';
 import {exists} from 'shared/fs';
 import {removeLeadingPathSep} from 'shared/pathUtils';
 import {notEmpty, randomId, nullthrows} from 'shared/utils';
@@ -654,6 +654,52 @@ export class Repository {
     return {args, stdin};
   }
 
+  private async operationIPC(
+    ctx: RepositoryContext,
+    onProgress: OperationCommandProgressReporter,
+    child: EjecaChildProcess,
+    options: EjecaOptions,
+  ): Promise<void> {
+    if (!options.ipc) {
+      return;
+    }
+
+    interface IpcProgressBar {
+      id: number;
+      topic: string;
+      unit: string;
+      total: number;
+      position: number;
+      parent_id?: number;
+    }
+
+    while (true) {
+      try {
+        // eslint-disable-next-line no-await-in-loop
+        const message = await child.getOneMessage();
+        if (
+          message === null ||
+          typeof message !== 'object' ||
+          !('progress_bar_update' in message)
+        ) {
+          break;
+        }
+        const bars = message.progress_bar_update as IpcProgressBar[];
+        const blen = bars.length;
+        if (blen > 0) {
+          const msg = bars[blen - 1];
+          onProgress('progress', {
+            message: msg.topic,
+            progress: msg.position,
+            progressTotal: msg.total,
+          });
+        }
+      } catch (err) {
+        break;
+      }
+    }
+  }
+
   /**
    * Called by this.operationQueue in response to runOrQueueOperation when an operation is ready to actually run.
    */
@@ -671,11 +717,12 @@ export class Repository {
       this.getMergeToolEnvVars(ctx),
     ]);
 
+    const ipc = (ctx.knownConfigs?.get('isl.sl-progress-enabled') ?? 'false') === 'true';
     const {command, args, options} = getExecParams(
       this.info.command,
       cwdRelativeArgs,
       cwd,
-      stdin ? {input: stdin} : undefined,
+      stdin ? {input: stdin, ipc} : {ipc},
       {
         ...env[0],
         ...env[1],
@@ -689,7 +736,7 @@ export class Repository {
       throw new Error(`command "${args.join(' ')}" is not allowed`);
     }
 
-    const execution = execa(command, args, options);
+    const execution = ejeca(command, args, options);
     // It would be more appropriate to call this in reponse to execution.on('spawn'), but
     // this seems to be inconsistent about firing in all versions of node.
     // Just send spawn immediately. Errors during spawn like ENOENT will still be reported by `exit`.
@@ -705,10 +752,11 @@ export class Repository {
     });
     handleAbortSignalOnProcess(execution, signal);
     try {
+      this.operationIPC(ctx, onProgress, execution, options);
       const result = await execution;
       onProgress('exit', result.exitCode || 0);
     } catch (err) {
-      onProgress('exit', isExecaError(err) ? err.exitCode : -1);
+      onProgress('exit', isEjecaError(err) ? err.exitCode : -1);
       throw err;
     }
   }
@@ -789,7 +837,7 @@ export class Repository {
       this.uncommittedChangesEmitter.emit('change', this.uncommittedChanges);
     } catch (err) {
       let error = err;
-      if (isExecaError(error)) {
+      if (isEjecaError(error)) {
         if (error.stderr.includes('checkout is currently in progress')) {
           this.initialConnectionContext.logger.info(
             'Ignoring `sl status` error caused by in-progress checkout',
@@ -799,8 +847,8 @@ export class Repository {
       }
 
       this.initialConnectionContext.logger.error('Error fetching files: ', error);
-      if (isExecaError(error)) {
-        error = simplifyExecaError(error);
+      if (isEjecaError(error)) {
+        error = simplifyEjecaError(error);
       }
 
       // emit an error, but don't save it to this.uncommittedChanges
@@ -895,13 +943,13 @@ export class Repository {
       if (internalError) {
         error = internalError;
       }
-      if (isExecaError(error) && error.stderr.includes('Please check your internet connection')) {
+      if (isEjecaError(error) && error.stderr.includes('Please check your internet connection')) {
         error = Error('Network request failed. Please check your internet connection.');
       }
 
       this.initialConnectionContext.logger.error('Error fetching commits: ', error);
-      if (isExecaError(error)) {
-        error = simplifyExecaError(error);
+      if (isEjecaError(error)) {
+        error = simplifyEjecaError(error);
       }
 
       this.smartlogCommitsChangesEmitter.emit('change', {
@@ -1373,7 +1421,7 @@ export class Repository {
     /** Which event name to track for this command. If undefined, generic 'RunCommand' is used. */
     eventName: TrackEventName | undefined,
     ctx: RepositoryContext,
-    options?: execa.Options,
+    options?: EjecaOptions,
     timeout?: number,
   ) {
     const id = randomId();
@@ -1508,8 +1556,8 @@ function isUnhealthyEdenFs(cwd: string): Promise<boolean> {
 }
 
 /**
- * Extract the actually useful stderr part of the Execa Error, to avoid the long command args being printed first.
+ * Extract the actually useful stderr part of the Ejeca Error, to avoid the long command args being printed first.
  * */
-function simplifyExecaError(error: ExecaError): Error {
+function simplifyEjecaError(error: EjecaError): Error {
   return new Error(error.stderr.trim() || error.message);
 }

addons/isl-server/src/ServerToClientAPI.ts

@@ -11,7 +11,6 @@ import type {ServerSideTracker} from './analytics/serverSideTracker';
 import type {Logger} from './logger';
 import type {ServerPlatform} from './serverPlatform';
 import type {RepositoryContext} from './serverTypes';
-import type {ExecaError} from 'execa';
 import type {TypeaheadResult} from 'isl-components/Types';
 import type {Serializable} from 'isl/src/serialize';
 import type {
@@ -29,6 +28,7 @@ import type {
   CodeReviewProviderSpecificClientToServerMessages,
   StableLocationData,
 } from 'isl/src/types';
+import type {EjecaError} from 'shared/ejeca';
 import type {ExportStack, ImportedStack} from 'shared/types/stack';
 
 import {generatedFilesDetector} from './GeneratedFiles';
@@ -963,7 +963,7 @@ export default class ServerToClientAPI {
               url: {value: result.stdout},
             });
           })
-          .catch((err: ExecaError) => {
+          .catch((err: EjecaError) => {
             this.logger.error('Failed to get repo url at hash:', err);
             this.postMessage({
               type: 'gotRepoUrlAtHash',