Skip to content

Commit

Permalink
Allow to run for different shells
Browse files Browse the repository at this point in the history
  • Loading branch information
Kartik Raj committed Feb 24, 2023
1 parent 83d6489 commit 5dac08c
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 34 deletions.
34 changes: 19 additions & 15 deletions src/client/common/terminal/shellDetectors/baseShellDetector.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,22 +54,26 @@ export abstract class BaseShellDetector implements IShellDetector {
terminal?: Terminal,
): TerminalShellType | undefined;
public identifyShellFromShellPath(shellPath: string): TerminalShellType {
// Remove .exe extension so shells can be more consistently detected
// on Windows (including Cygwin).
const basePath = shellPath.replace(/\.exe$/, '');
return identifyShellFromShellPath(shellPath);
}
}

const shell = Array.from(detectableShells.keys()).reduce((matchedShell, shellToDetect) => {
if (matchedShell === TerminalShellType.other) {
const pat = detectableShells.get(shellToDetect);
if (pat && pat.test(basePath)) {
return shellToDetect;
}
export function identifyShellFromShellPath(shellPath: string): TerminalShellType {
// Remove .exe extension so shells can be more consistently detected
// on Windows (including Cygwin).
const basePath = shellPath.replace(/\.exe$/, '');

const shell = Array.from(detectableShells.keys()).reduce((matchedShell, shellToDetect) => {
if (matchedShell === TerminalShellType.other) {
const pat = detectableShells.get(shellToDetect);
if (pat && pat.test(basePath)) {
return shellToDetect;
}
return matchedShell;
}, TerminalShellType.other);
}
return matchedShell;
}, TerminalShellType.other);

traceVerbose(`Shell path '${shellPath}', base path '${basePath}'`);
traceVerbose(`Shell path identified as shell '${shell}'`);
return shell;
}
traceVerbose(`Shell path '${shellPath}', base path '${basePath}'`);
traceVerbose(`Shell path identified as shell '${shell}'`);
return shell;
}
43 changes: 35 additions & 8 deletions src/client/interpreter/activation/service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import '../../common/extensions';
import { inject, injectable } from 'inversify';

import { ProgressLocation, ProgressOptions } from 'vscode';
import { IApplicationShell, IWorkspaceService } from '../../common/application/types';
import { IApplicationEnvironment, IApplicationShell, IWorkspaceService } from '../../common/application/types';
import { PYTHON_WARNINGS } from '../../common/constants';
import { IPlatformService } from '../../common/platform/types';
import * as internalScripts from '../../common/process/internal/scripts';
Expand Down Expand Up @@ -39,6 +39,7 @@ import { StopWatch } from '../../common/utils/stopWatch';
import { Interpreters } from '../../common/utils/localize';
import { IExtensionSingleActivationService } from '../../activation/types';
import { inTerminalEnvVarExperiment } from '../../common/experiments/helpers';
import { identifyShellFromShellPath } from '../../common/terminal/shellDetectors/baseShellDetector';

const ENVIRONMENT_PREFIX = 'e8b39361-0157-4923-80e1-22d70d46dee6';
const CACHE_DURATION = 10 * 60 * 1000;
Expand Down Expand Up @@ -139,6 +140,7 @@ export class EnvironmentActivationService
@inject(IExtensionContext) private context: IExtensionContext,
@inject(IApplicationShell) private shell: IApplicationShell,
@inject(IExperimentService) private experimentService: IExperimentService,
@inject(IApplicationEnvironment) private applicationEnvironment: IApplicationEnvironment,
) {
this.envVarsService.onDidEnvironmentVariablesChange(
() => this.activatedEnvVariablesCache.clear(),
Expand All @@ -155,14 +157,14 @@ export class EnvironmentActivationService
async (resource) => {
this.showProgress();
this.activatedEnvVariablesCache.clear();
await this.initializeEnvironmentCollection(resource);
await this.applyCollection(resource);
this.hideProgress();
},
this,
this.disposables,
);

this.initializeEnvironmentCollection(undefined).ignoreErrors();
this.initializeCollection(undefined).ignoreErrors();
}

private isEnvCollectionEnabled() {
Expand All @@ -178,21 +180,22 @@ export class EnvironmentActivationService
resource: Resource,
interpreter?: PythonEnvironment,
allowExceptions?: boolean,
shell?: string,
): Promise<NodeJS.ProcessEnv | undefined> {
const stopWatch = new StopWatch();
// Cache key = resource + interpreter.
const workspaceKey = this.workspace.getWorkspaceFolderIdentifier(resource);
interpreter = interpreter ?? (await this.interpreterService.getActiveInterpreter(resource));
const interpreterPath = this.platform.isWindows ? interpreter?.path.toLowerCase() : interpreter?.path;
const cacheKey = `${workspaceKey}_${interpreterPath}`;
const cacheKey = `${workspaceKey}_${interpreterPath}_${shell}`;

if (this.activatedEnvVariablesCache.get(cacheKey)?.hasData) {
return this.activatedEnvVariablesCache.get(cacheKey)!.data;
}

// Cache only if successful, else keep trying & failing if necessary.
const cache = new InMemoryCache<NodeJS.ProcessEnv | undefined>(CACHE_DURATION);
return this.getActivatedEnvironmentVariablesImpl(resource, interpreter, allowExceptions)
return this.getActivatedEnvironmentVariablesImpl(resource, interpreter, allowExceptions, shell)
.then((vars) => {
cache.data = vars;
this.activatedEnvVariablesCache.set(cacheKey, cache);
Expand Down Expand Up @@ -228,11 +231,16 @@ export class EnvironmentActivationService
resource: Resource,
interpreter?: PythonEnvironment,
allowExceptions?: boolean,
shell?: string,
): Promise<NodeJS.ProcessEnv | undefined> {
const shellInfo = defaultShells[this.platform.osType];
let shellInfo = defaultShells[this.platform.osType];
if (!shellInfo) {
return undefined;
}
if (shell) {
const customShellType = identifyShellFromShellPath(shell);
shellInfo = { shellType: customShellType, shell };
}
try {
let command: string | undefined;
const [args, parse] = internalScripts.printEnvVariables();
Expand Down Expand Up @@ -368,9 +376,15 @@ export class EnvironmentActivationService
return parse(js);
}

private async initializeEnvironmentCollection(resource: Resource) {
const env = await this.getActivatedEnvironmentVariables(resource);
private async applyCollection(resource: Resource, shell?: string) {
const env = await this.getActivatedEnvironmentVariables(resource, undefined, undefined, shell);
if (!env) {
if (shell) {
// Default shells are known to work, hence we can safely ignore errors. However commands to fetch
// env vars may fail in custom shells due to unknown reasons, so do not clear collection even on
// failure.
return;
}
this.context.environmentVariableCollection.clear();
this.previousEnvVars = normCaseKeys(process.env);
return;
Expand Down Expand Up @@ -399,6 +413,19 @@ export class EnvironmentActivationService
});
}

private async initializeCollection(resource: Resource) {
await this.applyCollection(resource);
await this.applyCollectionForSelectedShell(resource);
}

private async applyCollectionForSelectedShell(resource: Resource) {
const customShellType = identifyShellFromShellPath(this.applicationEnvironment.shell);
if (customShellType !== defaultShells[this.platform.osType]?.shellType) {
// If the user has a custom shell which different from default shell, we need to re-apply the environment collection.
await this.applyCollection(resource, this.applicationEnvironment.shell);
}
}

@traceDecoratorVerbose('Display activating terminals')
private showProgress(): void {
if (!this.deferred) {
Expand Down
27 changes: 16 additions & 11 deletions src/client/pythonEnvironments/common/environmentManagers/conda.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { isTestExecution } from '../../../common/constants';
import { traceError, traceVerbose } from '../../../logging';
import { OUTPUT_MARKER_SCRIPT } from '../../../common/process/internal/scripts';
import { splitLines } from '../../../common/stringUtils';
import { SpawnOptions } from '../../../common/process/types';

export const AnacondaCompanyName = 'Anaconda, Inc.';
export const CONDAPATH_SETTING_KEY = 'condaPath';
Expand Down Expand Up @@ -248,7 +249,7 @@ export class Conda {
* need a Conda instance should use getConda() to obtain it, and should never access
* this property directly.
*/
private static condaPromise: Promise<Conda | undefined> | undefined;
private static condaPromise = new Map<string | undefined, Promise<Conda | undefined>>();

private condaInfoCached: Promise<CondaInfo> | undefined;

Expand All @@ -263,18 +264,18 @@ export class Conda {
* @param command - Command used to spawn conda. This has the same meaning as the
* first argument of spawn() - i.e. it can be a full path, or just a binary name.
*/
constructor(readonly command: string, shellCommand?: string) {
constructor(readonly command: string, shellCommand?: string, private readonly shellPath?: string) {
this.shellCommand = shellCommand ?? command;
onDidChangePythonSetting(CONDAPATH_SETTING_KEY, () => {
Conda.condaPromise = undefined;
Conda.condaPromise = new Map<string | undefined, Promise<Conda | undefined>>();
});
}

public static async getConda(): Promise<Conda | undefined> {
if (Conda.condaPromise === undefined || isTestExecution()) {
Conda.condaPromise = Conda.locate();
public static async getConda(shellPath?: string): Promise<Conda | undefined> {
if (Conda.condaPromise.get(shellPath) === undefined || isTestExecution()) {
Conda.condaPromise.set(shellPath, Conda.locate(shellPath));
}
return Conda.condaPromise;
return Conda.condaPromise.get(shellPath);
}

/**
Expand All @@ -283,7 +284,7 @@ export class Conda {
*
* @return A Conda instance corresponding to the binary, if successful; otherwise, undefined.
*/
private static async locate(): Promise<Conda | undefined> {
private static async locate(shellPath?: string): Promise<Conda | undefined> {
traceVerbose(`Searching for conda.`);
const home = getUserHomeDir();
const customCondaPath = getPythonSetting<string>(CONDAPATH_SETTING_KEY);
Expand Down Expand Up @@ -392,9 +393,9 @@ export class Conda {
const condaBatFile = await getCondaBatFile(condaPath);
try {
if (condaBatFile) {
const condaBat = new Conda(condaBatFile);
const condaBat = new Conda(condaBatFile, undefined, shellPath);
await condaBat.getInfo();
conda = new Conda(condaPath, condaBatFile);
conda = new Conda(condaPath, condaBatFile, shellPath);
}
} catch (ex) {
traceVerbose('Failed to spawn conda bat file', condaBatFile, ex);
Expand Down Expand Up @@ -432,7 +433,11 @@ export class Conda {
@cache(30_000, true, 10_000)
// eslint-disable-next-line class-methods-use-this
private async getInfoImpl(command: string): Promise<CondaInfo> {
const result = await exec(command, ['info', '--json'], { timeout: CONDA_GENERAL_TIMEOUT });
const options: SpawnOptions = { timeout: CONDA_GENERAL_TIMEOUT };
if (this.shellPath) {
options.shell = this.shellPath;
}
const result = await exec(command, ['info', '--json'], options);
traceVerbose(`${command} info --json: ${result.stdout}`);
return JSON.parse(result.stdout);
}
Expand Down

0 comments on commit 5dac08c

Please sign in to comment.