From 5850a3ad82151e82faf91e472d7ed7d331c689b7 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] 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 */