Add FSWatching locator base class

Author: Kartik Raj

src/client/common/platform/fileSystemWatcher.ts

@@ -5,7 +5,7 @@
 
 import * as chokidar from 'chokidar';
 import * as path from 'path';
-import { RelativePattern, workspace } from 'vscode';
+import { Disposable, RelativePattern, workspace } from 'vscode';
 import { traceError, traceVerbose, traceWarning } from '../logger';
 import { normCasePath } from './fs-paths';
 
@@ -22,35 +22,40 @@ const POLLING_INTERVAL = 5000;
 export function watchLocationForPattern(
     baseDir: string,
     pattern: string,
-    callback: (type: FileChangeType, absPath: string) => void
-): void {
+    callback: (type: FileChangeType, absPath: string) => void,
+): Disposable {
     // Use VSCode API iff base directory to exists within the current workspace folders
     const found = workspace.workspaceFolders?.find((e) => normCasePath(baseDir).startsWith(normCasePath(e.uri.fsPath)));
     if (found) {
-        watchLocationUsingVSCodeAPI(baseDir, pattern, callback);
-    } else {
-        // Fallback to chokidar as base directory to lookup doesn't exist within the current workspace folders
-        watchLocationUsingChokidar(baseDir, pattern, callback);
+        return watchLocationUsingVSCodeAPI(baseDir, pattern, callback);
     }
+    // Fallback to chokidar as base directory to lookup doesn't exist within the current workspace folders
+    return watchLocationUsingChokidar(baseDir, pattern, callback);
 }
 
 function watchLocationUsingVSCodeAPI(
     baseDir: string,
     pattern: string,
-    callback: (type: FileChangeType, absPath: string) => void
+    callback: (type: FileChangeType, absPath: string) => void,
 ) {
     const globPattern = new RelativePattern(baseDir, pattern);
+    const disposables: Disposable[] = [];
     traceVerbose(`Start watching: ${baseDir} with pattern ${pattern} using VSCode API`);
     const watcher = workspace.createFileSystemWatcher(globPattern);
-    watcher.onDidCreate((e) => callback(FileChangeType.Created, e.fsPath));
-    watcher.onDidChange((e) => callback(FileChangeType.Changed, e.fsPath));
-    watcher.onDidDelete((e) => callback(FileChangeType.Deleted, e.fsPath));
+    disposables.push(watcher.onDidCreate((e) => callback(FileChangeType.Created, e.fsPath)));
+    disposables.push(watcher.onDidChange((e) => callback(FileChangeType.Changed, e.fsPath)));
+    disposables.push(watcher.onDidDelete((e) => callback(FileChangeType.Deleted, e.fsPath)));
+    return {
+        dispose: async () => {
+            disposables.forEach((d) => d.dispose());
+        },
+    };
 }
 
 function watchLocationUsingChokidar(
     baseDir: string,
     pattern: string,
-    callback: (type: FileChangeType, absPath: string) => void
+    callback: (type: FileChangeType, absPath: string) => void,
 ) {
     const watcherOpts: chokidar.WatchOptions = {
         cwd: baseDir,
@@ -69,9 +74,9 @@ function watchLocationUsingChokidar(
             '**/.hg/store/**',
             '/dev/**',
             '/proc/**',
-            '/sys/**'
+            '/sys/**',
         ], // https://github.com/microsoft/vscode/issues/23954
-        followSymlinks: false
+        followSymlinks: false,
     };
     traceVerbose(`Start watching: ${baseDir} with pattern ${pattern} using chokidar`);
     let watcher: chokidar.FSWatcher | null = chokidar.watch(pattern, watcherOpts);
@@ -96,6 +101,13 @@ function watchLocationUsingChokidar(
         callback(eventType, absPath);
     });
 
+    const dispose = async () => {
+        if (watcher) {
+            await watcher.close();
+            watcher = null;
+        }
+    };
+
     watcher.on('error', async (error: NodeJS.ErrnoException) => {
         if (error) {
             // Specially handle ENOSPC errors that can happen when
@@ -105,13 +117,12 @@ function watchLocationUsingChokidar(
             // See https://github.com/Microsoft/vscode/issues/7950
             if (error.code === 'ENOSPC') {
                 traceError(`Inotify limit reached (ENOSPC) for ${baseDir} with pattern ${pattern}`);
-                if (watcher) {
-                    await watcher.close();
-                    watcher = null;
-                }
+                await dispose();
             } else {
                 traceWarning(error.toString());
             }
         }
     });
+
+    return { dispose };
 }

src/client/pythonEnvironments/base/locator.ts

@@ -1,7 +1,10 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
+// eslint-disable-next-line max-classes-per-file
 import { Event, Uri } from 'vscode';
+import { AsyncDisposableRegistry } from '../../common/asyncDisposableRegistry';
+import { IAsyncDisposableRegistry, IDisposable } from '../../common/types';
 import { iterEmpty } from '../../common/utils/async';
 import { PythonEnvInfo, PythonEnvKind } from './info';
 import {
@@ -178,9 +181,14 @@ interface IEmitter<E extends BasicPythonEnvsChangedEvent> {
  * should be used.  Only in low-level cases should you consider using
  * `BasicPythonEnvsChangedEvent`.
  */
-export abstract class LocatorBase<E extends BasicPythonEnvsChangedEvent = PythonEnvsChangedEvent> implements ILocator<E> {
+export abstract class LocatorBase<E extends BasicPythonEnvsChangedEvent = PythonEnvsChangedEvent>
+implements IDisposable, ILocator<E> {
     public readonly onChanged: Event<E>;
+
     protected readonly emitter: IEmitter<E>;
+
+    protected readonly disposables: IAsyncDisposableRegistry = new AsyncDisposableRegistry();
+
     constructor(watcher: IPythonEnvsWatcher<E> & IEmitter<E>) {
         this.emitter = watcher;
         this.onChanged = watcher.onChanged;
@@ -191,6 +199,10 @@ export abstract class LocatorBase<E extends BasicPythonEnvsChangedEvent = Python
     public async resolveEnv(_env: string | PythonEnvInfo): Promise<PythonEnvInfo | undefined> {
         return undefined;
     }
+
+    public dispose(): void {
+        this.disposables.dispose().ignoreErrors();
+    }
 }
 
 /**

src/client/pythonEnvironments/base/locators/lowLevel/locator.ts

@@ -0,0 +1,14 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+import { Locator } from '../../locator';
+
+/**
+ * The base for Python envs locators who watch the file system.
+ * Most low-level locators should be using this.
+ *
+ * Subclasses can call `this.emitter.fire()` * to emit events.
+ */
+export abstract class FSWatchingLocator extends Locator {
+    public async abstract initialize(): Promise<void>;
+}

src/client/pythonEnvironments/common/pythonBinariesWatcher.ts

@@ -5,6 +5,7 @@
 
 import * as minimatch from 'minimatch';
 import * as path from 'path';
+import { Disposable } from 'vscode';
 import { FileChangeType, watchLocationForPattern } from '../../common/platform/fileSystemWatcher';
 import { getOSType, OSType } from '../../common/utils/platform';
 
@@ -14,17 +15,23 @@ export function watchLocationForPythonBinaries(
     baseDir: string,
     callback: (type: FileChangeType, absPath: string) => void,
     executableGlob: string = executable,
-): void {
+): Disposable {
     const patterns = [executableGlob, `*/${executableGlob}`, `*/${binName}/${executableGlob}`];
+    const disposables: Disposable[] = [];
     for (const pattern of patterns) {
-        watchLocationForPattern(baseDir, pattern, (type: FileChangeType, e: string) => {
+        disposables.push(watchLocationForPattern(baseDir, pattern, (type: FileChangeType, e: string) => {
             const isMatch = minimatch(e, path.join('**', executableGlob), { nocase: getOSType() === OSType.Windows });
             if (!isMatch) {
                 // When deleting the file for some reason path to all directories leading up to python are reported
                 // Skip those events
                 return;
             }
             callback(type, e);
-        });
+        }));
     }
+    return {
+        dispose: async () => {
+            disposables.forEach((d) => d.dispose());
+        },
+    };
 }

src/client/pythonEnvironments/discovery/locators/services/globalVirtualEnvronmentLocator.ts

@@ -7,19 +7,20 @@ import { traceVerbose } from '../../../../common/logger';
 import { FileChangeType } from '../../../../common/platform/fileSystemWatcher';
 import { chain, iterable, sleep } from '../../../../common/utils/async';
 import {
-    getEnvironmentVariable, getOSType, getUserHomeDir, OSType
+    getEnvironmentVariable, getOSType, getUserHomeDir, OSType,
 } from '../../../../common/utils/platform';
 import { PythonEnvInfo, PythonEnvKind, UNKNOWN_PYTHON_VERSION } from '../../../base/info';
 import { buildEnvInfo } from '../../../base/info/env';
-import { IPythonEnvsIterator, Locator } from '../../../base/locator';
+import { IPythonEnvsIterator } from '../../../base/locator';
+import { FSWatchingLocator } from '../../../base/locators/lowLevel/locator';
 import { findInterpretersInDir } from '../../../common/commonUtils';
 import { getFileInfo, pathExists } from '../../../common/externalDependencies';
 import { watchLocationForPythonBinaries } from '../../../common/pythonBinariesWatcher';
 import { isPipenvEnvironment } from './pipEnvHelper';
 import {
     isVenvEnvironment,
     isVirtualenvEnvironment,
-    isVirtualenvwrapperEnvironment
+    isVirtualenvwrapperEnvironment,
 } from './virtualEnvironmentIdentifier';
 
 const DEFAULT_SEARCH_DEPTH = 2;
@@ -80,7 +81,7 @@ async function getVirtualEnvKind(interpreterPath: string): Promise<PythonEnvKind
 /**
  * Finds and resolves virtual environments created in known global locations.
  */
-export class GlobalVirtualEnvironmentLocator extends Locator {
+export class GlobalVirtualEnvironmentLocator extends FSWatchingLocator {
     private virtualEnvKinds = [
         PythonEnvKind.Venv,
         PythonEnvKind.VirtualEnv,
@@ -90,7 +91,10 @@ export class GlobalVirtualEnvironmentLocator extends Locator {
 
     public constructor(private readonly searchDepth?: number) {
         super();
-        this.registerWatchers().ignoreErrors();
+    }
+
+    public async initialize(): Promise<void> {
+        await this.startWatchers();
     }
 
     public iterEnvs(): IPythonEnvsIterator {
@@ -168,15 +172,20 @@ export class GlobalVirtualEnvironmentLocator extends Locator {
         return undefined;
     }
 
-    private async registerWatchers(): Promise<void> {
+    private async startWatchers(): Promise<void> {
         const dirs = await getGlobalVirtualEnvDirs();
-        dirs.forEach((d) => watchLocationForPythonBinaries(d, async (type: FileChangeType, executablePath: string) => {
-            // Note detecting kind of virtual env depends on the file structure around the executable, so we need to
-            // wait before attempting to detect it. However even if the type detected is incorrect, it doesn't do any
-            // practical harm as kinds in this locator are used in the same way (same activation commands etc.)
-            await sleep(1000);
-            const kind = await getVirtualEnvKind(executablePath);
-            this.emitter.fire({ type, kind });
-        }));
+        dirs.forEach(
+            (d) => this.disposables.push(
+                watchLocationForPythonBinaries(d, async (type: FileChangeType, executablePath: string) => {
+                    // Note detecting kind of virtual env depends on the file structure around the
+                    // executable, so we need to wait before attempting to detect it. However even
+                    // if the type detected is incorrect, it doesn't do any practical harm as kinds
+                    // in this locator are used in the same way (same activation commands etc.)
+                    await sleep(1000);
+                    const kind = await getVirtualEnvKind(executablePath);
+                    this.emitter.fire({ type, kind });
+                }),
+            ),
+        );
     }
 }

src/client/pythonEnvironments/discovery/locators/services/windowsStoreLocator.ts

@@ -10,7 +10,8 @@ import { Architecture, getEnvironmentVariable } from '../../../../common/utils/p
 import { PythonEnvInfo, PythonEnvKind } from '../../../base/info';
 import { buildEnvInfo } from '../../../base/info/env';
 import { getPythonVersionFromPath } from '../../../base/info/pythonVersion';
-import { IPythonEnvsIterator, Locator } from '../../../base/locator';
+import { IPythonEnvsIterator } from '../../../base/locator';
+import { FSWatchingLocator } from '../../../base/locators/lowLevel/locator';
 import { getFileInfo } from '../../../common/externalDependencies';
 import { watchLocationForPythonBinaries } from '../../../common/pythonBinariesWatcher';
 
@@ -137,10 +138,10 @@ export async function getWindowsStorePythonExes(): Promise<string[]> {
         .filter(isWindowsStorePythonExe);
 }
 
-export class WindowsStoreLocator extends Locator {
+export class WindowsStoreLocator extends FSWatchingLocator {
     private readonly kind: PythonEnvKind = PythonEnvKind.WindowsStore;
 
-    public initialize(): void {
+    public async initialize(): Promise<void> {
         this.startWatcher();
     }
 
@@ -176,12 +177,14 @@ export class WindowsStoreLocator extends Locator {
 
     private startWatcher(): void {
         const windowsAppsRoot = getWindowsStoreAppsRoot();
-        watchLocationForPythonBinaries(
-            windowsAppsRoot,
-            (type: FileChangeType) => {
-                this.emitter.fire({ type, kind: this.kind });
-            },
-            pythonExeGlob,
+        this.disposables.push(
+            watchLocationForPythonBinaries(
+                windowsAppsRoot,
+                (type: FileChangeType) => {
+                    this.emitter.fire({ type, kind: this.kind });
+                },
+                pythonExeGlob,
+            ),
         );
     }
 }