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

The failure is because of intermittent unavailability of black or one of
its packages from the PyPI index.

Instead, install back into the environment, i.e., superchain. Fallback
to installing it once in the user's home directory.

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<void> {
@@ -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<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 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<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,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);
+    });
+  });
+});

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+