From 9651021ffdd67b53a67b29f1ad86edd96e05cb5b Mon Sep 17 00:00:00 2001 From: Eli Polonsky Date: Mon, 19 Sep 2022 18:11:46 +0300 Subject: [PATCH] revert(init-templates): csharp and fsharp app init fails when path contains space (#22112) Reverts aws/aws-cdk#21049 Resolves https://github.com/aws/aws-cdk/issues/22090 --- .../app/csharp/add-project.hook.ts | 32 ++++++++++++--- .../app/fsharp/add-project.hook.ts | 32 ++++++++++++--- .../sample-app/csharp/add-project.hook.ts | 32 ++++++++++++--- .../sample-app/fsharp/add-project.hook.ts | 32 ++++++++++++--- packages/aws-cdk/lib/init.ts | 32 +++++---------- packages/aws-cdk/lib/os.ts | 40 ++++++++++--------- packages/aws-cdk/test/init.test.ts | 36 +---------------- 7 files changed, 137 insertions(+), 99 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 ad074041fab32..c839c1e01db08 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,13 +1,33 @@ +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'); - 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}`); - } + + 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}`)); + } + }); + }); }; 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 e3cae76dba992..efeed98d57ee2 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,13 +1,33 @@ +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'); - 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}`); - } + + 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}`)); + } + }); + }); }; 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 ad074041fab32..c839c1e01db08 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,13 +1,33 @@ +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'); - 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}`); - } + + 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}`)); + } + }); + }); }; 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 e3cae76dba992..efeed98d57ee2 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,13 +1,33 @@ +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'); - 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}`); - } + + 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}`)); + } + }); + }); }; diff --git a/packages/aws-cdk/lib/init.ts b/packages/aws-cdk/lib/init.ts index e5ecef421cde4..0a6d1dc0cf458 100644 --- a/packages/aws-cdk/lib/init.ts +++ b/packages/aws-cdk/lib/init.ts @@ -69,12 +69,11 @@ 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)); + const languages = (await listDirectory(basePath)).filter(f => f !== INFO_DOT_JSON); const info = await fs.readJson(path.join(basePath, INFO_DOT_JSON)); return new InitTemplate(basePath, name, languages, info); } @@ -118,9 +117,6 @@ 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) { @@ -131,31 +127,28 @@ export class InitTemplate { }, }; - 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); - } + const sourceDirectory = path.join(this.basePath, language); + const hookTempDirectory = path.join(targetDirectory, 'tmp'); + 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); } - private async installFiles(sourceDirectory: string, targetDirectory: string, hookTempDirectory: string, language:string, project: ProjectInfo) { + private async installFiles(sourceDirectory: string, targetDirectory: 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, hookTempDirectory, language, project); + await this.installFiles(fromFile, toFile, 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(hookTempDirectory, file), language, project); + await this.installProcessed(fromFile, path.join(targetDirectory, 'tmp', file), language, project); continue; } else { await fs.copy(fromFile, toFile); @@ -279,9 +272,6 @@ 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(); } diff --git a/packages/aws-cdk/lib/os.ts b/packages/aws-cdk/lib/os.ts index 0ae21508e7e0c..95d459a266145 100644 --- a/packages/aws-cdk/lib/os.ts +++ b/packages/aws-cdk/lib/os.ts @@ -2,18 +2,20 @@ 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; +} + /** * OS helpers * * Shell function which both prints to stdout and collects the output into a * string. */ -export async function shell(command: string[]): Promise { - 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, +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), { + ...options, stdio: ['ignore', 'pipe', 'inherit'], }); @@ -22,7 +24,9 @@ export async function shell(command: string[]): Promise { // Both write to stdout and collect child.stdout.on('data', chunk => { - process.stdout.write(chunk); + if (!options.quiet) { + process.stdout.write(chunk); + } stdout.push(chunk); }); @@ -30,22 +34,20 @@ export async function shell(command: string[]): Promise { child.once('exit', code => { if (code === 0) { - resolve(Buffer.from(stdout).toString('utf-8')); + resolve(Buffer.concat(stdout).toString('utf-8')); } else { - reject(new Error(`${commandLine} exited with error code ${code}`)); + reject(new Error(`${renderCommandLine(command)} exited with error code ${code}`)); } }); }); } -function renderCommandLine(cmd: string[]) { - return renderArguments(cmd).join(' '); -} - /** - * Render the arguments to include escape characters for each platform. + * Render the given command line as a string + * + * Probably missing some cases but giving it a good effort. */ -function renderArguments(cmd: string[]) { +function renderCommandLine(cmd: string[]) { if (process.platform !== 'win32') { return doRender(cmd, hasAnyChars(' ', '\\', '!', '"', "'", '&', '$'), posixEscape); } else { @@ -56,8 +58,8 @@ function renderArguments(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); +function doRender(cmd: string[], needsEscaping: (x: string) => boolean, doEscape: (x: string) => string): string { + return cmd.map(x => needsEscaping(x) ? doEscape(x) : x).join(' '); } /** @@ -76,7 +78,7 @@ function hasAnyChars(...chars: string[]): (x: string) => boolean { */ function posixEscape(x: string) { // Turn ' -> '"'"' - x = x.replace(/'/g, "'\"'\"'"); + x = x.replace("'", "'\"'\"'"); return `'${x}'`; } @@ -93,4 +95,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/test/init.test.ts b/packages/aws-cdk/test/init.test.ts index 0e674ff8ab963..fc4750beef69f 100644 --- a/packages/aws-cdk/test/init.test.ts +++ b/packages/aws-cdk/test/init.test.ts @@ -76,28 +76,6 @@ 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); @@ -169,6 +147,7 @@ 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); }); @@ -185,19 +164,6 @@ 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 */