revert(init-templates): csharp and fsharp app init fails when path co…

…ntains space (aws#22112) Reverts aws#21049 Resolves aws#22090
homakk · Oct 10, 2022 · 9651021 · 9651021
1 parent 8f5be46
commit 9651021
Show file tree

Hide file tree

Showing 7 changed files with 137 additions and 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
@@ -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<string>((resolve, reject) => {
+    const stdout = new Array<any>();
+
+    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
@@ -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<string>((resolve, reject) => {
+    const stdout = new Array<any>();
+
+    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
@@ -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<string>((resolve, reject) => {
+    const stdout = new Array<any>();
+
+    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
@@ -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<string>((resolve, reject) => {
+    const stdout = new Array<any>();
+
+    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
@@ -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
@@ -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<string> {
-  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<string> {
+  debug(`Executing ${chalk.blue(renderCommandLine(command))}`);
+  const child = child_process.spawn(command[0], command.slice(1), {
+    ...options,
     stdio: ['ignore', 'pipe', 'inherit'],
   });
 
@@ -22,30 +24,30 @@ export async function shell(command: string[]): Promise<string> {
 
     // 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);
     });
 
     child.once('error', reject);
 
     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<string>(['"', '&', '^', '%']);
   return x.split('').map(c => shellMeta.has(x) ? '^' + c : c).join('');
-}
+}
diff --git 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(/\<PackageReference Include="Constructs" Version="\[10\..*,11\..*\)"/));
-  });
-
-  cliTestWithDirSpaces('fsharp app with spaces', async (workDir) => {
-    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(/\<PackageReference Include="Constructs" Version="\[10\..*,11\..*\)"/));
-  });
-
   cliTest('create a Python app project', async (workDir) => {
     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<any>) {
   }
 }
 
-function cliTestWithDirSpaces(name: string, handler: (dir: string) => void | Promise<any>): void {
-  test(name, () => withTempDirWithSpaces(handler));
-}
-
-async function withTempDirWithSpaces(cb: (dir: string) => void | Promise<any>) {
-  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
  */