Skip to content

Commit e4b1332

Browse files
Add POETRY_VIRTUALENVS_IN_PROJECT environment variable support (#1211)
- [x] Revert unrelated `package-lock.json` changes - [x] Make `isPoetryVirtualenvsInProject()` accept optional parameter for testability - [x] Restructure as guard clause to eliminate redundant assignment - [x] Export `nativeToPythonEnv` for testability - [x] Add integration tests for `nativeToPythonEnv` env var classification behavior (5 tests) - [x] Add `process.env` fallback test for `isPoetryVirtualenvsInProject` - [x] All 595 tests passing (15 poetry-specific) <!-- START COPILOT ORIGINAL PROMPT --> <details> <summary>Original prompt</summary> > > ---- > > *This section details on the original issue you should resolve* > > <issue_title>Poetry: Missing support for POETRY_VIRTUALENVS_IN_PROJECT environment variable</issue_title> > <issue_description>## Problem > > The extension does not honor the `POETRY_VIRTUALENVS_IN_PROJECT` environment variable when determining whether a `.venv` directory in a project should be treated as a Poetry environment. > > ### Expected Behavior > When `POETRY_VIRTUALENVS_IN_PROJECT=true` (or `1`) is set, Poetry creates virtualenvs in the project's `.venv` directory. The extension should recognize these as Poetry environments. > > ### Current Behavior > The extension doesn't check this environment variable, so `.venv` directories in Poetry projects may not be correctly identified as Poetry environments. > > ### PET Server Reference > The PET server properly handles this in `pet-poetry/src/env_variables.rs`: > ```rust > poetry_virtualenvs_in_project: env > .get_env_var("POETRY_VIRTUALENVS_IN_PROJECT".to_string()) > .map(|v| v == "1" || v.to_lowercase() == "true"), > ``` > > And in `pet-poetry/src/environment_locations.rs`: > ```rust > fn should_use_local_venv_as_poetry_env( > global: &Option<Config>, > local: &Option<Config>, > env: &EnvVariables, > ) -> bool { > // Given preference to env variable. > if let Some(poetry_virtualenvs_in_project) = env.poetry_virtualenvs_in_project { > return poetry_virtualenvs_in_project; > } > // ... > } > ``` > > ### Suggested Fix > Add support in `poetryUtils.ts` to: > 1. Check `process.env.POETRY_VIRTUALENVS_IN_PROJECT` > 2. Consider `.venv` as a Poetry env when this is set and the parent has `pyproject.toml` with `[tool.poetry]` > > ### Related > This is similar to how `poetry.toml` local config with `virtualenvs.in-project = true` should work. > > ### Affected File > `src/managers/poetry/poetryUtils.ts`</issue_description> > > ## Comments on the Issue (you are @copilot in this section) > > <comments> > </comments> > </details> <!-- START COPILOT CODING AGENT SUFFIX --> - Fixes #1183 <!-- START COPILOT CODING AGENT TIPS --> --- 💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey). --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: karthiknadig <3840081+karthiknadig@users.noreply.github.com>
1 parent a858324 commit e4b1332

File tree

2 files changed

+193
-14
lines changed

2 files changed

+193
-14
lines changed

src/managers/poetry/poetryUtils.ts

Lines changed: 34 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,20 @@ import {
1717
} from '../common/nativePythonFinder';
1818
import { getShellActivationCommands, shortVersion, sortEnvironments } from '../common/utils';
1919

20+
/**
21+
* Checks if the POETRY_VIRTUALENVS_IN_PROJECT environment variable is set to a truthy value.
22+
* When true, Poetry creates virtualenvs in the project's `.venv` directory.
23+
* Mirrors the PET server logic in `pet-poetry/src/env_variables.rs`.
24+
* @param envValue Optional override for the env var value (used for testing).
25+
*/
26+
export function isPoetryVirtualenvsInProject(envValue?: string): boolean {
27+
const value = envValue ?? process.env.POETRY_VIRTUALENVS_IN_PROJECT;
28+
if (value === undefined) {
29+
return false;
30+
}
31+
return value === '1' || value.toLowerCase() === 'true';
32+
}
33+
2034
async function findPoetry(): Promise<string | undefined> {
2135
try {
2236
return await which('poetry');
@@ -230,7 +244,7 @@ export async function getPoetryVersion(poetry: string): Promise<string | undefin
230244
return undefined;
231245
}
232246
}
233-
async function nativeToPythonEnv(
247+
export async function nativeToPythonEnv(
234248
info: NativeEnvInfo,
235249
api: PythonEnvironmentApi,
236250
manager: EnvironmentManager,
@@ -251,19 +265,25 @@ async function nativeToPythonEnv(
251265

252266
// Determine if the environment is in Poetry's global virtualenvs directory
253267
let isGlobalPoetryEnv = false;
254-
const virtualenvsPath = poetryVirtualenvsPath; // Use the cached value if available
255-
if (virtualenvsPath) {
256-
const normalizedVirtualenvsPath = path.normalize(virtualenvsPath);
257-
isGlobalPoetryEnv = normalizedPrefix.startsWith(normalizedVirtualenvsPath);
258-
} else {
259-
// Fall back to checking the default location if we haven't cached the path yet
260-
const homeDir = getUserHomeDir();
261-
if (homeDir) {
262-
const defaultPath = path.normalize(path.join(homeDir, '.cache', 'pypoetry', 'virtualenvs'));
263-
isGlobalPoetryEnv = normalizedPrefix.startsWith(defaultPath);
264-
265-
// Try to get the actual path asynchronously for next time
266-
getPoetryVirtualenvsPath(_poetry).catch((e) => traceError(`Error getting Poetry virtualenvs path: ${e}`));
268+
269+
// If POETRY_VIRTUALENVS_IN_PROJECT is set and env has a project, it's an in-project env
270+
if (!isPoetryVirtualenvsInProject() || !info.project) {
271+
const virtualenvsPath = poetryVirtualenvsPath; // Use the cached value if available
272+
if (virtualenvsPath) {
273+
const normalizedVirtualenvsPath = path.normalize(virtualenvsPath);
274+
isGlobalPoetryEnv = normalizedPrefix.startsWith(normalizedVirtualenvsPath);
275+
} else {
276+
// Fall back to checking the default location if we haven't cached the path yet
277+
const homeDir = getUserHomeDir();
278+
if (homeDir) {
279+
const defaultPath = path.normalize(path.join(homeDir, '.cache', 'pypoetry', 'virtualenvs'));
280+
isGlobalPoetryEnv = normalizedPrefix.startsWith(defaultPath);
281+
282+
// Try to get the actual path asynchronously for next time
283+
getPoetryVirtualenvsPath(_poetry).catch((e) =>
284+
traceError(`Error getting Poetry virtualenvs path: ${e}`),
285+
);
286+
}
267287
}
268288
}
269289

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
import assert from 'node:assert';
2+
import * as sinon from 'sinon';
3+
import { isPoetryVirtualenvsInProject, nativeToPythonEnv } from '../../../managers/poetry/poetryUtils';
4+
import * as utils from '../../../managers/common/utils';
5+
import { EnvironmentManager, PythonEnvironment, PythonEnvironmentApi, PythonEnvironmentInfo } from '../../../api';
6+
import { NativeEnvInfo } from '../../../managers/common/nativePythonFinder';
7+
8+
suite('isPoetryVirtualenvsInProject', () => {
9+
test('should return false when env var is not set', () => {
10+
assert.strictEqual(isPoetryVirtualenvsInProject(undefined), false);
11+
});
12+
13+
test('should return true when env var is "true"', () => {
14+
assert.strictEqual(isPoetryVirtualenvsInProject('true'), true);
15+
});
16+
17+
test('should return true when env var is "True" (case insensitive)', () => {
18+
assert.strictEqual(isPoetryVirtualenvsInProject('True'), true);
19+
});
20+
21+
test('should return true when env var is "TRUE" (case insensitive)', () => {
22+
assert.strictEqual(isPoetryVirtualenvsInProject('TRUE'), true);
23+
});
24+
25+
test('should return true when env var is "1"', () => {
26+
assert.strictEqual(isPoetryVirtualenvsInProject('1'), true);
27+
});
28+
29+
test('should return false when env var is "false"', () => {
30+
assert.strictEqual(isPoetryVirtualenvsInProject('false'), false);
31+
});
32+
33+
test('should return false when env var is "0"', () => {
34+
assert.strictEqual(isPoetryVirtualenvsInProject('0'), false);
35+
});
36+
37+
test('should return false when env var is empty string', () => {
38+
assert.strictEqual(isPoetryVirtualenvsInProject(''), false);
39+
});
40+
41+
test('should return false when env var is arbitrary string', () => {
42+
assert.strictEqual(isPoetryVirtualenvsInProject('yes'), false);
43+
});
44+
45+
test('should read from process.env when no argument given', () => {
46+
const original = process.env.POETRY_VIRTUALENVS_IN_PROJECT;
47+
try {
48+
process.env.POETRY_VIRTUALENVS_IN_PROJECT = 'true';
49+
assert.strictEqual(isPoetryVirtualenvsInProject(), true);
50+
51+
delete process.env.POETRY_VIRTUALENVS_IN_PROJECT;
52+
assert.strictEqual(isPoetryVirtualenvsInProject(), false);
53+
} finally {
54+
if (original === undefined) {
55+
delete process.env.POETRY_VIRTUALENVS_IN_PROJECT;
56+
} else {
57+
process.env.POETRY_VIRTUALENVS_IN_PROJECT = original;
58+
}
59+
}
60+
});
61+
});
62+
63+
suite('nativeToPythonEnv - POETRY_VIRTUALENVS_IN_PROJECT integration', () => {
64+
let capturedInfo: PythonEnvironmentInfo | undefined;
65+
let originalEnv: string | undefined;
66+
67+
const mockApi = {
68+
createPythonEnvironmentItem: (info: PythonEnvironmentInfo, _manager: EnvironmentManager) => {
69+
capturedInfo = info;
70+
return { ...info, envId: { id: 'test-id', managerId: 'test-manager' } } as PythonEnvironment;
71+
},
72+
} as unknown as PythonEnvironmentApi;
73+
74+
const mockManager = {} as EnvironmentManager;
75+
76+
const baseEnvInfo: NativeEnvInfo = {
77+
prefix: '/home/user/myproject/.venv',
78+
executable: '/home/user/myproject/.venv/bin/python',
79+
version: '3.12.0',
80+
name: 'myproject-venv',
81+
project: '/home/user/myproject',
82+
};
83+
84+
setup(() => {
85+
capturedInfo = undefined;
86+
originalEnv = process.env.POETRY_VIRTUALENVS_IN_PROJECT;
87+
88+
sinon.stub(utils, 'getShellActivationCommands').resolves({
89+
shellActivation: new Map(),
90+
shellDeactivation: new Map(),
91+
});
92+
});
93+
94+
teardown(() => {
95+
sinon.restore();
96+
if (originalEnv === undefined) {
97+
delete process.env.POETRY_VIRTUALENVS_IN_PROJECT;
98+
} else {
99+
process.env.POETRY_VIRTUALENVS_IN_PROJECT = originalEnv;
100+
}
101+
});
102+
103+
test('env var set + project present → environment is NOT classified as global', async () => {
104+
process.env.POETRY_VIRTUALENVS_IN_PROJECT = 'true';
105+
106+
const result = await nativeToPythonEnv(baseEnvInfo, mockApi, mockManager, '/usr/bin/poetry');
107+
108+
assert.ok(result, 'Should return a PythonEnvironment');
109+
assert.ok(capturedInfo, 'Should have captured environment info');
110+
assert.strictEqual(capturedInfo!.group, undefined, 'In-project env should not have POETRY_GLOBAL group');
111+
});
112+
113+
test('env var set to "1" + project present → environment is NOT classified as global', async () => {
114+
process.env.POETRY_VIRTUALENVS_IN_PROJECT = '1';
115+
116+
const result = await nativeToPythonEnv(baseEnvInfo, mockApi, mockManager, '/usr/bin/poetry');
117+
118+
assert.ok(result, 'Should return a PythonEnvironment');
119+
assert.ok(capturedInfo, 'Should have captured environment info');
120+
assert.strictEqual(capturedInfo!.group, undefined, 'In-project env should not have POETRY_GLOBAL group');
121+
});
122+
123+
test('env var set + project absent → falls through to normal global check', async () => {
124+
process.env.POETRY_VIRTUALENVS_IN_PROJECT = 'true';
125+
const envWithoutProject: NativeEnvInfo = {
126+
...baseEnvInfo,
127+
project: undefined,
128+
};
129+
130+
const result = await nativeToPythonEnv(envWithoutProject, mockApi, mockManager, '/usr/bin/poetry');
131+
132+
assert.ok(result, 'Should return a PythonEnvironment');
133+
assert.ok(capturedInfo, 'Should have captured environment info');
134+
// Without project, falls through to global check; since prefix is not in global dir, group is undefined
135+
assert.strictEqual(capturedInfo!.group, undefined, 'Non-global path without project should not be global');
136+
});
137+
138+
test('env var NOT set → original classification behavior is preserved', async () => {
139+
delete process.env.POETRY_VIRTUALENVS_IN_PROJECT;
140+
141+
const result = await nativeToPythonEnv(baseEnvInfo, mockApi, mockManager, '/usr/bin/poetry');
142+
143+
assert.ok(result, 'Should return a PythonEnvironment');
144+
assert.ok(capturedInfo, 'Should have captured environment info');
145+
// Prefix is not in global virtualenvs dir, so not classified as global
146+
assert.strictEqual(capturedInfo!.group, undefined, 'Non-global path should not be global');
147+
});
148+
149+
test('env var set to "false" → original classification behavior is preserved', async () => {
150+
process.env.POETRY_VIRTUALENVS_IN_PROJECT = 'false';
151+
152+
const result = await nativeToPythonEnv(baseEnvInfo, mockApi, mockManager, '/usr/bin/poetry');
153+
154+
assert.ok(result, 'Should return a PythonEnvironment');
155+
assert.ok(capturedInfo, 'Should have captured environment info');
156+
// Falls through to normal check since env var is falsy
157+
assert.strictEqual(capturedInfo!.group, undefined, 'Non-global path should not be global');
158+
});
159+
});

0 commit comments

Comments
 (0)