Skip to content

Commit 133d019

Browse files
karthiknadigKartik Raj
karthiknadig
authored and
Kartik Raj
committed
Revert "Add a locator for executables on $PATH on Windows. (microsoft#14675) (microsoft#14951)" (microsoft#14968)
This reverts commit 651a6b9.
1 parent 07a8272 commit 133d019

18 files changed

Lines changed: 226 additions & 1122 deletions

File tree

‎src/client/common/utils/exec.ts‎

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -44,45 +44,12 @@ function parseSearchPathEntries(envVarValue: string): string[] {
4444
* If the file does not exist or has any other problem when accessed
4545
* then `false` is returned. The caller is responsible to determine
4646
* whether or not the file exists.
47-
*
48-
* If it could not be determined if the file is executable (e.g. on
49-
* Windows) then `undefined` is returned. This allows the caller
50-
* to decide what to do.
5147
*/
5248
export async function isValidAndExecutable(filename: string): Promise<boolean | undefined> {
53-
// There are three options when it comes to checking if a file
54-
// is executable: `fs.stat()`, `fs.access()`, and
55-
// `child_process.exec()`. `stat()` requires non-trivial logic
56-
// to deal with user/group/everyone permissions. `exec()` requires
57-
// that we make an attempt to actually execute the file, which is
58-
// beyond the scope of this function (due to potential security
59-
// risks). That leaves `access()`, which is what we use.
6049
try {
61-
// We do not need to check if the file exists. `fs.access()`
62-
// takes care of that for us.
6350
await fsapi.promises.access(filename, fsapi.constants.X_OK);
6451
} catch (err) {
6552
return false;
6653
}
67-
if (getOSType() === OSType.Windows) {
68-
// On Windows a file is determined to be executable through
69-
// its ACLs. However, the FS-related functionality provided
70-
// by node does not check them (currently). This includes both
71-
// `fs.stat()` and `fs.access()` (which we used above). One
72-
// option is to use the "acl" NPM package (or something similar)
73-
// to make the relevant checks. However, we want to avoid
74-
// adding a dependency needlessly. Another option is to fall
75-
// back to checking the filename's suffix (e.g. ".exe"). The
76-
// problem there is that such a check is a bit *too* naive.
77-
// Finally, we could still go the `exec()` route. We'd
78-
// rather not given the concern identified above. Instead,
79-
// it is good enough to return `undefined` and let the
80-
// caller decide what to do about it. That is better
81-
// than returning `true` when we aren't sure.
82-
//
83-
// Note that we still call `fs.access()` on Windows first,
84-
// in case node makes it smarter later.
85-
return undefined;
86-
}
8754
return true;
8855
}

‎src/client/common/utils/text.ts‎

Lines changed: 0 additions & 135 deletions
Original file line numberDiff line numberDiff line change
@@ -111,138 +111,3 @@ export function parsePosition(raw: string | number): Position {
111111
}
112112
return new Position(line, col);
113113
}
114-
115-
/**
116-
* Return the indentation part of the given line.
117-
*/
118-
export function getIndent(line: string): string {
119-
const found = line.match(/^ */);
120-
return found![0];
121-
}
122-
123-
/**
124-
* Return the dedented lines in the given text.
125-
*
126-
* This is used to represent text concisely and readably, which is
127-
* particularly useful for declarative definitions (e.g. in tests).
128-
*
129-
* (inspired by Python's `textwrap.dedent()`)
130-
*/
131-
export function getDedentedLines(text: string): string[] {
132-
const linesep = text.includes('\r') ? '\r\n' : '\n';
133-
const lines = text.split(linesep);
134-
if (!lines) {
135-
return [text];
136-
}
137-
138-
if (lines[0] !== '') {
139-
throw Error('expected actual first line to be blank');
140-
}
141-
lines.shift();
142-
143-
if (lines[0] === '') {
144-
throw Error('expected "first" line to not be blank');
145-
}
146-
const leading = getIndent(lines[0]).length;
147-
148-
for (let i = 0; i < lines.length; i += 1) {
149-
const line = lines[i];
150-
if (getIndent(line).length < leading) {
151-
throw Error(`line ${i} has less indent than the "first" line`);
152-
}
153-
lines[i] = line.substring(leading);
154-
}
155-
156-
return lines;
157-
}
158-
159-
/**
160-
* Extract a tree based on the given text.
161-
*
162-
* The tree is derived from the indent level of each line. The caller
163-
* is responsible for applying any meaning to the text of each node
164-
* in the tree.
165-
*
166-
* Blank lines and comments (with a leading `#`) are ignored. Also,
167-
* the full text is automatically dedented until at least one line
168-
* has no indent (i.e. is treated as a root).
169-
*
170-
* @returns - the list of nodes in the tree (pairs of text & parent index)
171-
* (note that the parent index of roots is `-1`)
172-
*
173-
* Example:
174-
*
175-
* parseTree(`
176-
* # This comment and the following blank line are ignored.
177-
*
178-
* this is a root
179-
* the first branch
180-
* a sub-branch # This comment is ignored.
181-
* this is the first leaf node!
182-
* another leaf node...
183-
* middle
184-
*
185-
* the second main branch
186-
* # indents do not have to be consistent across the full text.
187-
* # ...and the indent of comments is not relevant.
188-
* node 1
189-
* node 2
190-
*
191-
* the last leaf node!
192-
*
193-
* another root
194-
* nothing to see here!
195-
*
196-
* # this comment is ignored
197-
* `.trim())
198-
*
199-
* would produce the following:
200-
*
201-
* [
202-
* ['this is a root', -1],
203-
* ['the first branch', 0],
204-
* ['a sub-branch', 1],
205-
* ['this is the first leaf node!', 2],
206-
* ['another leaf node...', 1],
207-
* ['middle', 1],
208-
* ['the second main branch', 0],
209-
* ['node 1', 6],
210-
* ['node 2', 6],
211-
* ['the last leaf node!', 0],
212-
* ['another root', -1],
213-
* ['nothing to see here!', 10],
214-
* ]
215-
*/
216-
export function parseTree(text: string): [string, number][] {
217-
const parsed: [string, number][] = [];
218-
const parents: [string, number][] = [];
219-
220-
const lines = getDedentedLines(text)
221-
.map((l) => l.split(' #')[0].split(' //')[0].trimEnd())
222-
.filter((l) => l.trim() !== '');
223-
lines.forEach((line) => {
224-
const indent = getIndent(line);
225-
const entry = line.trim();
226-
227-
let parentIndex: number;
228-
if (indent === '') {
229-
parentIndex = -1;
230-
parents.push([indent, parsed.length]);
231-
} else if (parsed.length === 0) {
232-
throw Error(`expected non-indented line, got ${line}`);
233-
} else {
234-
let parentIndent: string;
235-
[parentIndent, parentIndex] = parents[parents.length - 1];
236-
while (indent.length <= parentIndent.length) {
237-
parents.pop();
238-
[parentIndent, parentIndex] = parents[parents.length - 1];
239-
}
240-
if (parentIndent.length < indent.length) {
241-
parents.push([indent, parsed.length]);
242-
}
243-
}
244-
parsed.push([entry, parentIndex!]);
245-
});
246-
247-
return parsed;
248-
}

‎src/client/pythonEnvironments/base/envsCache.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,7 @@ export class PythonEnvsCache {
6565
if (this.byExecutable === undefined) {
6666
this.byExecutable = {};
6767
for (const env of this.envs) {
68-
const key = getEnvExecutable(env.executable.filename);
69-
this.byExecutable[key] = env;
68+
this.byExecutable[env.executable.filename] = env;
7069
}
7170
}
7271
return this.byExecutable[executable];

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

Lines changed: 7 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,8 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
33

4-
// tslint:disable-next-line:no-single-line-block-comment
5-
/* eslint-disable max-classes-per-file */
6-
74
import { Event, EventEmitter } from 'vscode';
8-
import { findInterpretersInDir } from '../../../common/commonUtils';
9-
import { normalizePath } from '../../../common/externalDependencies';
5+
import { normalizeFilename } from '../../../../common/utils/filesystem';
106
import { PythonEnvInfo, PythonEnvKind } from '../../info';
117
import { getFastEnvInfo } from '../../info/env';
128
import {
@@ -17,7 +13,6 @@ import {
1713
} from '../../locator';
1814
import { iterAndUpdateEnvs, resolveEnvFromIterator } from '../../locatorUtils';
1915
import { PythonEnvsChangedEvent, PythonEnvsWatcher } from '../../watcher';
20-
import { FSWatchingLocator } from './fsWatchingLocator';
2116

2217
/**
2318
* A naive locator the wraps a function that finds Python executables.
@@ -30,8 +25,13 @@ export class FoundFilesLocator implements ILocator {
3025
constructor(
3126
private readonly kind: PythonEnvKind,
3227
private readonly getExecutables: () => Promise<string[]> | AsyncIterableIterator<string>,
28+
onUpdated?: Event<void>,
3329
) {
3430
this.onChanged = this.watcher.onChanged;
31+
32+
if (onUpdated !== undefined) {
33+
onUpdated(() => this.watcher.fire({}));
34+
}
3535
}
3636

3737
// eslint-disable-next-line @typescript-eslint/no-unused-vars
@@ -66,37 +66,7 @@ async function* iterMinimalEnvsFromExecutables(
6666
kind: PythonEnvKind,
6767
): AsyncIterableIterator<PythonEnvInfo> {
6868
for await (const filename of executables) {
69-
const executable = normalizePath(filename);
69+
const executable = normalizeFilename(filename);
7070
yield getFastEnvInfo(kind, executable);
7171
}
7272
}
73-
74-
/**
75-
* A locator for executables in a single directory.
76-
*/
77-
export class DirFilesLocator extends FSWatchingLocator {
78-
private readonly subLocator: ILocator;
79-
80-
constructor(
81-
dirname: string,
82-
kind: PythonEnvKind,
83-
getExecutables: (dir: string) => AsyncIterableIterator<string> = findInterpretersInDir,
84-
) {
85-
super(
86-
() => [dirname],
87-
async () => kind,
88-
);
89-
this.subLocator = new FoundFilesLocator(
90-
kind,
91-
() => getExecutables(dirname),
92-
);
93-
}
94-
95-
protected doIterEnvs(query: PythonLocatorQuery): IPythonEnvsIterator {
96-
return this.subLocator.iterEnvs(query);
97-
}
98-
99-
protected async doResolveEnv(env: string | PythonEnvInfo): Promise<PythonEnvInfo | undefined> {
100-
return this.subLocator.resolveEnv(env);
101-
}
102-
}

‎src/client/pythonEnvironments/base/locators/lowLevel/windowsKnownPathsLocator.ts‎

Lines changed: 0 additions & 90 deletions
This file was deleted.

0 commit comments

Comments
 (0)