Revert "Add a locator for executables on $PATH on Windows. (#14981)

This reverts commit 651a6b9.

Co-authored-by: Karthik Nadig <kanadig@microsoft.com>

Author: Kartik Raj

src/client/common/utils/exec.ts

@@ -44,45 +44,12 @@ 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;
 }

src/client/common/utils/text.ts

@@ -111,138 +111,3 @@ 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;
-}

src/client/pythonEnvironments/base/envsCache.ts

@@ -65,8 +65,7 @@ export class PythonEnvsCache {
         if (this.byExecutable === undefined) {
             this.byExecutable = {};
             for (const env of this.envs) {
-                const key = getEnvExecutable(env.executable.filename);
-                this.byExecutable[key] = env;
+                this.byExecutable[env.executable.filename] = env;
             }
         }
         return this.byExecutable[executable];

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

@@ -1,12 +1,8 @@
 // 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 { findInterpretersInDir } from '../../../common/commonUtils';
-import { normalizePath } from '../../../common/externalDependencies';
+import { normalizeFilename } from '../../../../common/utils/filesystem';
 import { PythonEnvInfo, PythonEnvKind } from '../../info';
 import { getFastEnvInfo } from '../../info/env';
 import {
@@ -17,7 +13,6 @@ 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.
@@ -30,8 +25,13 @@ 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,37 +66,7 @@ async function* iterMinimalEnvsFromExecutables(
     kind: PythonEnvKind,
 ): AsyncIterableIterator<PythonEnvInfo> {
     for await (const filename of executables) {
-        const executable = normalizePath(filename);
+        const executable = normalizeFilename(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);
-    }
-}