From 3ca22a5b62d1f5465566396ebe7b0bcec923130c Mon Sep 17 00:00:00 2001 From: Ben Fitzhardinge Date: Mon, 25 Aug 2025 17:39:27 +0800 Subject: [PATCH] fix: remove bug where pipenv being installed would cause false positive match for python package management --- .../src/lib/dependencies/package-manager.ts | 423 +++++++++++++----- 1 file changed, 308 insertions(+), 115 deletions(-) diff --git a/packages/@cdktf/cli-core/src/lib/dependencies/package-manager.ts b/packages/@cdktf/cli-core/src/lib/dependencies/package-manager.ts index fc2fe7814f..1d9d0634f9 100644 --- a/packages/@cdktf/cli-core/src/lib/dependencies/package-manager.ts +++ b/packages/@cdktf/cli-core/src/lib/dependencies/package-manager.ts @@ -241,132 +241,147 @@ class NodePackageManager extends PackageManager { } } -class PythonPackageManager extends PackageManager { - private get appCommand() { +/* eslint-disable no-unused-vars */ +abstract class PythonPackageTool { + /* Abstract base class for Python package manager tools. + This is a bit messy, but it seems smarter to split each package manager up + into its own context than it does to create a single class with a bunch of if statements. + */ + constructor(protected readonly workingDirectory: string) {} + + abstract get name(): string; + + abstract validateInstallation(): void; + + abstract addPackage( + packageName: string, + packageVersion?: string, + ): Promise; + + abstract listPackages(): Promise<{ name: string; version: string }[]>; // Returns parsed JSON output +} +/* eslint-enable no-unused-vars */ + +class PipTool extends PythonPackageTool { + get name(): string { + return "pip"; + } + + validateInstallation(): void { try { - return JSON.parse( - fs.readFileSync( - path.resolve(this.workingDirectory, "cdktf.json"), - "utf8", - ), - )["app"]; - } catch (e: any) { + exec("pip", ["--version"], { cwd: this.workingDirectory }).then(); + return; + } catch { throw Errors.Usage( - `Could not find find and parse cdktf.json in ${this.workingDirectory}`, - e, + "Using Python with pip, but the pip command is not available. Please check your environment and try again.", ); } } - public async addPackage( + async addPackage( packageName: string, packageVersion?: string, ): Promise { - const usePipenv = this.appCommand.includes("pipenv"); - - if (usePipenv) { - console.log( - `Installing package ${packageName} @ ${packageVersion} using pipenv.`, + const requirementsFilePath = path.join( + this.workingDirectory, + "requirements.txt", + ); + if (!fs.existsSync(requirementsFilePath)) { + throw Errors.Usage( + `Could not find requirements.txt in ${this.workingDirectory}`, ); + } + // add new requirements to the requirements.txt file + const requirements = await fs.readFile(requirementsFilePath, "utf8"); + const requirementLine = requirements + .split("\n") + .find((line: string) => line.includes(packageName)); - await exec("pipenv", ["install", `${packageName}~=${packageVersion}`], { - cwd: this.workingDirectory, - env: { - ...process.env, - PIPENV_QUIET: "1", - }, - stdio: ["inherit", 1, 1], - }); - - console.log("Package installed."); - } else { - console.log( - `Installing package ${packageName} @ ${packageVersion} using pip.`, - ); + logger.debug( + `Read requirements.txt file and found line including ${packageName}: ${requirementLine}`, + ); - const requirementsFilePath = path.join( - this.workingDirectory, - "requirements.txt", - ); - if (!fs.existsSync(requirementsFilePath)) { - throw Errors.Usage( - `Could not find requirements.txt in ${this.workingDirectory}`, + if (requirementLine) { + if (packageVersion ? requirementLine.includes(packageVersion) : true) { + logger.info( + `Package ${packageName} already in requirements file. Skipping installation.`, + ); + return; + } else { + logger.debug( + `Found the package ${packageName} already in requirements file, but with a different version, continuing`, ); } + } - const requirements = await fs.readFile(requirementsFilePath, "utf8"); - const requirementLine = requirements + const newRequirements = + requirements .split("\n") - .find((line) => line.includes(packageName)); - - logger.debug( - `Read requirements.txt file and found line including ${packageName}: ${requirementLine}`, - ); - - if (requirementLine) { - if (packageVersion ? requirementLine.includes(packageVersion) : true) { - logger.info( - `Package ${packageName} already installed. Skipping installation.`, - ); - return; - } else { - logger.debug( - `Found the package but with a different version, continuing`, - ); - } - } - - const newRequirements = - requirements - .split("\n") - .filter((line) => !line.startsWith(packageName)) - .join("\n") + - `\n${packageName}${packageVersion ? `~=${packageVersion}` : ""}`; - await fs.writeFile(requirementsFilePath, newRequirements, "utf8"); + .filter((line: string) => !line.startsWith(packageName)) + .join("\n") + + `\n${packageName}${packageVersion ? `~=${packageVersion}` : ""}`; + await fs.writeFile(requirementsFilePath, newRequirements, "utf8"); + + await exec("pip", ["install", "-r", "requirements.txt"], { + cwd: this.workingDirectory, + stdio: ["inherit", 1, 1], + }); + } - await exec("pip", ["install", "-r", "requirements.txt"], { + async listPackages(): Promise<{ name: string; version: string }[]> { + try { + const stdout: string = await exec("pip", ["list", "--format=json"], { cwd: this.workingDirectory, - stdio: ["inherit", 1, 1], }); - - console.log("Package installed."); + logger.debug( + `Listing pip packages using "pip list --format=json": ${stdout}`, + ); + return pipPackageSchema.parse(JSON.parse(stdout)); + } catch (e: any) { + throw new Error( + `Could not determine installed packages using 'pip list --format=json': ${e.message}`, + ); } } +} - public async isNpmVersionAvailable( - packageName: string, - packageVersion: string, - ): Promise { - logger.debug( - `Checking if ${packageName}@${packageVersion} is available for Python`, - ); - const url = `https://pypi.org/pypi/${packageName}/${packageVersion}/json`; - logger.debug(`Fetching package information for ${packageName} from ${url}`); - - const response = await fetch(url); - const json = (await response.json()) as any; - logger.debug( - `Got response from PyPI for ${packageName}@${packageVersion}: ${JSON.stringify( - json, - )}`, - ); +class PipenvTool extends PythonPackageTool { + get name(): string { + return "pipenv"; + } - if (json.info) { - // We found the version, so it exists - return true; - } else { - logger.debug( - `Could not get PyPI package info, got: ${JSON.stringify(json)}`, + validateInstallation(): void { + try { + exec("pipenv", ["--version"], { cwd: this.workingDirectory }); + return; + } catch { + throw Errors.Usage( + 'Found "pipenv" in app command but the pipenv command is not available. Please install pipenv or update your app command.', ); - return false; } } - public async listPipenvPackages(): Promise< - { name: string; version: string }[] - > { + async addPackage( + packageName: string, + packageVersion?: string, + ): Promise { + const args: string[] = [ + "install", + packageVersion ? `${packageName}~=${packageVersion}` : packageName, + ]; + await exec("pipenv", args, { + cwd: this.workingDirectory, + env: { + ...process.env, + PIPENV_QUIET: "1", + }, + stdio: ["inherit", 1, 1], + }); + } + + async listPackages(): Promise<{ name: string; version: string }[]> { try { - const stdout = await exec( + const stdout: string = await exec( "pipenv", ["run", "pip", "list", "--format=json"], { @@ -376,44 +391,222 @@ class PythonPackageManager extends PackageManager { logger.debug( `Listing pipenv packages using "pipenv run pip list --format=json": ${stdout}`, ); - - const list = pipPackageSchema.parse(JSON.parse(stdout)); - return list.filter((item) => - item.name.startsWith("cdktf-cdktf-provider"), - ); + return pipPackageSchema.parse(JSON.parse(stdout)); } catch (e: any) { throw new Error( `Could not determine installed packages using 'pipenv run pip list --format=json': ${e.message}`, ); } } +} - public async listPipPackages(): Promise<{ name: string; version: string }[]> { +class PoetryTool extends PythonPackageTool { + get name(): string { + return "poetry"; + } + + validateInstallation() { try { - const stdout = await exec("pip", ["list", "--format=json"], { - cwd: this.workingDirectory, - }); + exec("poetry", ["--version"], { cwd: this.workingDirectory }); + return; + } catch { + throw Errors.Usage( + 'Found "poetry" in app command but the poetry command is not available. Please install poetry or update your app command.', + ); + } + } + + async addPackage( + packageName: string, + packageVersion?: string, + ): Promise { + const args = [ + "add", + packageVersion ? `${packageName}@^${packageVersion}` : packageName, + ]; + await exec("poetry", args, { + cwd: this.workingDirectory, + stdio: ["inherit", 1, 1], + }); + } + + async listPackages(): Promise<{ name: string; version: string }[]> { + try { + const stdout: string = await exec( + "poetry", + ["run", "pip", "list", "--format=json"], + { + cwd: this.workingDirectory, + }, + ); logger.debug( - `Listing pipenv packages using "pip list --format=json": ${stdout}`, + `Listing poetry packages using "poetry show --format=json": ${stdout}`, + ); + return pipPackageSchema.parse(JSON.parse(stdout)); + } catch (e: any) { + throw new Error( + `Could not determine installed packages using 'poetry run pip list --format=json': ${e.message}`, + ); + } + } +} + +class UvTool extends PythonPackageTool { + get name(): string { + return "uv"; + } + + validateInstallation() { + try { + exec("uv", ["--version"], { cwd: this.workingDirectory }); + return; + } catch { + throw Errors.Usage( + 'Found "uv" in app command but the uv command is not available. Please install uv or update your app command.', ); + } + } - const list = pipPackageSchema.parse(JSON.parse(stdout)); - return list.filter((item) => - item.name.startsWith("cdktf-cdktf-provider"), + async addPackage( + packageName: string, + packageVersion?: string, + ): Promise { + const args: string[] = [ + "add", + packageVersion ? `${packageName}~=${packageVersion}` : packageName, + ]; + await exec("uv", args, { + cwd: this.workingDirectory, + stdio: ["inherit", 1, 1], + }); + } + + async listPackages(): Promise<{ name: string; version: string }[]> { + try { + const stdout: string = await exec( + "uv", + ["pip", "list", "--format=json"], + { + cwd: this.workingDirectory, + }, + ); + logger.debug( + `Listing uv packages using "uv list --format=json": ${stdout}`, ); + return pipPackageSchema.parse(JSON.parse(stdout)); } catch (e: any) { throw new Error( - `Could not determine installed packages using 'pip list --format=json': ${e.message}`, + `Could not determine installed packages using 'uv pip list --format=json': ${e.message}`, + ); + } + } +} + +class PythonPackageManager extends PackageManager { + /* + Python Package Manager should default to using pip + if it cannot detect the usage of any other package manager. + A package manager being installed does not mean it is being used for this project. + We will check the app command in cdktf.json for pipenv usage. + + What would be better is for this to be explicitly set in cdktf.json + but that would be a breaking change. + Ideally language `python` would be python with pip and + `python-pipenv` would be python with pipenv. + */ + + private _pythonPackageTool: PythonPackageTool | undefined; + + get packageManager(): PythonPackageTool { + if (this._pythonPackageTool !== undefined) { + return this._pythonPackageTool; + } + + const appCmd = this.appCommand?.toLowerCase() || ""; + const firstAppCmd: string = appCmd.split(" ")[0]; + switch (firstAppCmd) { + case "pipenv": + this._pythonPackageTool = new PipenvTool(this.workingDirectory); + break; + case "poetry": + this._pythonPackageTool = new PoetryTool(this.workingDirectory); + break; + case "uv": + this._pythonPackageTool = new UvTool(this.workingDirectory); + break; + default: + // Fall back to pip as the default + this._pythonPackageTool = new PipTool(this.workingDirectory); + } + // Validate the installation of the package manager + this._pythonPackageTool.validateInstallation(); + + return this._pythonPackageTool; + } + + private get appCommand() { + // extract the app command from cdktf.json + try { + return JSON.parse( + fs.readFileSync( + path.resolve(this.workingDirectory, "cdktf.json"), + "utf8", + ), + )["app"]; + } catch (e: any) { + throw Errors.Usage( + `Could not find find and parse cdktf.json in ${this.workingDirectory}`, + e, ); } } + public async addPackage( + packageName: string, + packageVersion?: string, + ): Promise { + console.log( + `Installing package ${packageName} @ ${packageVersion} using ${this.packageManager.name}.`, + ); + await this.packageManager.addPackage(packageName, packageVersion); + console.log("Package installed."); + } + public async listProviderPackages(): Promise< { name: string; version: string }[] > { - return this.appCommand.includes("pipenv") - ? this.listPipenvPackages() - : this.listPipPackages(); + const list: { name: string; version: string }[] = + await this.packageManager.listPackages(); + return list.filter((item) => item.name.startsWith("cdktf-cdktf-provider")); + } + + public async isNpmVersionAvailable( + packageName: string, + packageVersion: string, + ): Promise { + logger.debug( + `Checking if ${packageName}@${packageVersion} is available for Python`, + ); + const url = `https://pypi.org/pypi/${packageName}/${packageVersion}/json`; + logger.debug(`Fetching package information for ${packageName} from ${url}`); + + const response = await fetch(url); + const json = (await response.json()) as any; + logger.debug( + `Got response from PyPI for ${packageName}@${packageVersion}: ${JSON.stringify( + json, + )}`, + ); + + if (json.info) { + // We found the version, so it exists + return true; + } else { + logger.debug( + `Could not get PyPI package info, got: ${JSON.stringify(json)}`, + ); + return false; + } } }