Skip to content

Commit

Permalink
Ensure interpreter path isn't truncated for workspace-relative paths …
Browse files Browse the repository at this point in the history
…when storing value (#20661)

For #20660

I'm not quite sure why this was done. It doesn't make sense to do this
only for display.
  • Loading branch information
Kartik Raj committed Feb 7, 2023
1 parent 377067f commit 02a92fc
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 61 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import * as path from 'path';
import { ConfigurationTarget, Uri } from 'vscode';
import { IInterpreterPathService } from '../../../common/types';
import { IPythonPathUpdaterService } from '../types';
Expand All @@ -11,9 +10,6 @@ export class WorkspaceFolderPythonPathUpdaterService implements IPythonPathUpdat
if (pythonPathValue && pythonPathValue.workspaceFolderValue === pythonPath) {
return;
}
if (pythonPath && pythonPath.startsWith(this.workspaceFolder.fsPath)) {
pythonPath = path.relative(this.workspaceFolder.fsPath, pythonPath);
}
await this.interpreterPathService.update(this.workspaceFolder, ConfigurationTarget.WorkspaceFolder, pythonPath);
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import * as path from 'path';
import { ConfigurationTarget, Uri } from 'vscode';
import { IInterpreterPathService } from '../../../common/types';
import { IPythonPathUpdaterService } from '../types';
Expand All @@ -11,9 +10,6 @@ export class WorkspacePythonPathUpdaterService implements IPythonPathUpdaterServ
if (pythonPathValue && pythonPathValue.workspaceValue === pythonPath) {
return;
}
if (pythonPath && pythonPath.startsWith(this.workspace.fsPath)) {
pythonPath = path.relative(this.workspace.fsPath, pythonPath);
}
await this.interpreterPathService.update(this.workspace, ConfigurationTarget.Workspace, pythonPath);
}
}
42 changes: 21 additions & 21 deletions src/client/interpreter/interpreterService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,28 +186,28 @@ export class InterpreterService implements Disposable, IInterpreterService {
// However we need not wait on the update to take place, as we can use the value directly.
if (!path) {
path = this.configService.getSettings(resource).pythonPath;
}
if (pathUtils.basename(path) === path) {
// Value can be `python`, `python3`, `python3.9` etc.
// Note the following triggers autoselection if no interpreter is explictly
// selected, i.e the value is `python`.
// During shutdown we might not be able to get items out of the service container.
const pythonExecutionFactory = this.serviceContainer.tryGet<IPythonExecutionFactory>(
IPythonExecutionFactory,
);
const pythonExecutionService = pythonExecutionFactory
? await pythonExecutionFactory.create({ resource })
: undefined;
const fullyQualifiedPath = pythonExecutionService
? await pythonExecutionService.getExecutablePath().catch((ex) => {
traceError(ex);
})
: undefined;
// Python path is invalid or python isn't installed.
if (!fullyQualifiedPath) {
return undefined;
if (pathUtils.basename(path) === path) {
// Value can be `python`, `python3`, `python3.9` etc.
// Note the following triggers autoselection if no interpreter is explictly
// selected, i.e the value is `python`.
// During shutdown we might not be able to get items out of the service container.
const pythonExecutionFactory = this.serviceContainer.tryGet<IPythonExecutionFactory>(
IPythonExecutionFactory,
);
const pythonExecutionService = pythonExecutionFactory
? await pythonExecutionFactory.create({ resource })
: undefined;
const fullyQualifiedPath = pythonExecutionService
? await pythonExecutionService.getExecutablePath().catch((ex) => {
traceError(ex);
})
: undefined;
// Python path is invalid or python isn't installed.
if (!fullyQualifiedPath) {
return undefined;
}
path = fullyQualifiedPath;
}
path = fullyQualifiedPath;
}
return this.getInterpreterDetails(path);
}
Expand Down
32 changes: 0 additions & 32 deletions src/test/interpreters/pythonPathUpdaterFactory.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,21 +94,6 @@ suite('Python Path Settings Updater', () => {
await updater.updatePythonPath(pythonPath);
interpreterPathService.verifyAll();
});
test('Python Path should be truncated for workspace-relative paths', async () => {
const workspaceFolderPath = path.join('user', 'desktop', 'development');
const workspaceFolder = Uri.file(workspaceFolderPath);
const pythonPath = Uri.file(path.join(workspaceFolderPath, 'env', 'bin', 'python')).fsPath;
const expectedPythonPath = path.join('env', 'bin', 'python');
interpreterPathService.setup((i) => i.inspect(workspaceFolder)).returns(() => ({}));
interpreterPathService
.setup((i) => i.update(workspaceFolder, ConfigurationTarget.WorkspaceFolder, expectedPythonPath))
.returns(() => Promise.resolve())
.verifiable(TypeMoq.Times.once());

const updater = updaterServiceFactory.getWorkspaceFolderPythonPathConfigurationService(workspaceFolder);
await updater.updatePythonPath(pythonPath);
interpreterPathService.verifyAll();
});
});
suite('Workspace (multiroot scenario)', () => {
setup(() => setupMocks());
Expand Down Expand Up @@ -142,23 +127,6 @@ suite('Python Path Settings Updater', () => {
const updater = updaterServiceFactory.getWorkspacePythonPathConfigurationService(workspaceFolder);
await updater.updatePythonPath(pythonPath);

interpreterPathService.verifyAll();
});
test('Python Path should be truncated for workspace-relative paths', async () => {
const workspaceFolderPath = path.join('user', 'desktop', 'development');
const workspaceFolder = Uri.file(workspaceFolderPath);
const pythonPath = Uri.file(path.join(workspaceFolderPath, 'env', 'bin', 'python')).fsPath;
const expectedPythonPath = path.join('env', 'bin', 'python');

interpreterPathService.setup((i) => i.inspect(workspaceFolder)).returns(() => ({}));
interpreterPathService
.setup((i) => i.update(workspaceFolder, ConfigurationTarget.Workspace, expectedPythonPath))
.returns(() => Promise.resolve())
.verifiable(TypeMoq.Times.once());

const updater = updaterServiceFactory.getWorkspacePythonPathConfigurationService(workspaceFolder);
await updater.updatePythonPath(pythonPath);

interpreterPathService.verifyAll();
});
});
Expand Down

0 comments on commit 02a92fc

Please sign in to comment.