From 0960e902d9fb68f2518cfb518707fe9e3a298b4b Mon Sep 17 00:00:00 2001 From: Kendra Neil <53584728+TheRealAmazonKendra@users.noreply.github.com> Date: Thu, 7 Jul 2022 22:16:09 -0700 Subject: [PATCH 1/6] fix(init-templates): csharp and fsharp init fails when path contains space When running or with a space in the path to the project, init will fail. This fix adds in handling for spaces and other special characters in the filepath for both windows systems and posix systems. Tests have been added for posix and manual testing was performed on a windows machine. Closes issue #18803. --- .../app/csharp/add-project.hook.ts | 30 ++------------- .../app/fsharp/add-project.hook.ts | 28 +------------- .../lib/{ => init-templates/app/util}/os.ts | 32 +++++++--------- .../sample-app/csharp/add-project.hook.ts | 2 +- .../sample-app/fsharp/add-project.hook.ts | 2 +- packages/aws-cdk/lib/init.ts | 37 +++++++++++++++++-- packages/aws-cdk/test/init.test.ts | 36 +++++++++++++++++- 7 files changed, 88 insertions(+), 79 deletions(-) rename packages/aws-cdk/lib/{ => init-templates/app/util}/os.ts (72%) diff --git a/packages/aws-cdk/lib/init-templates/app/csharp/add-project.hook.ts b/packages/aws-cdk/lib/init-templates/app/csharp/add-project.hook.ts index c839c1e01db08..b9b0d4d8691dc 100644 --- a/packages/aws-cdk/lib/init-templates/app/csharp/add-project.hook.ts +++ b/packages/aws-cdk/lib/init-templates/app/csharp/add-project.hook.ts @@ -1,33 +1,9 @@ -import * as child_process from 'child_process'; import * as path from 'path'; import { InvokeHook } from '../../../init'; +import { shell } from '../util/os'; export const invoke: InvokeHook = async (targetDirectory: string) => { const slnPath = path.join(targetDirectory, 'src', '%name.PascalCased%.sln'); const csprojPath = path.join(targetDirectory, 'src', '%name.PascalCased%', '%name.PascalCased%.csproj'); - - const child = child_process.spawn('dotnet', ['sln', slnPath, 'add', csprojPath], { - // Need this for Windows where we want .cmd and .bat to be found as well. - shell: true, - stdio: ['ignore', 'pipe', 'inherit'], - }); - - await new Promise((resolve, reject) => { - const stdout = new Array(); - - child.stdout.on('data', chunk => { - process.stdout.write(chunk); - stdout.push(chunk); - }); - - child.once('error', reject); - - child.once('exit', code => { - if (code === 0) { - resolve(Buffer.concat(stdout).toString('utf-8')); - } else { - reject(new Error(`Could not add project %name.PascalCased%.csproj to solution %name.PascalCased%.sln. Error code: ${code}`)); - } - }); - }); -}; + await shell(['dotnet', 'sln', slnPath, 'add', csprojPath], { errorMessage: 'Could not add project %name.PascalCased%.csproj to solution %name.PascalCased%.sln.' }); +}; \ No newline at end of file diff --git a/packages/aws-cdk/lib/init-templates/app/fsharp/add-project.hook.ts b/packages/aws-cdk/lib/init-templates/app/fsharp/add-project.hook.ts index efeed98d57ee2..9ae40e2dbce2a 100644 --- a/packages/aws-cdk/lib/init-templates/app/fsharp/add-project.hook.ts +++ b/packages/aws-cdk/lib/init-templates/app/fsharp/add-project.hook.ts @@ -1,33 +1,9 @@ -import * as child_process from 'child_process'; import * as path from 'path'; import { InvokeHook } from '../../../init'; +import { shell } from '../util/os'; export const invoke: InvokeHook = async (targetDirectory: string) => { const slnPath = path.join(targetDirectory, 'src', '%name.PascalCased%.sln'); const fsprojPath = path.join(targetDirectory, 'src', '%name.PascalCased%', '%name.PascalCased%.fsproj'); - - const child = child_process.spawn('dotnet', ['sln', slnPath, 'add', fsprojPath], { - // Need this for Windows where we want .cmd and .bat to be found as well. - shell: true, - stdio: ['ignore', 'pipe', 'inherit'], - }); - - await new Promise((resolve, reject) => { - const stdout = new Array(); - - child.stdout.on('data', chunk => { - process.stdout.write(chunk); - stdout.push(chunk); - }); - - child.once('error', reject); - - child.once('exit', code => { - if (code === 0) { - resolve(Buffer.concat(stdout).toString('utf-8')); - } else { - reject(new Error(`Could not add project %name.PascalCased%.fsproj to solution %name.PascalCased%.sln. Error code: ${code}`)); - } - }); - }); + await shell(['dotnet', 'sln', slnPath, 'add', fsprojPath], { errorMessage: 'Could not add project %name.PascalCased%.fsproj to solution %name.PascalCased%.sln.' }); }; diff --git a/packages/aws-cdk/lib/os.ts b/packages/aws-cdk/lib/init-templates/app/util/os.ts similarity index 72% rename from packages/aws-cdk/lib/os.ts rename to packages/aws-cdk/lib/init-templates/app/util/os.ts index 95d459a266145..c006f38663f15 100644 --- a/packages/aws-cdk/lib/os.ts +++ b/packages/aws-cdk/lib/init-templates/app/util/os.ts @@ -1,9 +1,7 @@ import * as child_process from 'child_process'; -import * as chalk from 'chalk'; -import { debug } from './logging'; export interface ShellOptions extends child_process.SpawnOptions { - quiet?: boolean; + errorMessage: string; } /** @@ -12,9 +10,9 @@ export interface ShellOptions extends child_process.SpawnOptions { * Shell function which both prints to stdout and collects the output into a * string. */ -export async function shell(command: string[], options: ShellOptions = {}): Promise { - debug(`Executing ${chalk.blue(renderCommandLine(command))}`); - const child = child_process.spawn(command[0], command.slice(1), { +export async function shell(command: string[], options: ShellOptions): Promise { + const child = child_process.spawn(command[0], renderArguments(command.slice(1)), { + shell: true, ...options, stdio: ['ignore', 'pipe', 'inherit'], }); @@ -24,9 +22,7 @@ export async function shell(command: string[], options: ShellOptions = {}): Prom // Both write to stdout and collect child.stdout.on('data', chunk => { - if (!options.quiet) { - process.stdout.write(chunk); - } + process.stdout.write(chunk); stdout.push(chunk); }); @@ -34,20 +30,18 @@ export async function shell(command: string[], options: ShellOptions = {}): Prom child.once('exit', code => { if (code === 0) { - resolve(Buffer.concat(stdout).toString('utf-8')); + resolve(Buffer.from(stdout).toString('utf-8')); } else { - reject(new Error(`${renderCommandLine(command)} exited with error code ${code}`)); + reject(new Error(`${options.errorMessage} exited with error code ${code}`)); } }); }); } /** - * Render the given command line as a string - * - * Probably missing some cases but giving it a good effort. + * Render the arguments to include escape characters for each platform. */ -function renderCommandLine(cmd: string[]) { +function renderArguments(cmd: string[]) { if (process.platform !== 'win32') { return doRender(cmd, hasAnyChars(' ', '\\', '!', '"', "'", '&', '$'), posixEscape); } else { @@ -58,8 +52,8 @@ function renderCommandLine(cmd: string[]) { /** * Render a UNIX command line */ -function doRender(cmd: string[], needsEscaping: (x: string) => boolean, doEscape: (x: string) => string): string { - return cmd.map(x => needsEscaping(x) ? doEscape(x) : x).join(' '); +function doRender(cmd: string[], needsEscaping: (x: string) => boolean, doEscape: (x: string) => string): string[] { + return cmd.map(x => needsEscaping(x) ? doEscape(x) : x); } /** @@ -78,7 +72,7 @@ function hasAnyChars(...chars: string[]): (x: string) => boolean { */ function posixEscape(x: string) { // Turn ' -> '"'"' - x = x.replace("'", "'\"'\"'"); + x = x.replace(/'/g, "'\"'\"'"); return `'${x}'`; } @@ -95,4 +89,4 @@ function windowsEscape(x: string): string { // Now escape all special characters const shellMeta = new Set(['"', '&', '^', '%']); return x.split('').map(c => shellMeta.has(x) ? '^' + c : c).join(''); -} +} \ No newline at end of file diff --git a/packages/aws-cdk/lib/init-templates/sample-app/csharp/add-project.hook.ts b/packages/aws-cdk/lib/init-templates/sample-app/csharp/add-project.hook.ts index c839c1e01db08..8276126645f90 100644 --- a/packages/aws-cdk/lib/init-templates/sample-app/csharp/add-project.hook.ts +++ b/packages/aws-cdk/lib/init-templates/sample-app/csharp/add-project.hook.ts @@ -24,7 +24,7 @@ export const invoke: InvokeHook = async (targetDirectory: string) => { child.once('exit', code => { if (code === 0) { - resolve(Buffer.concat(stdout).toString('utf-8')); + resolve(Buffer.from(stdout).toString('utf-8')); } else { reject(new Error(`Could not add project %name.PascalCased%.csproj to solution %name.PascalCased%.sln. Error code: ${code}`)); } diff --git a/packages/aws-cdk/lib/init-templates/sample-app/fsharp/add-project.hook.ts b/packages/aws-cdk/lib/init-templates/sample-app/fsharp/add-project.hook.ts index efeed98d57ee2..73e75e4e3b6f2 100644 --- a/packages/aws-cdk/lib/init-templates/sample-app/fsharp/add-project.hook.ts +++ b/packages/aws-cdk/lib/init-templates/sample-app/fsharp/add-project.hook.ts @@ -24,7 +24,7 @@ export const invoke: InvokeHook = async (targetDirectory: string) => { child.once('exit', code => { if (code === 0) { - resolve(Buffer.concat(stdout).toString('utf-8')); + resolve(Buffer.from(stdout).toString('utf-8')); } else { reject(new Error(`Could not add project %name.PascalCased%.fsproj to solution %name.PascalCased%.sln. Error code: ${code}`)); } diff --git a/packages/aws-cdk/lib/init.ts b/packages/aws-cdk/lib/init.ts index 0a6d1dc0cf458..7d79a68f66596 100644 --- a/packages/aws-cdk/lib/init.ts +++ b/packages/aws-cdk/lib/init.ts @@ -55,7 +55,7 @@ export async function cliInit(type?: string, language?: string, canUseNetwork = throw new Error('No language was selected'); } - await initializeProject(template, language, canUseNetwork, generateOnly, workDir); + await initializeProject(template, language, type, canUseNetwork, generateOnly, workDir); } /** @@ -106,7 +106,7 @@ export class InitTemplate { * @param language the language to instantiate this template with * @param targetDirectory the directory where the template is to be instantiated into */ - public async install(language: string, targetDirectory: string) { + public async install(language: string, type: string, targetDirectory: string) { if (this.languages.indexOf(language) === -1) { error(`The ${chalk.blue(language)} language is not supported for ${chalk.green(this.name)} ` + `(it supports: ${this.languages.map(l => chalk.blue(l)).join(', ')})`); @@ -129,11 +129,17 @@ export class InitTemplate { const sourceDirectory = path.join(this.basePath, language); const hookTempDirectory = path.join(targetDirectory, 'tmp'); + const tempShellDirectory = this.needsShell(language, type) ? await this.copyShellTo(targetDirectory) : undefined; + await fs.mkdir(hookTempDirectory); await this.installFiles(sourceDirectory, targetDirectory, language, projectInfo); await this.applyFutureFlags(targetDirectory); await this.invokeHooks(hookTempDirectory, targetDirectory, hookContext); await fs.remove(hookTempDirectory); + + if (tempShellDirectory) { + await fs.remove(tempShellDirectory); + } } private async installFiles(sourceDirectory: string, targetDirectory: string, language:string, project: ProjectInfo) { @@ -156,6 +162,22 @@ export class InitTemplate { } } + private needsShell(language: string, type: string): boolean { + return (language === 'csharp' || language === 'fsharp') && (type === 'app' || type === 'default'); + } + + private async copyShellTo(targetDirectory: string): Promise { + const osFile = 'os.js'; + const osDirectory = 'util'; + const utilSourceDirectory = path.join(this.basePath, osDirectory); + const utilTempDirectory = path.join(targetDirectory, osDirectory); + const fromFile = path.join(utilSourceDirectory, osFile); + const toFile = path.join(utilTempDirectory, osFile); + await fs.mkdir(utilTempDirectory); + await fs.copy(fromFile, toFile); + return utilTempDirectory; + } + /** * @summary Invoke any javascript hooks that exist in the template. * @description Sometimes templates need more complex logic than just replacing tokens. A 'hook' is @@ -287,10 +309,17 @@ export async function printAvailableTemplates(language?: string) { } } -async function initializeProject(template: InitTemplate, language: string, canUseNetwork: boolean, generateOnly: boolean, workDir: string) { +async function initializeProject( + template: InitTemplate, + language: string, + type: string, + canUseNetwork: boolean, + generateOnly: boolean, + workDir: string, +) { await assertIsEmptyDirectory(workDir); print(`Applying project template ${chalk.green(template.name)} for ${chalk.blue(language)}`); - await template.install(language, workDir); + await template.install(language, type, workDir); if (await fs.pathExists('README.md')) { print(chalk.green(await fs.readFile('README.md', { encoding: 'utf-8' }))); } diff --git a/packages/aws-cdk/test/init.test.ts b/packages/aws-cdk/test/init.test.ts index fc4750beef69f..7f625c538823e 100644 --- a/packages/aws-cdk/test/init.test.ts +++ b/packages/aws-cdk/test/init.test.ts @@ -76,6 +76,28 @@ describe('constructs version', () => { expect(sln).toContainEqual(expect.stringMatching(/\"AwsCdkTest[a-zA-Z0-9]{6}\\AwsCdkTest[a-zA-Z0-9]{6}.fsproj\"/)); }); + cliTestWithDirSpaces('csharp app with spaces', async (workDir) => { + await cliInit('app', 'csharp', false, true, workDir); + + const csprojFile = (await recursiveListFiles(workDir)).filter(f => f.endsWith('.csproj'))[0]; + expect(csprojFile).toBeDefined(); + + const csproj = (await fs.readFile(csprojFile, 'utf8')).split(/\r?\n/); + + expect(csproj).toContainEqual(expect.stringMatching(/\ { + await cliInit('app', 'fsharp', false, true, workDir); + + const fsprojFile = (await recursiveListFiles(workDir)).filter(f => f.endsWith('.fsproj'))[0]; + expect(fsprojFile).toBeDefined(); + + const fsproj = (await fs.readFile(fsprojFile, 'utf8')).split(/\r?\n/); + + expect(fsproj).toContainEqual(expect.stringMatching(/\ { await cliInit('app', 'python', false, true, workDir); @@ -147,7 +169,6 @@ describe('constructs version', () => { }); test('when no version number is present (e.g., local development), the v2 templates are chosen by default', async () => { - expect((await availableInitTemplates()).length).toBeGreaterThan(0); }); @@ -164,6 +185,19 @@ async function withTempDir(cb: (dir: string) => void | Promise) { } } +function cliTestWithDirSpaces(name: string, handler: (dir: string) => void | Promise): void { + test(name, () => withTempDirWithSpaces(handler)); +} + +async function withTempDirWithSpaces(cb: (dir: string) => void | Promise) { + const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'aws-cdk-test with-space')); + try { + await cb(tmpDir); + } finally { + // await fs.remove(tmpDir); + } +} + /** * List all files underneath dir */ From 62a420542cbd55aabec3b6de43fd75c673c4fbf4 Mon Sep 17 00:00:00 2001 From: Kendra Neil <53584728+TheRealAmazonKendra@users.noreply.github.com> Date: Thu, 8 Sep 2022 20:31:03 -0700 Subject: [PATCH 2/6] move temp hook folder to source folder --- .../app/csharp/add-project.hook.ts | 4 +- .../app/fsharp/add-project.hook.ts | 4 +- packages/aws-cdk/lib/init.ts | 59 ++++++++----------- .../lib/{init-templates/app/util => }/os.ts | 20 +++---- packages/aws-cdk/test/init.test.ts | 2 +- 5 files changed, 37 insertions(+), 52 deletions(-) rename packages/aws-cdk/lib/{init-templates/app/util => }/os.ts (79%) diff --git a/packages/aws-cdk/lib/init-templates/app/csharp/add-project.hook.ts b/packages/aws-cdk/lib/init-templates/app/csharp/add-project.hook.ts index b9b0d4d8691dc..637f16f4eb0c4 100644 --- a/packages/aws-cdk/lib/init-templates/app/csharp/add-project.hook.ts +++ b/packages/aws-cdk/lib/init-templates/app/csharp/add-project.hook.ts @@ -1,9 +1,9 @@ import * as path from 'path'; import { InvokeHook } from '../../../init'; -import { shell } from '../util/os'; +import { shell } from '../../../os'; export const invoke: InvokeHook = async (targetDirectory: string) => { const slnPath = path.join(targetDirectory, 'src', '%name.PascalCased%.sln'); const csprojPath = path.join(targetDirectory, 'src', '%name.PascalCased%', '%name.PascalCased%.csproj'); - await shell(['dotnet', 'sln', slnPath, 'add', csprojPath], { errorMessage: 'Could not add project %name.PascalCased%.csproj to solution %name.PascalCased%.sln.' }); + await shell(['dotnet', 'sln', slnPath, 'add', csprojPath]); }; \ No newline at end of file diff --git a/packages/aws-cdk/lib/init-templates/app/fsharp/add-project.hook.ts b/packages/aws-cdk/lib/init-templates/app/fsharp/add-project.hook.ts index 9ae40e2dbce2a..b172240efb0c3 100644 --- a/packages/aws-cdk/lib/init-templates/app/fsharp/add-project.hook.ts +++ b/packages/aws-cdk/lib/init-templates/app/fsharp/add-project.hook.ts @@ -1,9 +1,9 @@ import * as path from 'path'; import { InvokeHook } from '../../../init'; -import { shell } from '../util/os'; +import { shell } from '../../../os'; export const invoke: InvokeHook = async (targetDirectory: string) => { const slnPath = path.join(targetDirectory, 'src', '%name.PascalCased%.sln'); const fsprojPath = path.join(targetDirectory, 'src', '%name.PascalCased%', '%name.PascalCased%.fsproj'); - await shell(['dotnet', 'sln', slnPath, 'add', fsprojPath], { errorMessage: 'Could not add project %name.PascalCased%.fsproj to solution %name.PascalCased%.sln.' }); + await shell(['dotnet', 'sln', slnPath, 'add', fsprojPath]); }; diff --git a/packages/aws-cdk/lib/init.ts b/packages/aws-cdk/lib/init.ts index 7d79a68f66596..84df4c1d0ff1b 100644 --- a/packages/aws-cdk/lib/init.ts +++ b/packages/aws-cdk/lib/init.ts @@ -55,7 +55,7 @@ export async function cliInit(type?: string, language?: string, canUseNetwork = throw new Error('No language was selected'); } - await initializeProject(template, language, type, canUseNetwork, generateOnly, workDir); + await initializeProject(template, language, canUseNetwork, generateOnly, workDir); } /** @@ -69,11 +69,12 @@ function pythonExecutable() { return python; } const INFO_DOT_JSON = 'info.json'; +const HOOK_DIR_PREFIX = 'tmp'; export class InitTemplate { public static async fromName(templatesDir: string, name: string) { const basePath = path.join(templatesDir, name); - const languages = (await listDirectory(basePath)).filter(f => f !== INFO_DOT_JSON); + const languages = (await listDirectory(basePath)); const info = await fs.readJson(path.join(basePath, INFO_DOT_JSON)); return new InitTemplate(basePath, name, languages, info); } @@ -106,7 +107,7 @@ export class InitTemplate { * @param language the language to instantiate this template with * @param targetDirectory the directory where the template is to be instantiated into */ - public async install(language: string, type: string, targetDirectory: string) { + public async install(language: string, targetDirectory: string) { if (this.languages.indexOf(language) === -1) { error(`The ${chalk.blue(language)} language is not supported for ${chalk.green(this.name)} ` + `(it supports: ${this.languages.map(l => chalk.blue(l)).join(', ')})`); @@ -117,6 +118,9 @@ export class InitTemplate { name: decamelize(path.basename(path.resolve(targetDirectory))), }; + const sourceDirectory = path.join(this.basePath, language); + const hookTempDirectory = path.join(this.basePath, `${HOOK_DIR_PREFIX}-${projectInfo.name}`); + const hookContext: HookContext = { substitutePlaceholdersIn: async (...fileNames: string[]) => { for (const fileName of fileNames) { @@ -127,34 +131,31 @@ export class InitTemplate { }, }; - const sourceDirectory = path.join(this.basePath, language); - const hookTempDirectory = path.join(targetDirectory, 'tmp'); - const tempShellDirectory = this.needsShell(language, type) ? await this.copyShellTo(targetDirectory) : undefined; - - await fs.mkdir(hookTempDirectory); - await this.installFiles(sourceDirectory, targetDirectory, language, projectInfo); - await this.applyFutureFlags(targetDirectory); - await this.invokeHooks(hookTempDirectory, targetDirectory, hookContext); - await fs.remove(hookTempDirectory); - - if (tempShellDirectory) { - await fs.remove(tempShellDirectory); + try { + await fs.mkdir(hookTempDirectory); + await this.installFiles(sourceDirectory, targetDirectory, hookTempDirectory, language, projectInfo); + await this.applyFutureFlags(targetDirectory); + await this.invokeHooks(hookTempDirectory, targetDirectory, hookContext); + } catch (e) { + warning(`Unable to create ${projectInfo.name}: ${e.message}`); + } finally { + await fs.remove(hookTempDirectory); } } - private async installFiles(sourceDirectory: string, targetDirectory: string, language:string, project: ProjectInfo) { + private async installFiles(sourceDirectory: string, targetDirectory: string, hookTempDirectory: string, language:string, project: ProjectInfo) { for (const file of await fs.readdir(sourceDirectory)) { const fromFile = path.join(sourceDirectory, file); const toFile = path.join(targetDirectory, this.expand(file, language, project)); if ((await fs.stat(fromFile)).isDirectory()) { await fs.mkdir(toFile); - await this.installFiles(fromFile, toFile, language, project); + await this.installFiles(fromFile, toFile, hookTempDirectory, language, project); continue; } else if (file.match(/^.*\.template\.[^.]+$/)) { await this.installProcessed(fromFile, toFile.replace(/\.template(\.[^.]+)$/, '$1'), language, project); continue; } else if (file.match(/^.*\.hook\.(d.)?[^.]+$/)) { - await this.installProcessed(fromFile, path.join(targetDirectory, 'tmp', file), language, project); + await this.installProcessed(fromFile, path.join(hookTempDirectory, file), language, project); continue; } else { await fs.copy(fromFile, toFile); @@ -162,22 +163,6 @@ export class InitTemplate { } } - private needsShell(language: string, type: string): boolean { - return (language === 'csharp' || language === 'fsharp') && (type === 'app' || type === 'default'); - } - - private async copyShellTo(targetDirectory: string): Promise { - const osFile = 'os.js'; - const osDirectory = 'util'; - const utilSourceDirectory = path.join(this.basePath, osDirectory); - const utilTempDirectory = path.join(targetDirectory, osDirectory); - const fromFile = path.join(utilSourceDirectory, osFile); - const toFile = path.join(utilTempDirectory, osFile); - await fs.mkdir(utilTempDirectory); - await fs.copy(fromFile, toFile); - return utilTempDirectory; - } - /** * @summary Invoke any javascript hooks that exist in the template. * @description Sometimes templates need more complex logic than just replacing tokens. A 'hook' is @@ -294,6 +279,9 @@ async function listDirectory(dirPath: string) { return (await fs.readdir(dirPath)) .filter(p => !p.startsWith('.')) .filter(p => !(p === 'LICENSE')) + // if, for some reason, the temp folder for the hook doesn't get deleted we don't want to display it in this list + .filter(p => !(p.startsWith(HOOK_DIR_PREFIX))) + .filter(p => !(p === INFO_DOT_JSON)) .sort(); } @@ -312,14 +300,13 @@ export async function printAvailableTemplates(language?: string) { async function initializeProject( template: InitTemplate, language: string, - type: string, canUseNetwork: boolean, generateOnly: boolean, workDir: string, ) { await assertIsEmptyDirectory(workDir); print(`Applying project template ${chalk.green(template.name)} for ${chalk.blue(language)}`); - await template.install(language, type, workDir); + await template.install(language, workDir); if (await fs.pathExists('README.md')) { print(chalk.green(await fs.readFile('README.md', { encoding: 'utf-8' }))); } diff --git a/packages/aws-cdk/lib/init-templates/app/util/os.ts b/packages/aws-cdk/lib/os.ts similarity index 79% rename from packages/aws-cdk/lib/init-templates/app/util/os.ts rename to packages/aws-cdk/lib/os.ts index c006f38663f15..b0898e765bf83 100644 --- a/packages/aws-cdk/lib/init-templates/app/util/os.ts +++ b/packages/aws-cdk/lib/os.ts @@ -1,8 +1,6 @@ import * as child_process from 'child_process'; - -export interface ShellOptions extends child_process.SpawnOptions { - errorMessage: string; -} +import * as chalk from 'chalk'; +import { debug } from './logging'; /** * OS helpers @@ -10,10 +8,10 @@ export interface ShellOptions extends child_process.SpawnOptions { * Shell function which both prints to stdout and collects the output into a * string. */ -export async function shell(command: string[], options: ShellOptions): Promise { - const child = child_process.spawn(command[0], renderArguments(command.slice(1)), { - shell: true, - ...options, +export async function shell(command: string[]): Promise { + const renderedCommand = renderCommandLine(command); + debug(`Executing ${chalk.blue(renderedCommand)}`); + const child = child_process.spawn(renderedCommand[0], renderCommandLine(renderedCommand.slice(1)), { stdio: ['ignore', 'pipe', 'inherit'], }); @@ -32,16 +30,16 @@ export async function shell(command: string[], options: ShellOptions): Promise void | Promise) { try { await cb(tmpDir); } finally { - // await fs.remove(tmpDir); + await fs.remove(tmpDir); } } From fa02577452951f28402e02a38cfe137a42f8e017 Mon Sep 17 00:00:00 2001 From: Kendra Neil <53584728+TheRealAmazonKendra@users.noreply.github.com> Date: Thu, 8 Sep 2022 20:40:13 -0700 Subject: [PATCH 3/6] formatting and missing shell line --- packages/aws-cdk/lib/init.ts | 10 ++-------- packages/aws-cdk/lib/os.ts | 2 ++ 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/packages/aws-cdk/lib/init.ts b/packages/aws-cdk/lib/init.ts index 84df4c1d0ff1b..e5ecef421cde4 100644 --- a/packages/aws-cdk/lib/init.ts +++ b/packages/aws-cdk/lib/init.ts @@ -280,7 +280,7 @@ async function listDirectory(dirPath: string) { .filter(p => !p.startsWith('.')) .filter(p => !(p === 'LICENSE')) // if, for some reason, the temp folder for the hook doesn't get deleted we don't want to display it in this list - .filter(p => !(p.startsWith(HOOK_DIR_PREFIX))) + .filter(p => !p.startsWith(HOOK_DIR_PREFIX)) .filter(p => !(p === INFO_DOT_JSON)) .sort(); } @@ -297,13 +297,7 @@ export async function printAvailableTemplates(language?: string) { } } -async function initializeProject( - template: InitTemplate, - language: string, - canUseNetwork: boolean, - generateOnly: boolean, - workDir: string, -) { +async function initializeProject(template: InitTemplate, language: string, canUseNetwork: boolean, generateOnly: boolean, workDir: string) { await assertIsEmptyDirectory(workDir); print(`Applying project template ${chalk.green(template.name)} for ${chalk.blue(language)}`); await template.install(language, workDir); diff --git a/packages/aws-cdk/lib/os.ts b/packages/aws-cdk/lib/os.ts index b0898e765bf83..5b2ee4cad0594 100644 --- a/packages/aws-cdk/lib/os.ts +++ b/packages/aws-cdk/lib/os.ts @@ -12,6 +12,8 @@ export async function shell(command: string[]): Promise { const renderedCommand = renderCommandLine(command); debug(`Executing ${chalk.blue(renderedCommand)}`); const child = child_process.spawn(renderedCommand[0], renderCommandLine(renderedCommand.slice(1)), { + // Need this for Windows where we want .cmd and .bat to be found as well. + shell: true, stdio: ['ignore', 'pipe', 'inherit'], }); From 8a7d16b8658dcae29bbe9ee6f13be6f33624bb13 Mon Sep 17 00:00:00 2001 From: Kendra Neil <53584728+TheRealAmazonKendra@users.noreply.github.com> Date: Thu, 8 Sep 2022 20:58:34 -0700 Subject: [PATCH 4/6] add more context to error message --- .../lib/init-templates/app/csharp/add-project.hook.ts | 7 ++++++- .../lib/init-templates/app/fsharp/add-project.hook.ts | 6 +++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/packages/aws-cdk/lib/init-templates/app/csharp/add-project.hook.ts b/packages/aws-cdk/lib/init-templates/app/csharp/add-project.hook.ts index 637f16f4eb0c4..bf31336ca5888 100644 --- a/packages/aws-cdk/lib/init-templates/app/csharp/add-project.hook.ts +++ b/packages/aws-cdk/lib/init-templates/app/csharp/add-project.hook.ts @@ -5,5 +5,10 @@ import { shell } from '../../../os'; export const invoke: InvokeHook = async (targetDirectory: string) => { const slnPath = path.join(targetDirectory, 'src', '%name.PascalCased%.sln'); const csprojPath = path.join(targetDirectory, 'src', '%name.PascalCased%', '%name.PascalCased%.csproj'); - await shell(['dotnet', 'sln', slnPath, 'add', csprojPath]); + try { + await shell(['dotnet', 'sln', slnPath, 'add', csprojPath]); + } catch (e) { + throw new Error(`Could not add project %name.PascalCased%.csproj to solution %name.PascalCased%.sln. ${e.message}`); + } + }; \ No newline at end of file diff --git a/packages/aws-cdk/lib/init-templates/app/fsharp/add-project.hook.ts b/packages/aws-cdk/lib/init-templates/app/fsharp/add-project.hook.ts index b172240efb0c3..e3cae76dba992 100644 --- a/packages/aws-cdk/lib/init-templates/app/fsharp/add-project.hook.ts +++ b/packages/aws-cdk/lib/init-templates/app/fsharp/add-project.hook.ts @@ -5,5 +5,9 @@ import { shell } from '../../../os'; export const invoke: InvokeHook = async (targetDirectory: string) => { const slnPath = path.join(targetDirectory, 'src', '%name.PascalCased%.sln'); const fsprojPath = path.join(targetDirectory, 'src', '%name.PascalCased%', '%name.PascalCased%.fsproj'); - await shell(['dotnet', 'sln', slnPath, 'add', fsprojPath]); + try { + await shell(['dotnet', 'sln', slnPath, 'add', fsprojPath]); + } catch (e) { + throw new Error(`Could not add project %name.PascalCased%.fsproj to solution %name.PascalCased%.sln. ${e.message}`); + } }; From 4929d3e8f6c80cd6fb446ccb8c7488f3e607c525 Mon Sep 17 00:00:00 2001 From: Kendra Neil <53584728+TheRealAmazonKendra@users.noreply.github.com> Date: Thu, 8 Sep 2022 22:28:30 -0700 Subject: [PATCH 5/6] render whole line --- packages/aws-cdk/lib/os.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/aws-cdk/lib/os.ts b/packages/aws-cdk/lib/os.ts index 5b2ee4cad0594..0ae21508e7e0c 100644 --- a/packages/aws-cdk/lib/os.ts +++ b/packages/aws-cdk/lib/os.ts @@ -9,9 +9,9 @@ import { debug } from './logging'; * string. */ export async function shell(command: string[]): Promise { - const renderedCommand = renderCommandLine(command); - debug(`Executing ${chalk.blue(renderedCommand)}`); - const child = child_process.spawn(renderedCommand[0], renderCommandLine(renderedCommand.slice(1)), { + const commandLine = renderCommandLine(command); + debug(`Executing ${chalk.blue(commandLine)}`); + const child = child_process.spawn(command[0], renderArguments(command.slice(1)), { // Need this for Windows where we want .cmd and .bat to be found as well. shell: true, stdio: ['ignore', 'pipe', 'inherit'], @@ -32,16 +32,20 @@ export async function shell(command: string[]): Promise { if (code === 0) { resolve(Buffer.from(stdout).toString('utf-8')); } else { - reject(new Error(`${renderedCommand} exited with error code ${code}`)); + reject(new Error(`${commandLine} exited with error code ${code}`)); } }); }); } +function renderCommandLine(cmd: string[]) { + return renderArguments(cmd).join(' '); +} + /** - * Render the command line to include escape characters for each platform. + * Render the arguments to include escape characters for each platform. */ -function renderCommandLine(cmd: string[]) { +function renderArguments(cmd: string[]) { if (process.platform !== 'win32') { return doRender(cmd, hasAnyChars(' ', '\\', '!', '"', "'", '&', '$'), posixEscape); } else { From 1f4a16b6b52606d146c7cb94ceaeab56acb98109 Mon Sep 17 00:00:00 2001 From: Kendra Neil <53584728+TheRealAmazonKendra@users.noreply.github.com> Date: Thu, 8 Sep 2022 23:19:18 -0700 Subject: [PATCH 6/6] update sample apps as well --- .../app/csharp/add-project.hook.ts | 3 +- .../sample-app/csharp/add-project.hook.ts | 32 ++++--------------- .../sample-app/fsharp/add-project.hook.ts | 32 ++++--------------- 3 files changed, 13 insertions(+), 54 deletions(-) diff --git a/packages/aws-cdk/lib/init-templates/app/csharp/add-project.hook.ts b/packages/aws-cdk/lib/init-templates/app/csharp/add-project.hook.ts index bf31336ca5888..ad074041fab32 100644 --- a/packages/aws-cdk/lib/init-templates/app/csharp/add-project.hook.ts +++ b/packages/aws-cdk/lib/init-templates/app/csharp/add-project.hook.ts @@ -10,5 +10,4 @@ export const invoke: InvokeHook = async (targetDirectory: string) => { } catch (e) { throw new Error(`Could not add project %name.PascalCased%.csproj to solution %name.PascalCased%.sln. ${e.message}`); } - -}; \ No newline at end of file +}; diff --git a/packages/aws-cdk/lib/init-templates/sample-app/csharp/add-project.hook.ts b/packages/aws-cdk/lib/init-templates/sample-app/csharp/add-project.hook.ts index 8276126645f90..ad074041fab32 100644 --- a/packages/aws-cdk/lib/init-templates/sample-app/csharp/add-project.hook.ts +++ b/packages/aws-cdk/lib/init-templates/sample-app/csharp/add-project.hook.ts @@ -1,33 +1,13 @@ -import * as child_process from 'child_process'; import * as path from 'path'; import { InvokeHook } from '../../../init'; +import { shell } from '../../../os'; export const invoke: InvokeHook = async (targetDirectory: string) => { const slnPath = path.join(targetDirectory, 'src', '%name.PascalCased%.sln'); const csprojPath = path.join(targetDirectory, 'src', '%name.PascalCased%', '%name.PascalCased%.csproj'); - - const child = child_process.spawn('dotnet', ['sln', slnPath, 'add', csprojPath], { - // Need this for Windows where we want .cmd and .bat to be found as well. - shell: true, - stdio: ['ignore', 'pipe', 'inherit'], - }); - - await new Promise((resolve, reject) => { - const stdout = new Array(); - - child.stdout.on('data', chunk => { - process.stdout.write(chunk); - stdout.push(chunk); - }); - - child.once('error', reject); - - child.once('exit', code => { - if (code === 0) { - resolve(Buffer.from(stdout).toString('utf-8')); - } else { - reject(new Error(`Could not add project %name.PascalCased%.csproj to solution %name.PascalCased%.sln. Error code: ${code}`)); - } - }); - }); + try { + await shell(['dotnet', 'sln', slnPath, 'add', csprojPath]); + } catch (e) { + throw new Error(`Could not add project %name.PascalCased%.csproj to solution %name.PascalCased%.sln. ${e.message}`); + } }; diff --git a/packages/aws-cdk/lib/init-templates/sample-app/fsharp/add-project.hook.ts b/packages/aws-cdk/lib/init-templates/sample-app/fsharp/add-project.hook.ts index 73e75e4e3b6f2..e3cae76dba992 100644 --- a/packages/aws-cdk/lib/init-templates/sample-app/fsharp/add-project.hook.ts +++ b/packages/aws-cdk/lib/init-templates/sample-app/fsharp/add-project.hook.ts @@ -1,33 +1,13 @@ -import * as child_process from 'child_process'; import * as path from 'path'; import { InvokeHook } from '../../../init'; +import { shell } from '../../../os'; export const invoke: InvokeHook = async (targetDirectory: string) => { const slnPath = path.join(targetDirectory, 'src', '%name.PascalCased%.sln'); const fsprojPath = path.join(targetDirectory, 'src', '%name.PascalCased%', '%name.PascalCased%.fsproj'); - - const child = child_process.spawn('dotnet', ['sln', slnPath, 'add', fsprojPath], { - // Need this for Windows where we want .cmd and .bat to be found as well. - shell: true, - stdio: ['ignore', 'pipe', 'inherit'], - }); - - await new Promise((resolve, reject) => { - const stdout = new Array(); - - child.stdout.on('data', chunk => { - process.stdout.write(chunk); - stdout.push(chunk); - }); - - child.once('error', reject); - - child.once('exit', code => { - if (code === 0) { - resolve(Buffer.from(stdout).toString('utf-8')); - } else { - reject(new Error(`Could not add project %name.PascalCased%.fsproj to solution %name.PascalCased%.sln. Error code: ${code}`)); - } - }); - }); + try { + await shell(['dotnet', 'sln', slnPath, 'add', fsprojPath]); + } catch (e) { + throw new Error(`Could not add project %name.PascalCased%.fsproj to solution %name.PascalCased%.sln. ${e.message}`); + } };