Skip to content

Commit

Permalink
Revert "revert(init-templates): csharp and fsharp app init fails when…
Browse files Browse the repository at this point in the history 
… path contains space (#22112)"

This reverts commit 205e493.
  • Loading branch information
@rix0rrr
rix0rrr committed Sep 19, 2022
1 parent 205e493 commit 5c63751
Show file tree
Hide file tree
Showing 7 changed files with 99 additions and 137 deletions.
32 changes: 6 additions & 26 deletions packages/aws-cdk/lib/init-templates/app/csharp/add-project.hook.ts
View file
Original file line number Diff line number Diff line change
@@ -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<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}`));
}
});
});
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}`);
}
};
32 changes: 6 additions & 26 deletions packages/aws-cdk/lib/init-templates/app/fsharp/add-project.hook.ts
View file
Original file line number Diff line number Diff line change
@@ -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<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}`));
}
});
});
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}`);
}
};
32 changes: 6 additions & 26 deletions packages/aws-cdk/lib/init-templates/sample-app/csharp/add-project.hook.ts
View file
Original file line number Diff line number Diff line change
@@ -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<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}`));
}
});
});
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}`);
}
};
32 changes: 6 additions & 26 deletions packages/aws-cdk/lib/init-templates/sample-app/fsharp/add-project.hook.ts
View file
Original file line number Diff line number Diff line change
@@ -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<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}`));
}
});
});
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}`);
}
};
32 changes: 21 additions & 11 deletions packages/aws-cdk/lib/init.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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) {
Expand All @@ -127,28 +131,31 @@ export class InitTemplate {
},
};

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

Expand Down
40 changes: 19 additions & 21 deletions packages/aws-cdk/lib/os.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,18 @@ 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[], options: ShellOptions = {}): Promise<string> {
debug(`Executing ${chalk.blue(renderCommandLine(command))}`);
const child = child_process.spawn(command[0], command.slice(1), {
...options,
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,
stdio: ['ignore', 'pipe', 'inherit'],
});

Expand All @@ -24,30 +22,30 @@ 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(`${commandLine} exited with error code ${code}`));
}
});
});
}

function renderCommandLine(cmd: string[]) {
return renderArguments(cmd).join(' ');
}

/**
* 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 +56,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 +76,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 +93,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('');
}
}
36 changes: 35 additions & 1 deletion packages/aws-cdk/test/init.test.ts
View file
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 5c63751

Please sign in to comment.