Skip to content

Commit

Permalink
Ensure workspace interpreters are discovered and watched (#17144)
Browse files Browse the repository at this point in the history
  • Loading branch information
Kartik Raj authored Aug 31, 2021
1 parent 7adee50 commit 71114fa
Show file tree
Hide file tree
Showing 8 changed files with 35 additions and 13 deletions.
1 change: 1 addition & 0 deletions news/2 Fixes/17144.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Ensure workspace interpreters are discovered and watched when in `pythonDiscoveryModuleWithoutWatcher` experiment.
4 changes: 4 additions & 0 deletions src/client/common/platform/fs-paths.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,3 +156,7 @@ export function normCasePath(filePath: string): string {
export function isParentPath(filePath: string, parentPath: string): boolean {
return normCasePath(filePath).startsWith(normCasePath(parentPath));
}

export function arePathsSame(path1: string, path2: string): boolean {
return normCasePath(path1) === normCasePath(path2);
}
9 changes: 5 additions & 4 deletions src/client/common/utils/misc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import type { TextDocument, Uri } from 'vscode';
import { InteractiveInputScheme, NotebookCellScheme } from '../constants';
import { InterpreterUri } from '../installer/types';
import { arePathsSame, isParentPath } from '../platform/fs-paths';
import { Resource } from '../types';
import { isPromise } from './async';
import { StopWatch } from './stopWatch';
Expand Down Expand Up @@ -126,15 +127,15 @@ export function getURIFilter(
while (candidate.path.endsWith('/')) {
candidatePath = candidatePath.slice(0, -1);
}
if (opts.checkExact && candidatePath === uriPath) {
if (opts.checkExact && arePathsSame(candidatePath, uriPath)) {
return true;
}
if (opts.checkParent && candidatePath.startsWith(uriRoot)) {
if (opts.checkParent && isParentPath(candidatePath, uriRoot)) {
return true;
}
if (opts.checkChild) {
const candidateRoot = `{candidatePath}/`;
if (uriPath.startsWith(candidateRoot)) {
const candidateRoot = `${candidatePath}/`;
if (isParentPath(uriPath, candidateRoot)) {
return true;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT License.

import { Event } from 'vscode';
import { traceInfo } from '../../../../common/logger';
import { asyncFilter } from '../../../../common/utils/arrayUtils';
import { pathExists } from '../../../common/externalDependencies';
import { PythonEnvInfo } from '../../info';
Expand Down Expand Up @@ -112,6 +113,7 @@ export class PythonEnvInfoCache extends PythonEnvsWatcher<PythonEnvCollectionCha

public async flush(): Promise<void> {
if (this.envs.length) {
traceInfo('Environments added to cache', JSON.stringify(this.envs));
await this.persistentStorage.store(this.envs);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import * as fs from 'fs';
import * as path from 'path';
import { Uri } from 'vscode';
import { DiscoveryVariants } from '../../../../common/experiments/groups';
import { traceError, traceVerbose } from '../../../../common/logger';
import { FileChangeType } from '../../../../common/platform/fileSystemWatcher';
import { sleep } from '../../../../common/utils/async';
import { logError } from '../../../../logging';
Expand Down Expand Up @@ -33,6 +34,7 @@ function checkDirWatchable(dirname: string): DirUnwatchableReason {
try {
names = fs.readdirSync(dirname);
} catch (err) {
traceError('Reading directory to watch failed', err);
if (err.code === 'ENOENT') {
// We treat a missing directory as watchable since it should
// be watchable if created later.
Expand Down Expand Up @@ -92,12 +94,15 @@ export abstract class FSWatchingLocator<I = PythonEnvInfo> extends LazyResourceB
// Enable global watchers only if the experiment allows it.
const enableGlobalWatchers = await inExperiment(DiscoveryVariants.discoverWithFileWatching);
if (!enableGlobalWatchers) {
traceVerbose('Watcher disabled');
return;
}
}

// Start the FS watchers.
traceVerbose('Getting roots');
let roots = await this.getRoots();
traceVerbose('Found roots');
if (typeof roots === 'string') {
roots = [roots];
}
Expand All @@ -106,13 +111,12 @@ export abstract class FSWatchingLocator<I = PythonEnvInfo> extends LazyResourceB
// that might be watched due to a glob are not checked.
const unwatchable = await checkDirWatchable(root);
if (unwatchable) {
logError(`dir "${root}" is not watchable (${unwatchable})`);
logError(`Dir "${root}" is not watchable (${unwatchable})`);
return undefined;
}
return root;
});
const watchableRoots = (await Promise.all(promises)).filter((root) => !!root) as string[];

watchableRoots.forEach((root) => this.startWatchers(root));
}

Expand Down Expand Up @@ -147,6 +151,7 @@ export abstract class FSWatchingLocator<I = PythonEnvInfo> extends LazyResourceB
// The structure determines which globs are returned.
this.opts.envStructure,
);
traceVerbose('Start watching root', root, 'for globs', JSON.stringify(globs));
const watchers = globs.map((g) => watchLocationForPythonBinaries(root, callback, g));
this.disposables.push(...watchers);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { traceError, traceVerbose } from '../../../../common/logger';
import { chain, iterable } from '../../../../common/utils/async';
import { PythonEnvKind } from '../../info';
import { BasicEnvInfo, IPythonEnvsIterator } from '../../locator';
import { FSWatchingLocator } from './fsWatchingLocator';
import { FSWatcherKind, FSWatchingLocator } from './fsWatchingLocator';
import { getInterpreterPathFromDir } from '../../../common/commonUtils';
import { pathExists } from '../../../common/externalDependencies';
import { isPoetryEnvironment, localPoetryEnvDirName, Poetry } from '../../../common/environmentManagers/poetry';
Expand Down Expand Up @@ -65,6 +65,8 @@ export class PoetryLocator extends FSWatchingLocator<BasicEnvInfo> {
super(
() => getRootVirtualEnvDir(root),
async () => PythonEnvKind.Poetry,
undefined,
FSWatcherKind.Workspace,
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { isPipenvEnvironment } from '../../../common/environmentManagers/pipenv'
import { isVenvEnvironment, isVirtualenvEnvironment } from '../../../common/environmentManagers/simplevirtualenvs';
import { PythonEnvKind } from '../../info';
import { BasicEnvInfo, IPythonEnvsIterator } from '../../locator';
import { FSWatchingLocator } from './fsWatchingLocator';
import { FSWatcherKind, FSWatchingLocator } from './fsWatchingLocator';
import '../../../../common/extensions';
import { asyncFilter } from '../../../../common/utils/arrayUtils';

Expand Down Expand Up @@ -52,11 +52,16 @@ async function getVirtualEnvKind(interpreterPath: string): Promise<PythonEnvKind
*/
export class WorkspaceVirtualEnvironmentLocator extends FSWatchingLocator<BasicEnvInfo> {
public constructor(private readonly root: string) {
super(() => getWorkspaceVirtualEnvDirs(this.root), getVirtualEnvKind, {
// Note detecting kind of virtual env depends on the file structure around the
// executable, so we need to wait before attempting to detect it.
delayOnCreated: 1000,
});
super(
() => getWorkspaceVirtualEnvDirs(this.root),
getVirtualEnvKind,
{
// Note detecting kind of virtual env depends on the file structure around the
// executable, so we need to wait before attempting to detect it.
delayOnCreated: 1000,
},
FSWatcherKind.Workspace,
);
}

protected doIterEnvs(): IPythonEnvsIterator<BasicEnvInfo> {
Expand Down
2 changes: 2 additions & 0 deletions src/client/pythonEnvironments/common/pythonBinariesWatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import * as minimatch from 'minimatch';
import * as path from 'path';
import { traceVerbose } from '../../common/logger';
import { FileChangeType, watchLocationForPattern } from '../../common/platform/fileSystemWatcher';
import { getOSType, OSType } from '../../common/utils/platform';
import { IDisposable } from '../../common/utils/resourceLifecycle';
Expand All @@ -26,6 +27,7 @@ export function watchLocationForPythonBinaries(
const resolvedGlob = path.posix.normalize(executableGlob);
const [baseGlob] = resolvedGlob.split('/').slice(-1);
function callbackClosure(type: FileChangeType, e: string) {
traceVerbose('Received event', JSON.stringify(e), 'for baseglob', baseGlob);
const isMatch = minimatch(path.basename(e), baseGlob, { nocase: getOSType() === OSType.Windows });
if (!isMatch) {
// When deleting the file for some reason path to all directories leading up to python are reported
Expand Down

0 comments on commit 71114fa

Please sign in to comment.