mirror of
https://github.com/agentscope-ai/ReMe.git
synced 2026-11-10 08:28:22 +00:00
fix(dsh): resolve interpreter paths with their own platform semantics
The interpreter helpers mixed simulated-platform knowledge with the host's node:path, so Windows fixtures only resolved on Windows: on macOS and Linux condaRootFromPath, pathContains, pythonPrefix, classifyOrigin, remeCliPath and the pip path check all fell back to POSIX semantics and nine tests failed. Paths now carry their flavor: pathOf() picks win32 or posix from the value itself, and the functions that already know the simulated platform use it explicitly. Behaviour on Windows is unchanged; the same fixtures now resolve identically on every host, which the new tests assert for both flavors.
This commit is contained in:
parent
755a3cc26c
commit
fdb9cc3d70
6 changed files with 126 additions and 35 deletions
|
|
@ -1,7 +1,7 @@
|
|||
import { basename, dirname, isAbsolute, join, resolve } from "node:path";
|
||||
import { join as posixJoin } from "node:path/posix";
|
||||
import { win32 } from "node:path";
|
||||
|
||||
import { safeError } from "../../endpoint.js";
|
||||
import { pathOf } from "./paths.js";
|
||||
import type { RunCommand, RunResult } from "./runner.js";
|
||||
import { probeReme, remeVersionAtLeast } from "./reme.js";
|
||||
import type { PythonDetection, PythonEnv, PythonOrigin } from "../types.js";
|
||||
|
|
@ -177,15 +177,19 @@ export type CondaRunner = (
|
|||
|
||||
/** Derive the conda installation root from any conda executable path. */
|
||||
export function condaRootFromPath(condaPath: string): string {
|
||||
const dir = dirname(condaPath);
|
||||
// The path may be written for another platform than the host, so its flavor
|
||||
// decides the semantics: dirname must not depend on where tests run.
|
||||
const path = pathOf(condaPath);
|
||||
const dir = path.dirname(condaPath);
|
||||
const lower = dir.toLowerCase();
|
||||
if (lower.endsWith("\\library\\bin") || lower.endsWith("/library/bin"))
|
||||
return dirname(dirname(dir));
|
||||
return path.dirname(path.dirname(dir));
|
||||
if (lower.endsWith("\\scripts") || lower.endsWith("/scripts"))
|
||||
return dirname(dir);
|
||||
return path.dirname(dir);
|
||||
if (lower.endsWith("\\condabin") || lower.endsWith("/condabin"))
|
||||
return dirname(dir);
|
||||
if (lower.endsWith("\\bin") || lower.endsWith("/bin")) return dirname(dir);
|
||||
return path.dirname(dir);
|
||||
if (lower.endsWith("\\bin") || lower.endsWith("/bin"))
|
||||
return path.dirname(dir);
|
||||
return "";
|
||||
}
|
||||
|
||||
|
|
@ -202,7 +206,8 @@ export function makeCondaRunner(
|
|||
): CondaRunner {
|
||||
const root = condaRootFromPath(condaPath);
|
||||
const exeCandidates: string[] = [];
|
||||
if (root && windows) exeCandidates.push(join(root, "Scripts", "conda.exe"));
|
||||
if (root && windows)
|
||||
exeCandidates.push(win32.join(root, "Scripts", "conda.exe"));
|
||||
if (!/\.(bat|cmd)$/i.test(condaPath)) exeCandidates.push(condaPath);
|
||||
return async (args, timeoutMs = 15_000) => {
|
||||
for (const candidate of exeCandidates) {
|
||||
|
|
@ -222,22 +227,23 @@ export function makeCondaRunner(
|
|||
|
||||
function commonCondaPaths(io: DetectIO, windows: boolean): string[] {
|
||||
const programData = io.env["ProgramData"] ?? "";
|
||||
const path = windows ? win32 : pathOf(io.homedir);
|
||||
const candidates = windows
|
||||
? [
|
||||
join(io.homedir, "miniconda3", "Scripts", "conda.exe"),
|
||||
join(io.homedir, "anaconda3", "Scripts", "conda.exe"),
|
||||
join(programData, "miniconda3", "Scripts", "conda.exe"),
|
||||
join(programData, "anaconda3", "Scripts", "conda.exe"),
|
||||
path.join(io.homedir, "miniconda3", "Scripts", "conda.exe"),
|
||||
path.join(io.homedir, "anaconda3", "Scripts", "conda.exe"),
|
||||
path.join(programData, "miniconda3", "Scripts", "conda.exe"),
|
||||
path.join(programData, "anaconda3", "Scripts", "conda.exe"),
|
||||
]
|
||||
: [
|
||||
join(io.homedir, "miniconda3", "bin", "conda"),
|
||||
join(io.homedir, "anaconda3", "bin", "conda"),
|
||||
join(io.homedir, "miniforge3", "bin", "conda"),
|
||||
path.join(io.homedir, "miniconda3", "bin", "conda"),
|
||||
path.join(io.homedir, "anaconda3", "bin", "conda"),
|
||||
path.join(io.homedir, "miniforge3", "bin", "conda"),
|
||||
"/opt/miniconda3/bin/conda",
|
||||
"/opt/anaconda3/bin/conda",
|
||||
"/opt/homebrew/Caskroom/miniconda/base/bin/conda",
|
||||
];
|
||||
return candidates.filter((path) => isAbsolute(path));
|
||||
return candidates.filter((candidate) => path.isAbsolute(candidate));
|
||||
}
|
||||
|
||||
/* ------------------------------------------------------------------ */
|
||||
|
|
@ -274,15 +280,19 @@ function addCandidate(
|
|||
}
|
||||
|
||||
function pathContains(dir: string, exe: string, windows: boolean): boolean {
|
||||
const base = normalizeExe(resolve(dir), windows);
|
||||
const full = normalizeExe(resolve(exe), windows);
|
||||
const path = windows ? win32 : pathOf(exe);
|
||||
const base = normalizeExe(path.resolve(dir), windows);
|
||||
const full = normalizeExe(path.resolve(exe), windows);
|
||||
return full.startsWith(`${base}${windows ? "\\" : "/"}`);
|
||||
}
|
||||
|
||||
/** prefix of an interpreter install from its executable location. */
|
||||
function pythonPrefix(exe: string, windows: boolean): string {
|
||||
const parent = dirname(exe);
|
||||
return windows || basename(parent) !== "bin" ? parent : dirname(parent);
|
||||
const path = windows ? win32 : pathOf(exe);
|
||||
const parent = path.dirname(exe);
|
||||
return windows || path.basename(parent) !== "bin"
|
||||
? parent
|
||||
: path.dirname(parent);
|
||||
}
|
||||
|
||||
function isVenv(info: VerifyInfo, windows: boolean): boolean {
|
||||
|
|
@ -312,7 +322,11 @@ function classifyOrigin(
|
|||
for (const envPath of conda.envs) {
|
||||
if (envPath === conda.root) continue;
|
||||
if (pathContains(envPath, info.exe, windows))
|
||||
return { kind: "conda", envName: basename(envPath), root: envPath };
|
||||
return {
|
||||
kind: "conda",
|
||||
envName: (windows ? win32 : pathOf(envPath)).basename(envPath),
|
||||
root: envPath,
|
||||
};
|
||||
}
|
||||
if (conda.root && pathContains(conda.root, info.exe, windows))
|
||||
return { kind: "conda", envName: "base", root: conda.root };
|
||||
|
|
@ -321,8 +335,8 @@ function classifyOrigin(
|
|||
return {
|
||||
kind: "venv",
|
||||
baseExe: windows
|
||||
? join(info.basePrefix, "python.exe")
|
||||
: posixJoin(info.basePrefix, "bin", "python3"),
|
||||
? win32.join(info.basePrefix, "python.exe")
|
||||
: pathOf(info.basePrefix).join(info.basePrefix, "bin", "python3"),
|
||||
};
|
||||
if ([...candidate.sources].some((source) => source.startsWith("registry")))
|
||||
return { kind: "python.org" };
|
||||
|
|
@ -491,8 +505,8 @@ export async function detectPythonEnvs(io: DetectIO): Promise<PythonDetection> {
|
|||
addCandidate(
|
||||
candidates,
|
||||
windows
|
||||
? join(envPath, "python.exe")
|
||||
: posixJoin(envPath, "bin", "python3"),
|
||||
? win32.join(envPath, "python.exe")
|
||||
: pathOf(envPath).join(envPath, "bin", "python3"),
|
||||
"conda env list",
|
||||
windows,
|
||||
);
|
||||
|
|
|
|||
|
|
@ -1,8 +1,9 @@
|
|||
import { spawn } from "node:child_process";
|
||||
import { closeSync, mkdirSync, openSync, readFileSync } from "node:fs";
|
||||
import { isAbsolute, join } from "node:path";
|
||||
import { join } from "node:path";
|
||||
|
||||
import type { InstallPhase, InstallRequest, InstallStatus } from "../types.js";
|
||||
import { pathOf } from "./paths.js";
|
||||
|
||||
export interface InstallChild {
|
||||
pid: number | undefined;
|
||||
|
|
@ -145,7 +146,9 @@ export class InstallJobs {
|
|||
|
||||
start(request: InstallRequest): InstallStatus {
|
||||
const pythonPath = (request.pythonPath ?? "").trim();
|
||||
if (!pythonPath || !isAbsolute(pythonPath))
|
||||
// The interpreter may be written for another platform than the host, so its
|
||||
// own flavor decides whether the path is absolute.
|
||||
if (!pythonPath || !pathOf(pythonPath).isAbsolute(pythonPath))
|
||||
throw new TypeError(
|
||||
"Install requires an absolute Python interpreter path",
|
||||
);
|
||||
|
|
|
|||
22
integrations/dsh/src/environment/host/paths.ts
Normal file
22
integrations/dsh/src/environment/host/paths.ts
Normal file
|
|
@ -0,0 +1,22 @@
|
|||
import { posix, win32, type PlatformPath } from "node:path";
|
||||
|
||||
/** True when a path is written the Windows way: drive letter, UNC or backslash. */
|
||||
export function isWindowsPath(value: string): boolean {
|
||||
return (
|
||||
/^[A-Za-z]:[\\/]/.test(value) ||
|
||||
value.startsWith("\\\\") ||
|
||||
value.includes("\\")
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* The path flavor a value is written in.
|
||||
*
|
||||
* Interpreter discovery and the probes around it see paths that may belong to
|
||||
* the simulated platform, so they must not fall back to the host's
|
||||
* `node:path`: the same fixtures then resolve identically on Windows, macOS and
|
||||
* Linux instead of only on the machine that wrote them.
|
||||
*/
|
||||
export function pathOf(value: string): PlatformPath {
|
||||
return isWindowsPath(value) ? win32 : posix;
|
||||
}
|
||||
|
|
@ -1,6 +1,7 @@
|
|||
import { dirname, isAbsolute, join } from "node:path";
|
||||
import { win32 } from "node:path";
|
||||
|
||||
import type { RemeCheck, RemeProbe } from "../types.js";
|
||||
import { pathOf } from "./paths.js";
|
||||
import type { RunCommand } from "./runner.js";
|
||||
|
||||
/** Minimum reme-ai version this DSH integration is tested against. */
|
||||
|
|
@ -67,18 +68,21 @@ export async function probeReme(
|
|||
|
||||
/** Locate the console script installed beside the interpreter. */
|
||||
export function remeCliPath(io: RemeIO, pythonPath: string): string {
|
||||
const directory = dirname(pythonPath);
|
||||
// The interpreter path may be written for another platform than the host, so
|
||||
// its flavor supplies the separators and joins.
|
||||
const path = io.platform === "win32" ? win32 : pathOf(pythonPath);
|
||||
const directory = path.dirname(pythonPath);
|
||||
// venv interpreters live inside the scripts/bin directory; conda ones do not.
|
||||
const scriptsDir =
|
||||
io.platform === "win32"
|
||||
? /[\\/]scripts$/i.test(directory)
|
||||
? directory
|
||||
: join(directory, "Scripts")
|
||||
: path.join(directory, "Scripts")
|
||||
: directory;
|
||||
const candidate =
|
||||
io.platform === "win32"
|
||||
? join(scriptsDir, "reme.exe")
|
||||
: join(scriptsDir, "reme");
|
||||
const candidate = path.join(
|
||||
scriptsDir,
|
||||
io.platform === "win32" ? "reme.exe" : "reme",
|
||||
);
|
||||
return io.exists(candidate) ? candidate : "";
|
||||
}
|
||||
|
||||
|
|
@ -91,7 +95,7 @@ export async function checkReme(
|
|||
pythonPath: string,
|
||||
): Promise<RemeCheck> {
|
||||
const path = pythonPath.trim();
|
||||
if (!path || !isAbsolute(path)) {
|
||||
if (!path || !pathOf(path).isAbsolute(path)) {
|
||||
return {
|
||||
installed: false,
|
||||
version: "",
|
||||
|
|
|
|||
|
|
@ -291,6 +291,24 @@ test("parses conda json output defensively", () => {
|
|||
assert.equal(parseCondaRoot("garbage"), "");
|
||||
});
|
||||
|
||||
test("resolves every fixture with its own platform's path semantics", () => {
|
||||
// The flavor comes from the path, not from the host running the test, so
|
||||
// these assertions hold on Windows, macOS and Linux alike.
|
||||
assert.equal(
|
||||
condaRootFromPath("D:\\ProgramFils\\Miniconda\\Scripts\\conda.exe"),
|
||||
"D:\\ProgramFils\\Miniconda",
|
||||
);
|
||||
assert.equal(
|
||||
condaRootFromPath("D:\\ProgramFils\\Miniconda\\condabin\\conda.bat"),
|
||||
"D:\\ProgramFils\\Miniconda",
|
||||
);
|
||||
assert.equal(
|
||||
condaRootFromPath("/opt/homebrew/Caskroom/miniconda/base/bin/conda"),
|
||||
"/opt/homebrew/Caskroom/miniconda/base",
|
||||
);
|
||||
assert.equal(condaRootFromPath("D:\\mc\\conda.bat"), "");
|
||||
});
|
||||
|
||||
test("derives conda roots and prefers the real conda.exe over the .bat", async () => {
|
||||
assert.equal(condaRootFromPath("D:\\mc\\Library\\bin\\conda.bat"), "D:\\mc");
|
||||
assert.equal(condaRootFromPath("D:\\mc\\Scripts\\conda.exe"), "D:\\mc");
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ import {
|
|||
checkReme,
|
||||
compareVersions,
|
||||
parseRemeVersion,
|
||||
remeCliPath,
|
||||
remeVersionAtLeast,
|
||||
} from "../dist/environment/host/reme.js";
|
||||
import {
|
||||
|
|
@ -190,6 +191,35 @@ function makeJobs(t) {
|
|||
return { jobs, children, directory };
|
||||
}
|
||||
|
||||
test("finds the reme CLI beside a Windows interpreter on any host", () => {
|
||||
const seen = [];
|
||||
const cli = remeCliPath(
|
||||
{
|
||||
run: async () => ({ code: 0, stdout: "", stderr: "" }),
|
||||
platform: "win32",
|
||||
exists: (path) => {
|
||||
seen.push(path);
|
||||
return path.endsWith("reme.exe");
|
||||
},
|
||||
},
|
||||
"C:\\env\\python.exe",
|
||||
);
|
||||
assert.equal(cli, "C:\\env\\Scripts\\reme.exe");
|
||||
assert.deepEqual(seen, ["C:\\env\\Scripts\\reme.exe"]);
|
||||
});
|
||||
|
||||
test("finds the reme CLI in a POSIX bin directory", () => {
|
||||
const cli = remeCliPath(
|
||||
{
|
||||
run: async () => ({ code: 0, stdout: "", stderr: "" }),
|
||||
platform: "linux",
|
||||
exists: () => true,
|
||||
},
|
||||
"/home/u/venv/bin/python",
|
||||
);
|
||||
assert.equal(cli, "/home/u/venv/bin/reme");
|
||||
});
|
||||
|
||||
test("tracks a successful pip job through exit", (t) => {
|
||||
const { jobs, children, directory } = makeJobs(t);
|
||||
const started = jobs.start({ pythonPath: "C:\\env\\python.exe" });
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue