Skip to content

Commit 25d851c

Browse files
Fix PyEnv Windows path calculation for environment grouping (#1210)
- [x] Fix `nativeToPythonEnv` in `pyenvUtils.ts` - extract `getPyenvDir` helper - [x] Address reviewer feedback: use `PYENV_ROOT` env var as primary source, revert 3-levels-up to 2-levels-up (old behavior was correct for pyenv-win since versions live under `~/.pyenv/pyenv-win/versions/`) - [x] Fix POSIX test to use absolute path with `path.sep` - [x] Add test for `PYENV_ROOT` preference - [x] Validate changes compile and all 583 tests pass (no regressions) - [x] Run code review and security scan <!-- START COPILOT ORIGINAL PROMPT --> <details> <summary>Original prompt</summary> > > ---- > > *This section details on the original issue you should resolve* > > <issue_title>PyEnv: Windows path calculation bug causes environment grouping to fail</issue_title> > <issue_description>## Problem > > In `pyenvUtils.ts`, the calculation of `versionsPath` and `envsPaths` is incorrect on Windows due to the pyenv-win directory structure. > > ### Current Code > ```typescript > // pyenvUtils.ts L128-130 > const versionsPath = normalizePath(path.join(path.dirname(path.dirname(pyenv)), 'versions')); > const envsPaths = normalizePath(path.join(path.dirname(versionsPath), 'envs')); > ``` > > ### Issue > On Windows, pyenv is located at `~/.pyenv/pyenv-win/bin/pyenv.bat`. The current logic calculates: > - `path.dirname(path.dirname(pyenv))` → `~/.pyenv/pyenv-win` ❌ > - Should be `~/.pyenv` to find `versions` folder properly > > This causes: > 1. Environment grouping (`PYENV_VERSIONS` vs `PYENV_ENVIRONMENTS`) to fail > 2. Environments not being associated with the correct manager > > ### PET Server Reference > The PET server handles this correctly with platform-specific code in `pet-pyenv/src/environment_locations.rs`: > ```rust > #[cfg(windows)] > pub fn get_home_pyenv_dir(env_vars: &EnvVariables) -> Option<PathBuf> { > let home = env_vars.home.clone()?; > Some(norm_case(home.join(".pyenv").join("pyenv-win"))) > } > ``` > > ### Suggested Fix > Add Windows-specific handling when calculating the versions and envs paths: > ```typescript > const pyenvDir = isWindows() > ? path.dirname(path.dirname(path.dirname(pyenv))) // Go up 3 levels on Windows > : path.dirname(path.dirname(pyenv)); > const versionsPath = normalizePath(path.join(pyenvDir, 'versions')); > ``` > > ### Affected File > `src/managers/pyenv/pyenvUtils.ts`</issue_description> > > ## Comments on the Issue (you are @copilot in this section) > > <comments> > </comments> > </details> <!-- START COPILOT CODING AGENT SUFFIX --> - Fixes #1180 <!-- START COPILOT CODING AGENT TIPS --> --- 💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more [Copilot coding agent tips](https://gh.io/copilot-coding-agent-tips) in the docs. --------- 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 de4493f commit 25d851c

File tree

2 files changed

+62
-2
lines changed

2 files changed

+62
-2
lines changed

src/managers/pyenv/pyenvUtils.ts

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,21 @@ import {
2323
} from '../common/nativePythonFinder';
2424
import { shortVersion, sortEnvironments } from '../common/utils';
2525

26+
/**
27+
* Returns the pyenv root directory from the pyenv executable path.
28+
* Prefers `PYENV_ROOT` env var when set, otherwise goes up 2 levels from the binary.
29+
* On POSIX, pyenv binary is at `<root>/bin/pyenv` (e.g. `~/.pyenv/bin/pyenv`).
30+
* On Windows, pyenv-win binary is at `<root>/bin/pyenv.bat` (e.g. `~/.pyenv/pyenv-win/bin/pyenv.bat`,
31+
* where `<root>` is `~/.pyenv/pyenv-win`).
32+
*/
33+
export function getPyenvDir(pyenv: string): string {
34+
const pyenvRoot = process.env.PYENV_ROOT;
35+
if (pyenvRoot) {
36+
return pyenvRoot;
37+
}
38+
return path.dirname(path.dirname(pyenv));
39+
}
40+
2641
async function findPyenv(): Promise<string | undefined> {
2742
try {
2843
return await which('pyenv');
@@ -174,8 +189,9 @@ function nativeToPythonEnv(
174189
return undefined;
175190
}
176191

177-
const versionsPath = normalizePath(path.join(path.dirname(path.dirname(pyenv)), 'versions'));
178-
const envsPaths = normalizePath(path.join(path.dirname(versionsPath), 'envs'));
192+
const pyenvDir = getPyenvDir(pyenv);
193+
const versionsPath = normalizePath(path.join(pyenvDir, 'versions'));
194+
const envsPaths = normalizePath(path.join(pyenvDir, 'envs'));
179195
let group = undefined;
180196
const normPrefix = normalizePath(info.prefix);
181197
if (normPrefix.startsWith(versionsPath)) {
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
import assert from 'node:assert';
2+
import * as path from 'path';
3+
import * as sinon from 'sinon';
4+
import { getPyenvDir } from '../../../managers/pyenv/pyenvUtils';
5+
6+
suite('pyenvUtils - getPyenvDir', () => {
7+
let originalPyenvRoot: string | undefined;
8+
9+
setup(() => {
10+
originalPyenvRoot = process.env.PYENV_ROOT;
11+
delete process.env.PYENV_ROOT;
12+
});
13+
14+
teardown(() => {
15+
sinon.restore();
16+
if (originalPyenvRoot !== undefined) {
17+
process.env.PYENV_ROOT = originalPyenvRoot;
18+
} else {
19+
delete process.env.PYENV_ROOT;
20+
}
21+
});
22+
23+
test('should use PYENV_ROOT when set', () => {
24+
const pyenvRoot = path.join(path.sep, 'custom', 'pyenv', 'root');
25+
process.env.PYENV_ROOT = pyenvRoot;
26+
const pyenvBin = path.join(path.sep, 'other', 'bin', 'pyenv');
27+
const result = getPyenvDir(pyenvBin);
28+
assert.strictEqual(result, pyenvRoot);
29+
});
30+
31+
test('should go up 2 levels on POSIX when PYENV_ROOT is not set (bin/pyenv -> pyenv root)', () => {
32+
// e.g. /home/user/.pyenv/bin/pyenv
33+
const pyenvBin = path.join(path.sep, 'home', 'user', '.pyenv', 'bin', 'pyenv');
34+
const result = getPyenvDir(pyenvBin);
35+
assert.strictEqual(result, path.join(path.sep, 'home', 'user', '.pyenv'));
36+
});
37+
38+
test('should go up 2 levels on Windows when PYENV_ROOT is not set (pyenv-win/bin/pyenv.bat -> pyenv-win)', () => {
39+
// e.g. C:\Users\user\.pyenv\pyenv-win\bin\pyenv.bat -> C:\Users\user\.pyenv\pyenv-win
40+
const pyenvBin = path.join('C:', 'Users', 'user', '.pyenv', 'pyenv-win', 'bin', 'pyenv.bat');
41+
const result = getPyenvDir(pyenvBin);
42+
assert.strictEqual(result, path.join('C:', 'Users', 'user', '.pyenv', 'pyenv-win'));
43+
});
44+
});

0 commit comments

Comments
 (0)