diff --git a/packages/jsii-pacmak/lib/targets/python.ts b/packages/jsii-pacmak/lib/targets/python.ts index 328351d363..448b7ab459 100644 --- a/packages/jsii-pacmak/lib/targets/python.ts +++ b/packages/jsii-pacmak/lib/targets/python.ts @@ -7,7 +7,7 @@ import * as path from 'path'; import * as spec from '@jsii/spec'; import { Stability } from '@jsii/spec'; import { Generator, GeneratorOptions } from '../generator'; -import { warn } from '../logging'; +import { info, warn } from '../logging'; import { md2rst } from '../markdown'; import { Target, TargetOptions } from '../target'; import { shell } from '../util'; @@ -46,28 +46,9 @@ export default class Python extends Target { await super.generateCode(outDir, tarball); // We'll just run "black" on that now, to make the generated code a little more readable. - const blackRoot = await fs.mkdtemp( - path.join(os.tmpdir(), 'jsii-pacmak-black-'), - ); - try { - await shell('python3', ['-m', 'venv', path.join(blackRoot, '.env')], { - cwd: blackRoot, - }); - await shell( - path.join(blackRoot, '.env', 'bin', 'pip'), - ['install', 'black'], - { cwd: blackRoot }, - ); - await shell( - path.join(blackRoot, '.env', 'bin', 'black'), - ['--py36', outDir], - { - cwd: outDir, - }, - ); - } finally { - await fs.remove(blackRoot); - } + await shell(await this.blackPath(), ['--py36', outDir], { + cwd: outDir, + }); } public async build(sourceDir: string, outDir: string): Promise { @@ -78,7 +59,7 @@ export default class Python extends Target { await shell('python3', ['setup.py', 'bdist_wheel', '--dist-dir', outDir], { cwd: sourceDir, }); - if (await twineIsPresent()) { + if (await isPresent('twine', sourceDir)) { await shell('twine', ['check', path.join(outDir, '*')], { cwd: sourceDir, }); @@ -88,22 +69,56 @@ export default class Python extends Target { 'Run `pip3 install twine` to enable distribution package validation.', ); } + } - // Approximating existence check using `which`, falling back on `pip3 show`. If that fails, assume twine is not there. - async function twineIsPresent(): Promise { - try { - await shell('which', ['twine'], { cwd: sourceDir }); - return true; - } catch { - try { - const output = await shell('pip3', ['show', 'twine'], { - cwd: sourceDir, - }); - return output.trim() !== ''; - } catch { - return false; - } - } + private async blackPath(): Promise { + if (await isPresent('black')) { + return 'black'; + } + + const blackInstallDir = path.join( + os.homedir(), + '.jsii-cache', + 'python-black', + ); + const exists = await fs.pathExists(blackInstallDir); + if (!exists) { + info( + `No existing black installation. Install afresh at ${blackInstallDir}...`, + ); + await fs.mkdirp(blackInstallDir); + await shell( + 'python3', + ['-m', 'venv', path.join(blackInstallDir, '.env')], + { + cwd: blackInstallDir, + }, + ); + await shell( + path.join(blackInstallDir, '.env', 'bin', 'pip'), + ['install', 'black'], + { cwd: blackInstallDir }, + ); + } + return path.join(blackInstallDir, '.env', 'bin', 'black'); + } +} + +// Approximating existence check using `which`, falling back on `pip3 show`. +async function isPresent(binary: string, sourceDir?: string): Promise { + try { + await shell('which', [binary], { + cwd: sourceDir, + }); + return true; + } catch { + try { + const output = await shell('pip3', ['show', binary], { + cwd: sourceDir, + }); + return output.trim() !== ''; + } catch { + return false; } } } diff --git a/packages/jsii-pacmak/lib/util.ts b/packages/jsii-pacmak/lib/util.ts index 41738b6d32..fdd6c596e1 100644 --- a/packages/jsii-pacmak/lib/util.ts +++ b/packages/jsii-pacmak/lib/util.ts @@ -33,7 +33,7 @@ export function resolveDependencyDirectory( export async function shell( cmd: string, args: string[], - options: ShellOptions, + options: ShellOptions = {}, ): Promise { // eslint-disable-next-line @typescript-eslint/require-await async function spawn1() { diff --git a/packages/jsii-pacmak/package.json b/packages/jsii-pacmak/package.json index e4305326f9..e597c05c2f 100644 --- a/packages/jsii-pacmak/package.json +++ b/packages/jsii-pacmak/package.json @@ -30,7 +30,7 @@ "watch": "tsc --build -w", "lint": "eslint . --ext .js,.ts --ignore-path=.gitignore", "lint:fix": "yarn lint --fix", - "test": "/bin/bash test/diff-test.sh && /bin/bash test/build-test.sh && jest", + "test": "jest && /bin/bash test/diff-test.sh && /bin/bash test/build-test.sh", "test:update": "UPDATE_DIFF=1 /bin/bash test/diff-test.sh && /bin/bash test/build-test.sh && jest -u", "package": "package-js" }, diff --git a/packages/jsii-pacmak/test/targets/python.test.ts b/packages/jsii-pacmak/test/targets/python.test.ts new file mode 100644 index 0000000000..a6f739cb2c --- /dev/null +++ b/packages/jsii-pacmak/test/targets/python.test.ts @@ -0,0 +1,108 @@ +import * as util from '../../lib/util'; +import * as os from 'os'; +import * as path from 'path'; +import * as fs from 'fs-extra'; +import Python from '../../lib/targets/python'; +import { Assembly } from 'jsii-reflect'; +import { Rosetta } from 'jsii-rosetta'; + +describe('python', () => { + describe('blackPath', () => { + const shellMock = jest.fn(); + const homedirMock = jest.fn(); + let homedir: string; + let python: Python; + + beforeEach(async () => { + // eslint-disable-next-line no-import-assign + Object.defineProperty(util, 'shell', { value: shellMock }); + // eslint-disable-next-line no-import-assign + Object.defineProperty(os, 'homedir', { value: homedirMock }); + homedir = await fs.mkdtemp(path.join(os.tmpdir(), 'jsii-pacmak-black-')); + homedirMock.mockImplementation(() => homedir); + python = new Python({ + targetName: 'python', + packageDir: '/dir', + assembly: {} as Assembly, + rosetta: new Rosetta(), + arguments: {}, + }); + }); + + afterEach(async () => { + shellMock.mockClear(); + homedirMock.mockClear(); + await fs.remove(homedir); + }); + + test('black is installed globally', async () => { + let badShellCommand: string | undefined; + shellMock.mockImplementation((cmd: string, args: string[], _) => { + return new Promise((ok, ko) => { + if (cmd === 'which' && args[0] === 'black') { + ok('/path/to/black'); + } else { + badShellCommand = `Unexpected call to shell [${cmd} ${args}]`; + ko(badShellCommand); + } + }); + }); + + const path = await (python as any).blackPath(); // call private method blackPath() + expect(badShellCommand).toBeUndefined(); + expect(path).toBe('black'); + }); + + test('black is installed if not found globally', async () => { + shellMock.mockImplementation((cmd: string, args: string[], _) => { + return new Promise((ok, ko) => { + if (cmd === 'which' && args[0] === 'black') { + ko('black not found'); + } else if ( + /pip.?$/.test(cmd) && + args[0] === 'show' && + args[1] === 'black' + ) { + ko(); + } else { + ok(); + } + }); + }); + + const path = await (python as any).blackPath(); // call private method blackPath() + expect(path).toBe(`${homedir}/.jsii-cache/python-black/.env/bin/black`); + }); + + test('local cache is reused', async () => { + let installCount = 0; + shellMock.mockImplementation((cmd: string, args: string[], _) => { + return new Promise((ok, ko) => { + if (cmd === 'which' && args[0] === 'black') { + ko('black not found'); + } else if ( + /pip.?$/.test(cmd) && + args[0] === 'show' && + args[1] === 'black' + ) { + ko(); + } else if ( + /pip.?$/.test(cmd) && + args[0] === 'install' && + args[1] === 'black' + ) { + installCount++; + ok(); + } else { + ok(); + } + }); + }); + + await (python as any).blackPath(); + await (python as any).blackPath(); + await (python as any).blackPath(); + expect(installCount).toEqual(1); + }); + }); +}); diff --git a/superchain/Dockerfile b/superchain/Dockerfile index b554a818c8..3067c72a86 100644 --- a/superchain/Dockerfile +++ b/superchain/Dockerfile @@ -20,7 +20,7 @@ RUN rpm --import "https://packages.microsoft.com/keys/microsoft.asc" # Install Python 3 RUN yum -y install python3 python3-pip \ - && python3 -m pip install --upgrade pip setuptools wheel twine \ + && python3 -m pip install --upgrade pip setuptools wheel twine black \ && yum clean all && rm -rf /var/cache/yum # Install Ruby 2.6+