fix(pacmak): python pack fails when installing 'black' via pip

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';
@@ -34,6 +34,11 @@ import { die, toPythonIdentifier } from './python/util';
 const spdxLicenseList = require('spdx-license-list');
 
 export default class Python extends Target {
+  private static readonly BLACK_INSTALL_DIR: string = path.join(
+    os.homedir(),
+    'python-black',
+  );
+
   protected readonly generator: PythonGenerator;
 
   public constructor(options: TargetOptions) {
@@ -46,28 +51,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<void> {
@@ -78,7 +64,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 +74,51 @@ 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<boolean> {
-      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<string> {
+    if (await isPresent('black')) {
+      return 'black';
+    }
+
+    const exists = await fs.pathExists(Python.BLACK_INSTALL_DIR);
+    if (!exists) {
+      info(
+        `No existing black installation. Install afresh at ${Python.BLACK_INSTALL_DIR}...`,
+      );
+      await fs.mkdir(Python.BLACK_INSTALL_DIR);
+      await shell(
+        'python3',
+        ['-m', 'venv', path.join(Python.BLACK_INSTALL_DIR, '.env')],
+        {
+          cwd: Python.BLACK_INSTALL_DIR,
+        },
+      );
+      await shell(
+        path.join(Python.BLACK_INSTALL_DIR, '.env', 'bin', 'pip'),
+        ['install', 'black'],
+        { cwd: Python.BLACK_INSTALL_DIR },
+      );
+    }
+    return path.join(Python.BLACK_INSTALL_DIR, '.env', 'bin', 'black');
+  }
+}
+
+// Approximating existence check using `which`, falling back on `pip3 show`.
+async function isPresent(binary: string, sourceDir?: string): Promise<boolean> {
+  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;
     }
   }
 }

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<string> {
   // eslint-disable-next-line @typescript-eslint/require-await
   async function spawn1() {

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"
   },

packages/jsii-pacmak/test/targets/python.test.ts

@@ -0,0 +1,51 @@
+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 osMock = jest.fn();
+    let tmpdir: string;
+
+    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: osMock });
+      tmpdir = await fs.mkdtemp(path.join(os.tmpdir(), 'jsii-pacmak-black-'));
+      osMock.mockImplementation(() => tmpdir);
+    });
+
+    afterEach(async () => {
+      shellMock.mockClear();
+      osMock.mockClear();
+      await fs.remove(tmpdir);
+    });
+
+    test('black is installed globally', async () => {
+      shellMock.mockImplementation((cmd: string, args: string[], _) => {
+        return new Promise((ok, ko) => {
+          if (cmd === 'which' && args[0] === 'black') {
+            ok('/path/to/black');
+          }
+          ko(`Unexpected call to shell ${cmd} ${args}`);
+        });
+      });
+
+      const python = new Python({
+        targetName: 'python',
+        packageDir: '/dir',
+        assembly: {} as Assembly,
+        rosetta: new Rosetta(),
+        arguments: {},
+      });
+      const path = await (python as any).blackPath();
+      expect(path).toBe('black');
+    });
+  });
+});

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+