Skip to content

Commit

Permalink
fix(init-templates): csharp and fsharp init fails when path contains …
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
TheRealAmazonKendra committed Sep 3, 2022
1 parent 80cb527 commit 0960e90
Show file tree
Hide file tree
Showing 7 changed files with 88 additions and 79 deletions.
30 changes: 3 additions & 27 deletions packages/aws-cdk/lib/init-templates/app/csharp/add-project.hook.ts
Original file line number Diff line number Diff line change
@@ -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<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}`));
}
});
});
};
await shell(['dotnet', 'sln', slnPath, 'add', csprojPath], { errorMessage: 'Could not add project %name.PascalCased%.csproj to solution %name.PascalCased%.sln.' });
};
28 changes: 2 additions & 26 deletions packages/aws-cdk/lib/init-templates/app/fsharp/add-project.hook.ts
Original file line number Diff line number Diff line change
@@ -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<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}`));
}
});
});
await shell(['dotnet', 'sln', slnPath, 'add', fsprojPath], { errorMessage: 'Could not add project %name.PascalCased%.fsproj to solution %name.PascalCased%.sln.' });
};
Original file line number Diff line number Diff line change
@@ -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;
}

/**
Expand All @@ -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<string> {
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<string> {
const child = child_process.spawn(command[0], renderArguments(command.slice(1)), {
shell: true,
...options,
stdio: ['ignore', 'pipe', 'inherit'],
});
Expand All @@ -24,30 +22,26 @@ 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);
});

child.once('error', reject);

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 {
Expand All @@ -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);
}

/**
Expand All @@ -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}'`;
}

Expand All @@ -95,4 +89,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('');
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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}`));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}`));
}
Expand Down
37 changes: 33 additions & 4 deletions packages/aws-cdk/lib/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down Expand Up @@ -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(', ')})`);
Expand All @@ -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) {
Expand All @@ -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<string> {
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
Expand Down Expand Up @@ -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' })));
}
Expand Down
36 changes: 35 additions & 1 deletion packages/aws-cdk/test/init.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(/\<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);

Expand Down Expand Up @@ -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);
});

Expand All @@ -164,6 +185,19 @@ 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
*/
Expand Down

0 comments on commit 0960e90

Please sign in to comment.