Skip to content
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.

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

Add extra logging regarding interpreter discovery #21639

Merged
merged 5 commits into from
Jul 17, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
4 changes: 3 additions & 1 deletion src/client/common/process/rawProcessApis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ export function plainExec(
const deferred = createDeferred<ExecutionResult<string>>();
const disposable: IDisposable = {
dispose: () => {
if (!proc.killed && !deferred.completed) {
if (!proc.killed && deferred.completed) {
Copy link
Author

Choose a reason for hiding this comment

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

@karthiknadig Not clear on why it was the way it was,

proc.kill();
}
},
Expand Down Expand Up @@ -156,10 +156,12 @@ export function plainExec(
deferred.resolve({ stdout, stderr });
}
internalDisposables.forEach((d) => d.dispose());
disposable.dispose();
Copy link
Author

Choose a reason for hiding this comment

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

@karthiknadig Hope forcefully killing processes after execution finishes is okay

Copy link
Member

Choose a reason for hiding this comment

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

This is technically not when execution is completed, but rather when dispose is called. But yes, it should be fine.

Copy link
Author

@karrtikr karrtikr Jul 17, 2023

Choose a reason for hiding this comment

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

This is executed on proc.once('close', which is run just after execution completes.

});
proc.once('error', (ex) => {
deferred.reject(ex);
internalDisposables.forEach((d) => d.dispose());
disposable.dispose();
});

return deferred.promise;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export class ActiveStateLocator extends LazyResourceBasedLocator {
traceVerbose(`Couldn't locate the state binary.`);
return;
}
traceVerbose(`Searching for active state environments`);
const projects = await state.getProjects();
if (projects === undefined) {
traceVerbose(`Couldn't fetch State Tool projects.`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ export class MicrosoftStoreLocator extends FSWatchingLocator {

protected doIterEnvs(): IPythonEnvsIterator<BasicEnvInfo> {
const iterator = async function* (kind: PythonEnvKind) {
traceVerbose('Searching for windows store envs');
const exes = await getMicrosoftStorePythonExes();
yield* exes.map(async (executablePath: string) => ({
kind,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export class PosixKnownPathsLocator extends Locator<BasicEnvInfo> {
}

const iterator = async function* (kind: PythonEnvKind) {
traceVerbose('Searching for interpreters in posix paths locator');
// Filter out pyenv shims. They are not actual python binaries, they are used to launch
// the binaries specified in .python-version file in the cwd. We should not be reporting
// those binaries as environments.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { traceError, traceVerbose } from '../../../../logging';
* all the environments (global or virtual) in that directory.
*/
async function* getPyenvEnvironments(): AsyncIterableIterator<BasicEnvInfo> {
traceVerbose('Searching for pyenv environments');
const pyenvVersionDir = getPyenvVersionsDir();

const subDirs = getSubDirs(pyenvVersionDir, { resolveSymlinks: true });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ function getDirFilesLocator(
// rather than in each low-level locator. In the meantime we
// take a naive approach.
async function* iterEnvs(query: PythonLocatorQuery): IPythonEnvsIterator<BasicEnvInfo> {
traceVerbose('Searching for windows path interpreters');
yield* await getEnvs(locator.iterEnvs(query));
traceVerbose('Finished searching for windows path interpreters');
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export class WindowsRegistryLocator extends Locator<BasicEnvInfo> {
// eslint-disable-next-line class-methods-use-this
public iterEnvs(): IPythonEnvsIterator<BasicEnvInfo> {
const iterator = async function* () {
traceVerbose('Searching for windows registry interpreters');
const interpreters = await getRegistryInterpreters();
for (const interpreter of interpreters) {
try {
Expand Down