Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

revert(init-templates): csharp and fsharp app init fails when path contains space #22112

Merged
merged 2 commits into from
Sep 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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}`));
}
});
});
};
Original file line number Diff line number Diff line change
@@ -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}`));
}
});
});
};
Original file line number Diff line number Diff line change
@@ -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}`));
}
});
});
};
Original file line number Diff line number Diff line change
@@ -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}`));
}
});
});
};
32 changes: 11 additions & 21 deletions packages/aws-cdk/lib/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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) {
Expand All @@ -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);
Expand Down Expand Up @@ -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();
}

Expand Down
40 changes: 21 additions & 19 deletions packages/aws-cdk/lib/os.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
});

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

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

Expand All @@ -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('');
}
}
36 changes: 1 addition & 35 deletions packages/aws-cdk/test/init.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

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

Expand All @@ -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
*/
Expand Down