Skip to content

Commit

Permalink
Get activate env vars for non-conda as well (#9163)
Browse files Browse the repository at this point in the history
  • Loading branch information
DonJayamanne committed Mar 3, 2022
1 parent f64fef4 commit 5d3190d
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 3 deletions.
12 changes: 10 additions & 2 deletions src/client/datascience/kernel-launcher/kernelEnvVarsService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { noop } from '../../common/utils/misc';
import { IEnvironmentVariablesProvider, IEnvironmentVariablesService } from '../../common/variables/types';
import { IEnvironmentActivationService } from '../../interpreter/activation/types';
import { IInterpreterService } from '../../interpreter/contracts';
import { EnvironmentType, PythonEnvironment } from '../../pythonEnvironments/info';
import { PythonEnvironment } from '../../pythonEnvironments/info';
import { IJupyterKernelSpec } from '../types';

@injectable()
Expand Down Expand Up @@ -51,7 +51,7 @@ export class KernelEnvironmentVariablesService {

let [customEditVars, interpreterEnv, hasActivationCommands] = await Promise.all([
this.customEnvVars.getCustomEnvironmentVariables(resource).catch(noop),
interpreter && interpreter.envType == EnvironmentType.Conda
interpreter
? this.envActivation
.getActivatedEnvironmentVariables(resource, interpreter, false)
.catch<undefined>((ex) => {
Expand All @@ -63,6 +63,14 @@ export class KernelEnvironmentVariablesService {
]);
if (!interpreterEnv && Object.keys(customEditVars || {}).length === 0) {
traceInfo('No custom variables nor do we have a conda environment');
// Ensure the python env folder is always at the top of the PATH, this way all executables from that env are used.
// This way shell commands such as `!pip`, `!python` end up pointing to the right executables.
// Also applies to `!java` where java could be an executable in the conda bin directory.
if (interpreter) {
const env = kernelEnv || process.env;
this.envVarsService.prependPath(env, path.dirname(interpreter.path));
return env;
}
return kernelEnv;
}
// Merge the env variables with that of the kernel env.
Expand Down
30 changes: 29 additions & 1 deletion src/test/datascience/notebook/executionService.vscode.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { Common } from '../../../client/common/utils/localize';
import { IVSCodeNotebook } from '../../../client/common/application/types';
import { traceInfo, traceInfoIfCI } from '../../../client/common/logger';
import { IDisposable, Product } from '../../../client/common/types';
import { captureScreenShot, IExtensionTestApi, waitForCondition } from '../../common';
import { captureScreenShot, getOSType, IExtensionTestApi, OSType, waitForCondition } from '../../common';
import { EXTENSION_ROOT_DIR_FOR_TESTS, initialize } from '../../initialize';
import {
closeNotebooksAndCleanUpAfterTests,
Expand Down Expand Up @@ -406,6 +406,34 @@ suite('DataScience - VSCode Notebook - (Execution) (slow)', function () {
`First entry in PATH (${pathValue[0]}) does not point to executable (${sysExecutable})`
);
});
test('!python should point to the Environment', async function () {
if (IS_REMOTE_NATIVE_TEST) {
return this.skip();
}
await insertCodeCell(getOSType() === OSType.Windows ? '!where python' : '!which python', { index: 0 });
await insertCodeCell('import sys', { index: 1 });
await insertCodeCell('print(sys.executable)', { index: 2 });
const [cell1, , cell3] = vscodeNotebook.activeNotebookEditor!.document.getCells()!;

// Basically anything such as `!which python` and the like should point to the right executable.
// For that to work, the first directory in the PATH must be the Python environment.

await Promise.all([
runAllCellsInActiveNotebook(),
waitForExecutionCompletedSuccessfully(cell3),
waitForCondition(async () => getCellOutputs(cell3).length > 0, defaultNotebookTestTimeout, 'No output')
]);

// On windows `!where python`, prints multiple items in the output (all executables found).
const shellExecutable = getCellOutputs(cell1).trim().split('\n')[0].trim();
const sysExecutable = getCellOutputs(cell3).trim();

// First path in PATH must be the directory where executable is located.
assert.ok(
areInterpreterPathsSame(shellExecutable.toLowerCase(), sysExecutable.toLowerCase()),
`Python paths do not match ${shellExecutable}, ${sysExecutable}`
);
});
test('Testing streamed output', async () => {
// Assume you are executing a cell that prints numbers 1-100.
// When printing number 50, you click clear.
Expand Down

0 comments on commit 5d3190d

Please sign in to comment.