diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 4e3824fb..58f19862 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -79,7 +79,7 @@ jobs: git commit -s -m "chore: self mutation" git push origin HEAD:$PULL_REQUEST_REF rosetta-compat: - runs-on: ubuntu-latest + runs-on: ${{ matrix.os }} permissions: {} env: CI: "true" @@ -101,11 +101,14 @@ jobs: - name: compile+test run: |- npx projen compile - npx projen test + npx projen test --runInBand - name: Check Rosetta version run: test $(npx jsii-rosetta --version) = "${{ matrix.rosetta }}" strategy: matrix: + include: + - rosetta: 5.4.0 + os: windows-latest rosetta: - 1.85.0 - 5.0.14 @@ -113,3 +116,5 @@ jobs: - 5.2.0 - 5.3.0 - 5.4.0 + os: + - ubuntu-latest diff --git a/.projenrc.ts b/.projenrc.ts index bf78d740..9dd3f361 100644 --- a/.projenrc.ts +++ b/.projenrc.ts @@ -47,6 +47,11 @@ const project = new CdklabsTypeScriptProject({ jestOptions: { jestConfig: { setupFilesAfterEnv: ['/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 + '/test/**/*.test.ts', + ], }, }, tsconfig: { diff --git a/package.json b/package.json index ecf09ac2..fc63d553 100644 --- a/package.json +++ b/package.json @@ -78,6 +78,7 @@ "/test/setup-jest.ts" ], "testMatch": [ + "/test/**/*.test.ts", "/src/**/__tests__/**/*.ts?(x)", "/(test|src)/**/*(*.)@(spec|test).ts?(x)" ], diff --git a/projenrc/rosetta.ts b/projenrc/rosetta.ts index 4d0c8c0c..229c3d74 100644 --- a/projenrc/rosetta.ts +++ b/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'; + } } diff --git a/src/cli.ts b/src/cli.ts index 5ecac7c4..eba98920 100644 --- a/src/cli.ts +++ b/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; diff --git a/src/docgen/view/_npm.ts b/src/docgen/view/_npm.ts index c0201a11..f81ce153 100644 --- a/src/docgen/view/_npm.ts +++ b/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> { return new Promise>((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(); 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'; +} diff --git a/src/docgen/view/documentation.ts b/src/docgen/view/documentation.ts index 5942b2cc..c915c61a 100644 --- a/src/docgen/view/documentation.ts +++ b/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. diff --git a/test/docgen/view/documentation.test.ts b/test/docgen/view/documentation.test.ts index fb6104fd..00d0e961 100644 --- a/test/docgen/view/documentation.test.ts +++ b/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(