fix: Assembly not found on Windows (#1398)

jsii-docgen fails in Windows with the error `Assembly "<assembly-name>" not found`. 
The reason for this is that the path passed to the glob library we use must only use forward slashes.
However on Windows when using jsii-docgen, it may contain backslashes. 

To fix the issue, we pass the assemblies directory as CWD to glob instead. The CWD doesn't have the same limitation in regards to backslashes.
Other fixes involve making the npm helpers Windows aware.

And finally this PR adds a Windows test to the compat build matrix.

.projenrc.ts

@@ -47,6 +47,11 @@ const project = new CdklabsTypeScriptProject({
   jestOptions: {
     jestConfig: {
       setupFilesAfterEnv: ['<rootDir>/test/setup-jest.ts'],
+      testMatch: [
+        // On Windows the standard tests paths are not matched
+        // Use this simplified version instead that works good enough in this repo
+        '<rootDir>/test/**/*.test.ts',
+      ],
     },
   },
   tsconfig: {

projenrc/rosetta.ts

@@ -25,11 +25,14 @@ export class RosettaPeerDependency extends Component {
     super(project);
 
     const constraint = this.calculateVersionConstraint(options.supportedVersions);
+    const minVersions = this.calculateMinimalVersions(options.supportedVersions);
+    const latestVersion = this.calculateLatestVersion(options.supportedVersions);
+
     project.addDevDeps(constraint);
     project.addPeerDeps(constraint);
 
     project.github?.tryFindWorkflow('build')?.addJob('rosetta-compat', {
-      runsOn: ['ubuntu-latest'],
+      runsOn: ['${{ matrix.os }}'],
       permissions: {},
       env: {
         CI: 'true',
@@ -38,8 +41,13 @@ export class RosettaPeerDependency extends Component {
       strategy: {
         matrix: {
           domain: {
-            rosetta: this.calculateMinimalVersions(options.supportedVersions),
+            rosetta: minVersions,
+            os: ['ubuntu-latest'],
           },
+          include: [{
+            rosetta: latestVersion,
+            os: 'windows-latest',
+          }],
         },
       },
       steps: [{
@@ -68,7 +76,7 @@ export class RosettaPeerDependency extends Component {
       },
       {
         name: 'compile+test',
-        run: ['npx projen compile', 'npx projen test'].join('\n'),
+        run: ['npx projen compile', 'npx projen test --runInBand'].join('\n'),
       },
       {
         name: 'Check Rosetta version',
@@ -95,4 +103,9 @@ export class RosettaPeerDependency extends Component {
 
     return discoveredVersions;
   }
+
+  private calculateLatestVersion(versions: RosettaPeerDependencyOptions['supportedVersions']): string {
+    const discoveredVersions = this.calculateMinimalVersions(versions);
+    return discoveredVersions.sort().pop() ?? 'latest';
+  }
 }

src/cli.ts

@@ -59,7 +59,7 @@ export async function main() {
     .option('readme', { alias: 'r', type: 'boolean', required: false, desc: 'Include the user defined README.md in the documentation.' })
     .option('submodule', { alias: 's', type: 'string', required: false, desc: 'Generate docs for a specific submodule (or "root")' })
     .option('split-by-submodule', { type: 'boolean', required: false, desc: 'Generate a separate file for each submodule' })
-    .example('$0', 'Generate documentation for the current module as a single file (auto-resolves node depedencies)')
+    .example('$0', 'Generate documentation for the current module as a single file (auto-resolves node dependencies)')
     .argv;
 
   const submodule = args.submodule === 'root' ? undefined : args.submodule;

src/docgen/view/_npm.ts

@@ -38,7 +38,7 @@ export class Npm {
       // code execution as part of the install command using npm hooks. (e.g postInstall)
       '--ignore-scripts',
       // save time by not running audit
-      '--no-autit',
+      '--no-audit',
       // ensures npm does not insert anything in $PATH
       '--no-bin-links',
       // don't write or update a package-lock.json file
@@ -132,17 +132,20 @@ export class Npm {
       return this.#npmCommand;
     }
 
+    // Get the platform specific npm command
+    const npm = npmPlatformAwareCommand();
+
     try {
       // If the npm in $PATH is >= v7, we can use that directly. The
       // `npm version --json` command returns a JSON object containing the
       // versions of several components (npm, node, v8, etc...). We are only
       // interested in the `npm` key here.
       const { exitCode, stdout } = await this.runCommand(
-        'npm', ['version', '--json'],
+        npm, ['version', '--json'],
         chunksToObject,
       );
       if (exitCode === 0 && major((stdout as any).npm) >= 7) {
-        return this.#npmCommand = 'npm';
+        return this.#npmCommand = npm;
       }
     } catch (e) {
       this.logger('Could not determine version of npm in $PATH:', e);
@@ -152,7 +155,7 @@ export class Npm {
     // the full type system.
     this.logger('The npm in $PATH is not >= v7. Installing npm@8 locally...');
     const result = await this.runCommand(
-      'npm',
+      npm,
       ['install', 'npm@8', '--no-package-lock', '--no-save', '--json'],
       chunksToObject,
       {
@@ -162,7 +165,7 @@ export class Npm {
     );
     assertSuccess(result);
 
-    this.#npmCommand = join(this.workingDirectory, 'node_modules', '.bin', 'npm');
+    this.#npmCommand = join(this.workingDirectory, 'node_modules', '.bin', npm);
     this.logger(`Done installing npm@8 at ${this.#npmCommand}`);
     return this.#npmCommand;
   }
@@ -188,7 +191,10 @@ export class Npm {
     options?: SpawnOptionsWithoutStdio,
   ): Promise<CommandResult<T>> {
     return new Promise<CommandResult<T>>((ok, ko) => {
-      const child = spawn(command, args, { ...options, stdio: ['inherit', 'pipe', 'pipe'] });
+      // On Windows, spawning a program ending in .cmd or .bat needs to run in a shell
+      // https://nodejs.org/en/blog/vulnerability/april-2024-security-releases-2
+      const shell = onWindows() && (command.endsWith('.cmd') || command.endsWith('.bat'));
+      const child = spawn(command, args, { shell, ...options, stdio: ['inherit', 'pipe', 'pipe'] });
       const stdout = new Array<Buffer>();
       child.stdout.on('data', (chunk) => {
         stdout.push(Buffer.from(chunk));
@@ -310,7 +316,7 @@ function chunksToObject(chunks: readonly Buffer[], encoding: BufferEncoding = 'u
     // observed these log lines always start with 'npm', so we filter those out.
     // for example: "npm notice New patch version of npm available! 8.1.0 -> 8.1.3"
     // for example: "npm ERR! must provide string spec"
-    const onlyJson = raw.split(os.EOL)
+    const onlyJson = raw.split(/[\r\n]+/) // split on any newlines, because npm returns inconsistent newline characters on Windows
       .filter(l => !l.startsWith('npm'))
       // Suppress debugger messages, if present...
       .filter(l => l !== 'Debugger attached.')
@@ -330,3 +336,22 @@ type ResponseObject =
   | { readonly error: { readonly code: string; readonly summary: string; readonly detail: string } }
   // The successful objects are treated as opaque blobs here
   | { readonly error: undefined; readonly [key: string]: unknown };
+
+/**
+ * Helper to detect if we are running on Windows.
+ */
+function onWindows() {
+  return process.platform === 'win32';
+}
+
+/**
+ * Get the npm binary path depending on the platform.
+ * @returns "npm.cmd" on Windows, otherwise "npm"
+ */
+function npmPlatformAwareCommand() {
+  if (onWindows()) {
+    return 'npm.cmd';
+  }
+
+  return 'npm';
+}

src/docgen/view/documentation.ts

@@ -407,7 +407,14 @@ export class Documentation {
       });
 
       const ts = new reflect.TypeSystem();
-      for (let dotJsii of await glob.promise(`${this.assembliesDir}/**/${SPEC_FILE_NAME}`)) {
+
+      // assembliesDir might include backslashes on Windows.
+      // The glob pattern must only used forward slashes, so we pass the assembliesDir as CWD which does not have this restriction
+      const assemblies = await glob.promise(`**/${SPEC_FILE_NAME}`, {
+        cwd: path.normalize(this.assembliesDir),
+        absolute: true,
+      });
+      for (let dotJsii of assemblies) {
         // we only transliterate the top level assembly and not the entire type-system.
         // note that the only reason to translate dependant assemblies is to show code examples
         // for expanded python arguments - which we don't to right now anyway.

test/docgen/view/documentation.test.ts

@@ -76,7 +76,7 @@ test('package installation does not run lifecycle hooks, includes optional depen
   }
 });
 
-// This test is only revent when running on non-macOS platforms.
+// This test is only relevant when running on non-macOS platforms.
 (process.platform === 'darwin' ? test.skip : test)('package installation uses --force when EBADPLATFORM is encountered', async () => {
   // The package has a hard dependency on fsevents, which only supports macOS platforms.
   const docs = await Documentation.forPackage(