do not try to restore temp sketches.

Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
arduino · kittaakos · Aug 26, 2022 · Aug 23, 2022 · Aug 24, 2022 · Aug 24, 2022
commit ea69029fb57f713dfcb81d409d92f843bcad3f9c
diff --git a/arduino-ide-extension/src/browser/contributions/save-as-sketch.ts b/arduino-ide-extension/src/browser/contributions/save-as-sketch.ts
@@ -104,16 +104,17 @@ export class SaveAsSketch extends SketchContribution {
       await this.saveOntoCopiedSketch(sketch.mainFileUri, sketch.uri, workspaceUri);
     }
     if (workspaceUri && openAfterMove) {
+      this.windowService.setSafeToShutDown();
       if (wipeOriginal || (openAfterMove && execOnlyIfTemp)) {
-        try {
-          await this.fileService.delete(new URI(sketch.uri), {
-            recursive: true,
-          });
-        } catch {
-          /* NOOP: from time to time, it's not possible to wipe the old resource from the temp dir on Windows */
-        }
+        // This window will navigate away.
+        // Explicitly stop the contribution to dispose the file watcher before deleting the temp sketch.
+        // Otherwise, users might see irrelevant _Unable to watch for file changes in this large workspace._ notification.
+        // https://github.com/arduino/arduino-ide/issues/39.
+        this.sketchServiceClient.onStop();
+        // TODO: consider implementing the temp sketch deletion the following way:
+        // Open the other sketch with a `delete the temp sketch` startup-task.
+        this.sketchService.notifyDeleteSketch(sketch); // This is a notification and will execute on the backend.
       }
-      this.windowService.setSafeToShutDown();
       this.workspaceService.open(new URI(workspaceUri), {
         preserveWindow: true,
       });

diff --git a/arduino-ide-extension/src/common/protocol/sketches-service.ts b/arduino-ide-extension/src/common/protocol/sketches-service.ts
@@ -95,6 +95,11 @@ export interface SketchesService {
    * Based on https://github.com/arduino/arduino-cli/blob/550179eefd2d2bca299d50a4af9e9bfcfebec649/arduino/builder/builder.go#L30-L38
    */
   getIdeTempFolderUri(sketch: Sketch): Promise<string>;
+
+  /**
+   * Notifies the backend to recursively delete the sketch folder with all its content.
+   */
+  notifyDeleteSketch(sketch: Sketch): void;
 }
 
 export interface SketchRef {

diff --git a/arduino-ide-extension/src/electron-main/arduino-electron-main-module.ts b/arduino-ide-extension/src/electron-main/arduino-electron-main-module.ts
@@ -16,6 +16,7 @@ import {
   ElectronMainWindowServiceExt,
   electronMainWindowServiceExtPath,
 } from '../electron-common/electron-main-window-service-ext';
+import { IsTempSketch } from '../node/is-temp-sketch';
 import { ElectronMainWindowServiceExtImpl } from './electron-main-window-service-ext-impl';
 import { IDEUpdaterImpl } from './ide-updater/ide-updater-impl';
 import { ElectronMainApplication } from './theia/electron-main-application';
@@ -62,4 +63,6 @@ export default new ContainerModule((bind, unbind, isBound, rebind) => {
         )
     )
     .inSingletonScope();
+
+  bind(IsTempSketch).toSelf().inSingletonScope();
 });
diff --git a/arduino-ide-extension/src/electron-main/theia/electron-main-application.ts b/arduino-ide-extension/src/electron-main/theia/electron-main-application.ts
@@ -1,4 +1,4 @@
-import { injectable } from '@theia/core/shared/inversify';
+import { inject, injectable } from '@theia/core/shared/inversify';
 import {
   app,
   BrowserWindow,
@@ -22,6 +22,7 @@ import { Deferred } from '@theia/core/lib/common/promise-util';
 import * as os from '@theia/core/lib/common/os';
 import { Restart } from '@theia/core/lib/electron-common/messaging/electron-messages';
 import { TheiaBrowserWindowOptions } from '@theia/core/lib/electron-main/theia-electron-window';
+import { IsTempSketch } from '../../node/is-temp-sketch';
 
 app.commandLine.appendSwitch('disable-http-cache');
 
@@ -54,6 +55,8 @@ const APP_STARTED_WITH_CONTENT_TRACE =
 
 @injectable()
 export class ElectronMainApplication extends TheiaElectronMainApplication {
+  @inject(IsTempSketch)
+  private readonly isTempSketch: IsTempSketch;
   private startup = false;
   private _firstWindowId: number | undefined;
   private openFilePromise = new Deferred();
@@ -176,6 +179,12 @@ export class ElectronMainApplication extends TheiaElectronMainApplication {
       );
       for (const workspace of workspaces) {
         if (await this.isValidSketchPath(workspace.file)) {
+          if (this.isTempSketch.is(workspace.file)) {
+            console.info(
+              `Skipped opening sketch. The sketch was detected as temporary. Workspace path: ${workspace.file}.`
+            );
+            continue;
+          }
           useDefault = false;
           await this.openSketch(workspace);
         }
@@ -405,6 +414,15 @@ export class ElectronMainApplication extends TheiaElectronMainApplication {
         const workspaceUri = URI.file(workspace);
         const bounds = window.getNormalBounds();
         const now = Date.now();
+        // Do not try to reopen the sketch if it was temp.
+        // Unfortunately, IDE2 has two different logic of restoring recent sketches: the Theia default `recentworkspace.json` and there is the `recent-sketches.json`.
+        const file = workspaceUri.fsPath;
+        if (this.isTempSketch.is(file)) {
+          console.info(
+            `Ignored marking workspace as a closed sketch. The sketch was detected as temporary. Workspace URI: ${workspaceUri.toString()}.`
+          );
+          return;
+        }
         console.log(
           `Marking workspace as a closed sketch. Workspace URI: ${workspaceUri.toString()}. Date: ${now}.`
         );

diff --git a/arduino-ide-extension/src/node/arduino-ide-backend-module.ts b/arduino-ide-extension/src/node/arduino-ide-backend-module.ts
@@ -109,6 +109,7 @@ import {
   SurveyNotificationService,
   SurveyNotificationServicePath,
 } from '../common/protocol/survey-service';
+import { IsTempSketch } from './is-temp-sketch';
 
 export default new ContainerModule((bind, unbind, isBound, rebind) => {
   bind(BackendApplication).toSelf().inSingletonScope();
@@ -419,4 +420,6 @@ export default new ContainerModule((bind, unbind, isBound, rebind) => {
         )
     )
     .inSingletonScope();
+
+  bind(IsTempSketch).toSelf().inSingletonScope();
 });
diff --git a/arduino-ide-extension/src/node/is-temp-sketch.ts b/arduino-ide-extension/src/node/is-temp-sketch.ts
@@ -0,0 +1,47 @@
+import * as fs from 'fs';
+import * as tempDir from 'temp-dir';
+import { isWindows, isOSX } from '@theia/core/lib/common/os';
+import { injectable } from '@theia/core/shared/inversify';
+import { firstToLowerCase } from '../common/utils';
+
+const Win32DriveRegex = /^[a-zA-Z]:\\/;
+export const TempSketchPrefix = '.arduinoIDE-unsaved';
+
+@injectable()
+export class IsTempSketch {
+  // If on macOS, the `temp-dir` lib will make sure there is resolved realpath.
+  // If on Windows, the `C:\Users\KITTAA~1\AppData\Local\Temp` path will be resolved and normalized to `C:\Users\kittaakos\AppData\Local\Temp`.
+  // Note: VS Code URI normalizes the drive letter. `C:` will be converted into `c:`.
+  // https://github.com/Microsoft/vscode/issues/68325#issuecomment-462239992
+  private readonly tempDirRealpath = isOSX
+    ? tempDir
+    : maybeNormalizeDrive(fs.realpathSync.native(tempDir));
+
+  is(sketchPath: string): boolean {
+    // Consider the following paths:
+    // macOS:
+    // - Temp folder: /var/folders/k3/d2fkvv1j16v3_rz93k7f74180000gn/T
+    // - Sketch folder: /private/var/folders/k3/d2fkvv1j16v3_rz93k7f74180000gn/T/arduino-ide2-A0337D47F86B24A51DF3DBCF2CC17925
+    // Windows:
+    // - Temp folder: C:\Users\KITTAA~1\AppData\Local\Temp
+    // - Sketch folder: c:\Users\kittaakos\AppData\Local\Temp\.arduinoIDE-unsaved2022431-21824-116kfaz.9ljl\sketch_may31a
+    // Both sketches are valid and temp, but this function will give a false-negative result if we use the default `os.tmpdir()` logic.
+    const normalizedSketchPath = maybeNormalizeDrive(sketchPath);
+    const result =
+      normalizedSketchPath.startsWith(this.tempDirRealpath) &&
+      normalizedSketchPath.includes(TempSketchPrefix);
+    console.debug(`isTempSketch: ${result}. Input was ${normalizedSketchPath}`);
+    return result;
+  }
+}
+
+/**
+ * If on Windows, will change the input `C:\\path\\to\\somewhere` to `c:\\path\\to\\somewhere`.
+ * Otherwise, returns with the argument.
+ */
+export function maybeNormalizeDrive(fsPath: string): string {
+  if (isWindows && Win32DriveRegex.test(fsPath)) {
+    return firstToLowerCase(fsPath);
+  }
+  return fsPath;
+}
diff --git a/arduino-ide-extension/src/node/sketches-service-impl.ts b/arduino-ide-extension/src/node/sketches-service-impl.ts
@@ -2,14 +2,13 @@ import { injectable, inject } from '@theia/core/shared/inversify';
 import * as fs from 'fs';
 import * as os from 'os';
 import * as temp from 'temp';
-import * as tempDir from 'temp-dir';
+
 import * as path from 'path';
 import * as crypto from 'crypto';
 import { ncp } from 'ncp';
 import { promisify } from 'util';
 import URI from '@theia/core/lib/common/uri';
 import { FileUri } from '@theia/core/lib/node';
-import { isWindows, isOSX } from '@theia/core/lib/common/os';
 import { ConfigServiceImpl } from './config-service-impl';
 import {
   SketchesService,
@@ -18,7 +17,6 @@ import {
   SketchContainer,
   SketchesError,
 } from '../common/protocol/sketches-service';
-import { firstToLowerCase } from '../common/utils';
 import { NotificationServiceServerImpl } from './notification-service-server';
 import { EnvVariablesServer } from '@theia/core/lib/common/env-variables';
 import { CoreClientAware } from './core-client-provider';
@@ -30,10 +28,11 @@ import { duration } from '../common/decorators';
 import * as glob from 'glob';
 import { Deferred } from '@theia/core/lib/common/promise-util';
 import { ServiceError } from './service-error';
-
-const WIN32_DRIVE_REGEXP = /^[a-zA-Z]:\\/;
-
-const prefix = '.arduinoIDE-unsaved';
+import {
+  IsTempSketch,
+  maybeNormalizeDrive,
+  TempSketchPrefix,
+} from './is-temp-sketch';
 
 @injectable()
 export class SketchesServiceImpl
@@ -42,22 +41,18 @@ export class SketchesServiceImpl
 {
   private sketchSuffixIndex = 1;
   private lastSketchBaseName: string;
-  // If on macOS, the `temp-dir` lib will make sure there is resolved realpath.
-  // If on Windows, the `C:\Users\KITTAA~1\AppData\Local\Temp` path will be resolved and normalized to `C:\Users\kittaakos\AppData\Local\Temp`.
-  // Note: VS Code URI normalizes the drive letter. `C:` will be converted into `c:`.
-  // https://github.com/Microsoft/vscode/issues/68325#issuecomment-462239992
-  private tempDirRealpath = isOSX
-    ? tempDir
-    : maybeNormalizeDrive(fs.realpathSync.native(tempDir));
 
   @inject(ConfigServiceImpl)
-  protected readonly configService: ConfigServiceImpl;
+  private readonly configService: ConfigServiceImpl;
 
   @inject(NotificationServiceServerImpl)
-  protected readonly notificationService: NotificationServiceServerImpl;
+  private readonly notificationService: NotificationServiceServerImpl;
 
   @inject(EnvVariablesServer)
-  protected readonly envVariableServer: EnvVariablesServer;
+  private readonly envVariableServer: EnvVariablesServer;
+
+  @inject(IsTempSketch)
+  private readonly isTempSketch: IsTempSketch;
 
   async getSketches({
     uri,
@@ -424,7 +419,7 @@ void loop() {
    */
   private createTempFolder(): Promise<string> {
     return new Promise<string>((resolve, reject) => {
-      temp.mkdir({ prefix }, (createError, dirPath) => {
+      temp.mkdir({ prefix: TempSketchPrefix }, (createError, dirPath) => {
         if (createError) {
           reject(createError);
           return;
@@ -475,20 +470,7 @@ void loop() {
   }
 
   async isTemp(sketch: SketchRef): Promise<boolean> {
-    // Consider the following paths:
-    // macOS:
-    // - Temp folder: /var/folders/k3/d2fkvv1j16v3_rz93k7f74180000gn/T
-    // - Sketch folder: /private/var/folders/k3/d2fkvv1j16v3_rz93k7f74180000gn/T/arduino-ide2-A0337D47F86B24A51DF3DBCF2CC17925
-    // Windows:
-    // - Temp folder: C:\Users\KITTAA~1\AppData\Local\Temp
-    // - Sketch folder: c:\Users\kittaakos\AppData\Local\Temp\.arduinoIDE-unsaved2022431-21824-116kfaz.9ljl\sketch_may31a
-    // Both sketches are valid and temp, but this function will give a false-negative result if we use the default `os.tmpdir()` logic.
-    const sketchPath = maybeNormalizeDrive(FileUri.fsPath(sketch.uri));
-    const tempPath = this.tempDirRealpath; // https://github.com/sindresorhus/temp-dir
-    const result =
-      sketchPath.indexOf(prefix) !== -1 && sketchPath.startsWith(tempPath);
-    console.log('isTemp?', result, sketch.uri);
-    return result;
+    return this.isTempSketch.is(FileUri.fsPath(sketch.uri));
   }
 
   async copy(
@@ -578,6 +560,17 @@ void loop() {
     const suffix = crypto.createHash('md5').update(sketchPath).digest('hex');
     return path.join(os.tmpdir(), `arduino-ide2-${suffix}`);
   }
+
+  notifyDeleteSketch(sketch: Sketch): void {
+    const sketchPath = FileUri.fsPath(sketch.uri);
+    fs.rm(sketchPath, { recursive: true, maxRetries: 5 }, (error) => {
+      if (error) {
+        console.error(`Failed to delete sketch at ${sketchPath}.`, error);
+      } else {
+        console.error(`Successfully delete sketch at ${sketchPath}.`);
+      }
+    });
+  }
 }
 
 interface SketchWithDetails extends Sketch {
@@ -618,16 +611,6 @@ function isNotFoundError(err: unknown): err is ServiceError {
   return ServiceError.is(err) && err.code === 5; // `NOT_FOUND` https://grpc.github.io/grpc/core/md_doc_statuscodes.html
 }
 
-/**
- * If on Windows, will change the input `C:\\path\\to\\somewhere` to `c:\\path\\to\\somewhere`.
- */
-function maybeNormalizeDrive(input: string): string {
-  if (isWindows && WIN32_DRIVE_REGEXP.test(input)) {
-    return firstToLowerCase(input);
-  }
-  return input;
-}
-
 /*
  * When a new sketch is created, add a suffix to distinguish it
  * from other new sketches I created today.

diff --git a/arduino-ide-extension/src/node/theia/workspace/default-workspace-server.ts b/arduino-ide-extension/src/node/theia/workspace/default-workspace-server.ts
@@ -1,26 +1,16 @@
 import { promises as fs, constants } from 'fs';
 import { injectable, inject } from '@theia/core/shared/inversify';
-import { ILogger } from '@theia/core/lib/common/logger';
 import { DefaultWorkspaceServer as TheiaDefaultWorkspaceServer } from '@theia/workspace/lib/node/default-workspace-server';
-import { ConfigService } from '../../../common/protocol/config-service';
 import { SketchesService } from '../../../common/protocol';
 import { FileUri } from '@theia/core/lib/node';
+import { IsTempSketch } from '../../is-temp-sketch';
 
 @injectable()
 export class DefaultWorkspaceServer extends TheiaDefaultWorkspaceServer {
-  @inject(ConfigService)
-  protected readonly configService: ConfigService;
-
-  @inject(ILogger)
-  protected readonly logger: ILogger;
-
   @inject(SketchesService)
   private readonly sketchesService: SketchesService;
-
-  override async onStart(): Promise<void> {
-    // NOOP
-    // No need to remove untitled workspaces. IDE2 does not use workspaces.
-  }
+  @inject(IsTempSketch)
+  private readonly isTempSketch: IsTempSketch;
 
   override async getMostRecentlyUsedWorkspace(): Promise<string | undefined> {
     const uri = await super.getMostRecentlyUsedWorkspace();
@@ -51,6 +41,35 @@ export class DefaultWorkspaceServer extends TheiaDefaultWorkspaceServer {
     return listUri;
  }
 
+  protected override async writeToUserHome(
+    data: RecentWorkspacePathsData
+  ): Promise<void> {
+    return super.writeToUserHome(this.filterTempSketches(data));
+  }
+
+  protected override async readRecentWorkspacePathsFromUserHome(): Promise<
+    RecentWorkspacePathsData | undefined
+  > {
+    const data = await super.readRecentWorkspacePathsFromUserHome();
+    return data ? this.filterTempSketches(data) : undefined;
+  }
+
+  protected override async removeOldUntitledWorkspaces(): Promise<void> {
+    // NOOP
+    // No need to remove untitled workspaces. IDE2 does not use workspaces.
+  }
+
+  private filterTempSketches(
+    data: RecentWorkspacePathsData
+  ): RecentWorkspacePathsData {
+    const recentRoots = data.recentRoots.filter(
+      (uri) => !this.isTempSketch.is(FileUri.fsPath(uri))
+    );
+    return {
+      recentRoots,
+    };
+  }
+
   private async exists(uri: string): Promise<boolean> {
     try {
       await fs.access(FileUri.fsPath(uri), constants.R_OK | constants.W_OK);
@@ -60,3 +79,8 @@ export class DefaultWorkspaceServer extends TheiaDefaultWorkspaceServer {
     }
   }
 }
+
+// Remove after https://github.com/eclipse-theia/theia/pull/11603
+interface RecentWorkspacePathsData {
+  recentRoots: string[];
+}