Eric Snow (ericsnowcurrently) · Dec 10, 2020 · Nov 3, 2020 · Oct 6, 2020 · Nov 3, 2020 · Oct 28, 2020
diff --git a/src/client/common/utils/exec.ts b/src/client/common/utils/exec.ts
@@ -44,12 +44,45 @@ function parseSearchPathEntries(envVarValue: string): string[] {
  * If the file does not exist or has any other problem when accessed
  * then `false` is returned.  The caller is responsible to determine
  * whether or not the file exists.
+ *
+ * If it could not be determined if the file is executable (e.g. on
+ * Windows) then `undefined` is returned.  This allows the caller
+ * to decide what to do.
  */
 export async function isValidAndExecutable(filename: string): Promise<boolean | undefined> {
+    // There are three options when it comes to checking if a file
+    // is executable: `fs.stat()`, `fs.access()`, and
+    // `child_process.exec()`.  `stat()` requires non-trivial logic
+    // to deal with user/group/everyone permissions.  `exec()` requires
+    // that we make an attempt to actually execute the file, which is
+    // beyond the scope of this function (due to potential security
+    // risks).  That leaves `access()`, which is what we use.
     try {
+        // We do not need to check if the file exists.  `fs.access()`
+        // takes care of that for us.
         await fsapi.promises.access(filename, fsapi.constants.X_OK);
     } catch (err) {
         return false;
     }
+    if (getOSType() === OSType.Windows) {
+        // On Windows a file is determined to be executable through
+        // its ACLs.  However, the FS-related functionality provided
+        // by node does not check them (currently).  This includes both
+        // `fs.stat()` and `fs.access()` (which we used above).  One
+        // option is to use the "acl" NPM package (or something similar)
+        // to make the relevant checks.  However, we want to avoid
+        // adding a dependency needlessly.  Another option is to fall
+        // back to checking the filename's suffix (e.g. ".exe").  The
+        // problem there is that such a check is a bit *too* naive.
+        // Finally, we could still go the `exec()` route.  We'd
+        // rather not given the concern identified above.  Instead,
+        // it is good enough to return `undefined` and let the
+        // caller decide what to do about it.  That is better
+        // than returning `true` when we aren't sure.
+        //
+        // Note that we still call `fs.access()` on Windows first,
+        // in case node makes it smarter later.
+        return undefined;
+    }
     return true;
 }
diff --git a/src/client/common/utils/text.ts b/src/client/common/utils/text.ts
@@ -111,3 +111,138 @@ export function parsePosition(raw: string | number): Position {
     }
     return new Position(line, col);
 }
+
+/**
+ * Return the indentation part of the given line.
+ */
+export function getIndent(line: string): string {
+    const found = line.match(/^ */);
+    return found![0];
+}
+
+/**
+ * Return the dedented lines in the given text.
+ *
+ * This is used to represent text concisely and readably, which is
+ * particularly useful for declarative definitions (e.g. in tests).
+ *
+ * (inspired by Python's `textwrap.dedent()`)
+ */
+export function getDedentedLines(text: string): string[] {
+    const linesep = text.includes('\r') ? '\r\n' : '\n';
+    const lines = text.split(linesep);
+    if (!lines) {
+        return [text];
+    }
+
+    if (lines[0] !== '') {
+        throw Error('expected actual first line to be blank');
+    }
+    lines.shift();
+
+    if (lines[0] === '') {
+        throw Error('expected "first" line to not be blank');
+    }
+    const leading = getIndent(lines[0]).length;
+
+    for (let i = 0; i < lines.length; i += 1) {
+        const line = lines[i];
+        if (getIndent(line).length < leading) {
+            throw Error(`line ${i} has less indent than the "first" line`);
+        }
+        lines[i] = line.substring(leading);
+    }
+
+    return lines;
+}
+
+/**
+ * Extract a tree based on the given text.
+ *
+ * The tree is derived from the indent level of each line.  The caller
+ * is responsible for applying any meaning to the text of each node
+ * in the tree.
+ *
+ * Blank lines and comments (with a leading `#`) are ignored.  Also,
+ * the full text is automatically dedented until at least one line
+ * has no indent (i.e. is treated as a root).
+ *
+ * @returns - the list of nodes in the tree (pairs of text & parent index)
+ *            (note that the parent index of roots is `-1`)
+ *
+ * Example:
+ *
+ *   parseTree(`
+ *      # This comment and the following blank line are ignored.
+ *
+ *      this is a root
+ *        the first branch
+ *          a sub-branch  # This comment is ignored.
+ *            this is the first leaf node!
+ *          another leaf node...
+ *          middle
+ *
+ *        the second main branch
+ *            # indents do not have to be consistent across the full text.
+ *           # ...and the indent of comments is not relevant.
+ *            node 1
+ *            node 2
+ *
+ *        the last leaf node!
+ *
+ *      another root
+ *        nothing to see here!
+ *
+ *      # this comment is ignored
+ *   `.trim())
+ *
+ * would produce the following:
+ *
+ *   [
+ *       ['this is a root', -1],
+ *       ['the first branch', 0],
+ *       ['a sub-branch', 1],
+ *       ['this is the first leaf node!', 2],
+ *       ['another leaf node...', 1],
+ *       ['middle', 1],
+ *       ['the second main branch', 0],
+ *       ['node 1', 6],
+ *       ['node 2', 6],
+ *       ['the last leaf node!', 0],
+ *       ['another root', -1],
+ *       ['nothing to see here!', 10],
+ *   ]
+ */
+export function parseTree(text: string): [string, number][] {
+    const parsed: [string, number][] = [];
+    const parents: [string, number][] = [];
+
+    const lines = getDedentedLines(text)
+        .map((l) => l.split('  #')[0].split(' //')[0].trimEnd())
+        .filter((l) => l.trim() !== '');
+    lines.forEach((line) => {
+        const indent = getIndent(line);
+        const entry = line.trim();
+
+        let parentIndex: number;
+        if (indent === '') {
+            parentIndex = -1;
+            parents.push([indent, parsed.length]);
+        } else if (parsed.length === 0) {
+            throw Error(`expected non-indented line, got ${line}`);
+        } else {
+            let parentIndent: string;
+            [parentIndent, parentIndex] = parents[parents.length - 1];
+            while (indent.length <= parentIndent.length) {
+                parents.pop();
+                [parentIndent, parentIndex] = parents[parents.length - 1];
+            }
+            if (parentIndent.length < indent.length) {
+                parents.push([indent, parsed.length]);
+            }
+        }
+        parsed.push([entry, parentIndex!]);
+    });
+
+    return parsed;
+}
diff --git a/src/client/pythonEnvironments/base/envsCache.ts b/src/client/pythonEnvironments/base/envsCache.ts
@@ -65,7 +65,8 @@ export class PythonEnvsCache {
         if (this.byExecutable === undefined) {
             this.byExecutable = {};
             for (const env of this.envs) {
-                this.byExecutable[env.executable.filename] = env;
+                const key = getEnvExecutable(env.executable.filename);
+                this.byExecutable[key] = env;
             }
         }
         return this.byExecutable[executable];

diff --git a/src/client/pythonEnvironments/base/locators/lowLevel/filesLocator.ts b/src/client/pythonEnvironments/base/locators/lowLevel/filesLocator.ts
@@ -1,8 +1,12 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
+// tslint:disable-next-line:no-single-line-block-comment
+/* eslint-disable max-classes-per-file */
+
 import { Event, EventEmitter } from 'vscode';
-import { normalizeFilename } from '../../../../common/utils/filesystem';
+import { findInterpretersInDir } from '../../../common/commonUtils';
+import { normalizePath } from '../../../common/externalDependencies';
 import { PythonEnvInfo, PythonEnvKind } from '../../info';
 import { getFastEnvInfo } from '../../info/env';
 import {
@@ -13,6 +17,7 @@ import {
 } from '../../locator';
 import { iterAndUpdateEnvs, resolveEnvFromIterator } from '../../locatorUtils';
 import { PythonEnvsChangedEvent, PythonEnvsWatcher } from '../../watcher';
+import { FSWatchingLocator } from './fsWatchingLocator';
 
 /**
  * A naive locator the wraps a function that finds Python executables.
@@ -25,13 +30,8 @@ export class FoundFilesLocator implements ILocator {
     constructor(
         private readonly kind: PythonEnvKind,
         private readonly getExecutables: () => Promise<string[]> | AsyncIterableIterator<string>,
-        onUpdated?: Event<void>,
     ) {
         this.onChanged = this.watcher.onChanged;
-
-        if (onUpdated !== undefined) {
-            onUpdated(() => this.watcher.fire({}));
-        }
     }
 
     // eslint-disable-next-line @typescript-eslint/no-unused-vars
@@ -66,7 +66,37 @@ async function* iterMinimalEnvsFromExecutables(
     kind: PythonEnvKind,
 ): AsyncIterableIterator<PythonEnvInfo> {
     for await (const filename of executables) {
-        const executable = normalizeFilename(filename);
+        const executable = normalizePath(filename);
         yield getFastEnvInfo(kind, executable);
     }
 }
+
+/**
+ * A locator for executables in a single directory.
+ */
+export class DirFilesLocator extends FSWatchingLocator {
+    private readonly subLocator: ILocator;
+
+    constructor(
+        dirname: string,
+        kind: PythonEnvKind,
+        getExecutables: (dir: string) => AsyncIterableIterator<string> = findInterpretersInDir,
+    ) {
+        super(
+            () => [dirname],
+            async () => kind,
+        );
+        this.subLocator = new FoundFilesLocator(
+            kind,
+            () => getExecutables(dirname),
+        );
+    }
+
+    protected doIterEnvs(query: PythonLocatorQuery): IPythonEnvsIterator {
+        return this.subLocator.iterEnvs(query);
+    }
+
+    protected async doResolveEnv(env: string | PythonEnvInfo): Promise<PythonEnvInfo | undefined> {
+        return this.subLocator.resolveEnv(env);
+    }
+}
diff --git a/src/client/pythonEnvironments/base/locators/lowLevel/windowsKnownPathsLocator.ts b/src/client/pythonEnvironments/base/locators/lowLevel/windowsKnownPathsLocator.ts
@@ -0,0 +1,90 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+// tslint:disable-next-line:no-single-line-block-comment
+/* eslint-disable max-classes-per-file */
+
+import { Event } from 'vscode';
+import { getSearchPathEntries } from '../../../../common/utils/exec';
+import { Disposables, IDisposable } from '../../../../common/utils/resourceLifecycle';
+import { isStandardPythonBinary } from '../../../common/commonUtils';
+import { PythonEnvInfo, PythonEnvKind } from '../../info';
+import {
+    ILocator,
+    IPythonEnvsIterator,
+    PythonLocatorQuery,
+} from '../../locator';
+import { Locators } from '../../locators';
+import { getEnvs } from '../../locatorUtils';
+import { PythonEnvsChangedEvent } from '../../watcher';
+import { DirFilesLocator } from './filesLocator';
+
+/**
+ * A locator for Windows locators found under the $PATH env var.
+ *
+ * Note that we assume $PATH won't change, so we don't need to watch
+ * it for changes.
+ */
+export class WindowsPathEnvVarLocator implements ILocator, IDisposable {
+    public readonly onChanged: Event<PythonEnvsChangedEvent>;
+
+    private readonly locators: Locators;
+
+    private readonly disposables = new Disposables();
+
+    constructor() {
+        const dirLocators: (ILocator & IDisposable)[] = getSearchPathEntries()
+            .map((dirname) => getDirFilesLocator(dirname, PythonEnvKind.Unknown));
+        this.disposables.push(...dirLocators);
+        this.locators = new Locators(dirLocators);
+        this.onChanged = this.locators.onChanged;
+    }
+
+    public async dispose(): Promise<void> {
+        this.locators.dispose();
+        await this.disposables.dispose();
+    }
+
+    public iterEnvs(query?: PythonLocatorQuery): IPythonEnvsIterator {
+        // Note that we do no filtering here, including to check if files
+        // are valid executables.  That is left to callers (e.g. composite
+        // locators).
+        return this.locators.iterEnvs(query);
+    }
+
+    public async resolveEnv(env: string | PythonEnvInfo): Promise<PythonEnvInfo | undefined> {
+        return this.locators.resolveEnv(env);
+    }
+}
+
+function getDirFilesLocator(
+    dirname: string,
+    kind: PythonEnvKind,
+): ILocator & IDisposable {
+    const locator = new DirFilesLocator(dirname, kind);
+    // Really we should be checking for symlinks or something more
+    // sophisticated.  Also, this should be done in ReducingLocator
+    // rather than in each low-level locator.  In the meantime we
+    // take a naive approach.
+    async function* iterEnvs(query: PythonLocatorQuery): IPythonEnvsIterator {
+        const envs = await getEnvs(locator.iterEnvs(query));
+        for (const env of envs) {
+            if (isStandardPythonBinary(env.executable?.filename || '')) {
+                yield env;
+            }
+        }
+    }
+    async function resolveEnv(env: string | PythonEnvInfo): Promise<PythonEnvInfo | undefined> {
+        const executable = typeof env === 'string' ? env : env.executable?.filename || '';
+        if (!isStandardPythonBinary(executable)) {
+            return undefined;
+        }
+        return locator.resolveEnv(env);
+    }
+    return {
+        iterEnvs,
+        resolveEnv,
+        onChanged: locator.onChanged,
+        dispose: () => locator.dispose(),
+    };
+}