Skip to content

Ignore directories with the same name as python binaries #15367

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Karthik Nadig (karthiknadig) merged 2 commits into from
Feb 9, 2021
Merged

Ignore directories with the same name as python binaries #15367

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
972649f
Ignore directories with the same name as pyhton binaries
karthiknadig Feb 9, 2021
9fcae82
Use withFileTypes instead of lstat
karthiknadig Feb 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/client/pythonEnvironments/common/commonUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ import { comparePythonVersionSpecificity } from '../base/info/env';
import { parseVersion } from '../base/info/pythonVersion';
import { getPythonVersionFromConda } from '../discovery/locators/services/conda';
import { getPythonVersionFromPyvenvCfg } from '../discovery/locators/services/virtualEnvironmentIdentifier';
import { isPosixPythonBin } from './posixUtils';
import { isPosixPythonBinPattern } from './posixUtils';
import { isWindowsPythonExe } from './windowsUtils';

const matchPythonExecutable = getOSType() === OSType.Windows ? isWindowsPythonExe : isPosixPythonBin;
const matchPythonExecutable = getOSType() === OSType.Windows ? isWindowsPythonExe : isPosixPythonBinPattern;

type FileFilterFunc = (filename: string) => boolean;

Expand All @@ -33,7 +33,7 @@ export async function* findInterpretersInDir(
): AsyncIterableIterator<string> {
// "checkBin" is a local variable rather than global
// so we can stub it out during unit testing.
const checkBin = getOSType() === OSType.Windows ? isWindowsPythonExe : isPosixPythonBin;
const checkBin = getOSType() === OSType.Windows ? isWindowsPythonExe : isPosixPythonBinPattern;
const cfg = {
ignoreErrors,
filterSubDir,
Expand Down
2 changes: 1 addition & 1 deletion src/client/pythonEnvironments/common/posixUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { getSearchPathEntries } from '../../common/utils/exec';
* @param {string} interpreterPath : Path to python interpreter.
* @returns {boolean} : Returns true if the path matches pattern for windows python executable.
*/
export function isPosixPythonBin(interpreterPath: string): boolean {
export function isPosixPythonBinPattern(interpreterPath: string): boolean {
/**
* This Reg-ex matches following file names:
* python
Expand Down
28 changes: 16 additions & 12 deletions src/client/pythonEnvironments/discovery/locators/services/posixKnownPathsLocator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,25 +11,29 @@ import { buildEnvInfo } from '../../../base/info/env';
import { parseVersion } from '../../../base/info/pythonVersion';
import { IPythonEnvsIterator, Locator } from '../../../base/locator';
import { getFileInfo, resolveSymbolicLink } from '../../../common/externalDependencies';
import { commonPosixBinPaths, isPosixPythonBin } from '../../../common/posixUtils';
import { commonPosixBinPaths, isPosixPythonBinPattern } from '../../../common/posixUtils';

async function getPythonBinFromKnownPaths(): Promise<string[]> {
const knownPaths = await commonPosixBinPaths();
const pythonBins: Set<string> = new Set();
for (const knownPath of knownPaths) {
const files = (await fsapi.readdir(knownPath))
const paths = (await fsapi.readdir(knownPath))

@karrtikr Kartik Raj (karrtikr) Feb 9, 2021

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you use the following API we wouldn't need a lstat call,

vscode-python/src/client/pythonEnvironments/common/commonUtils.ts

Line 142 in b2545de

raw = await fs.promises.readdir(dirname, { withFileTypes: true });

Or we can directly call getExecutablesDefault like windows path locator is doing.

@karrtikr Kartik Raj (karrtikr) Feb 9, 2021

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the comment below, on non-Windows the file type of each entry is preserved for free, so it doesn't run lstat.

@karthiknadig Karthik Nadig (karthiknadig) Feb 9, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes... forgot about this. Thanks.

.map((filename: string) => path.join(knownPath, filename))
.filter(isPosixPythonBin);
.filter(isPosixPythonBinPattern);

for (const file of files) {
// Ensure that we have a collection of unique global binaries by
// resolving all symlinks to the target binaries.
try {
const resolvedBin = await resolveSymbolicLink(file);
pythonBins.add(resolvedBin);
traceInfo(`Found: ${file} --> ${resolvedBin}`);
} catch (ex) {
traceError('Failed to resolve symbolic link: ', ex);
for (const filepath of paths) {
// Skip any directories
const stats = await fsapi.lstat(filepath);
if (!stats.isDirectory()) {
// Ensure that we have a collection of unique global binaries by
// resolving all symlinks to the target binaries.
try {
const resolvedBin = await resolveSymbolicLink(filepath);
pythonBins.add(resolvedBin);
traceInfo(`Found: ${filepath} --> ${resolvedBin}`);
} catch (ex) {
traceError('Failed to resolve symbolic link: ', ex);
}
}
}
}
Expand Down
1 change: 1 addition & 0 deletions src/test/pythonEnvironments/common/envlayouts/posixroot/location3/python3.9/empty
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
this is intentionally empty