Use execFileSync in typing installer

This commit is contained in:
Sheetal Nandi 2019-02-07 13:35:07 -08:00
parent f944ed6d72
commit bc386c11fd
3 changed files with 26 additions and 19 deletions

View File

@ -1684,9 +1684,9 @@ namespace ts.projectSystem {
TI.getNpmCommandForInstallation(npmPath, tsVersion, packageNames, packageNames.length - Math.ceil(packageNames.length / 2)).command
];
it("works when the command is too long to install all packages at once", () => {
const commands: string[] = [];
const hasError = TI.installNpmPackages(npmPath, tsVersion, packageNames, command => {
commands.push(command);
const commands: [string, string[]][] = [];
const hasError = TI.installNpmPackages(npmPath, tsVersion, packageNames, (file, args) => {
commands.push([file, args]);
return false;
});
assert.isFalse(hasError);
@ -1694,9 +1694,9 @@ namespace ts.projectSystem {
});
it("installs remaining packages when one of the partial command fails", () => {
const commands: string[] = [];
const hasError = TI.installNpmPackages(npmPath, tsVersion, packageNames, command => {
commands.push(command);
const commands: [string, string[]][] = [];
const hasError = TI.installNpmPackages(npmPath, tsVersion, packageNames, (file, args) => {
commands.push([file, args]);
return commands.length === 1;
});
assert.isTrue(hasError);

View File

@ -70,10 +70,10 @@ namespace ts.server.typingsInstaller {
cwd: string;
encoding: "utf-8";
}
type ExecSync = (command: string, options: ExecSyncOptions) => string;
type ExecFileSync = (file: string, args: string[], options: ExecSyncOptions) => string;
export class NodeTypingsInstaller extends TypingsInstaller {
private readonly nodeExecSync: ExecSync;
private readonly nodeExecFileSync: ExecFileSync;
private readonly npmPath: string;
readonly typesRegistry: Map<MapLike<string>>;
@ -97,7 +97,7 @@ namespace ts.server.typingsInstaller {
this.log.writeLine(`Process id: ${process.pid}`);
this.log.writeLine(`NPM location: ${this.npmPath} (explicit '${Arguments.NpmLocation}' ${npmLocation === undefined ? "not " : ""} provided)`);
}
({ execSync: this.nodeExecSync } = require("child_process"));
({ execFileSync: this.nodeExecFileSync } = require("child_process"));
this.ensurePackageDirectoryExists(globalTypingsCacheLocation);
@ -105,7 +105,7 @@ namespace ts.server.typingsInstaller {
if (this.log.isEnabled()) {
this.log.writeLine(`Updating ${typesRegistryPackageName} npm package...`);
}
this.execSyncAndLog(`${this.npmPath} install --ignore-scripts ${typesRegistryPackageName}@${this.latestDistTag}`, { cwd: globalTypingsCacheLocation });
this.execFileSyncAndLog(this.npmPath, ["install", "--ignore-scripts", `${typesRegistryPackageName}@${this.latestDistTag}`], { cwd: globalTypingsCacheLocation });
if (this.log.isEnabled()) {
this.log.writeLine(`Updated ${typesRegistryPackageName} npm package`);
}
@ -189,7 +189,7 @@ namespace ts.server.typingsInstaller {
this.log.writeLine(`#${requestId} with arguments'${JSON.stringify(packageNames)}'.`);
}
const start = Date.now();
const hasError = installNpmPackages(this.npmPath, version, packageNames, command => this.execSyncAndLog(command, { cwd }));
const hasError = installNpmPackages(this.npmPath, version, packageNames, (file, args) => this.execFileSyncAndLog(file, args, { cwd }));
if (this.log.isEnabled()) {
this.log.writeLine(`npm install #${requestId} took: ${Date.now() - start} ms`);
}
@ -197,12 +197,12 @@ namespace ts.server.typingsInstaller {
}
/** Returns 'true' in case of error. */
private execSyncAndLog(command: string, options: Pick<ExecSyncOptions, "cwd">): boolean {
private execFileSyncAndLog(file: string, args: string[], options: Pick<ExecSyncOptions, "cwd">): boolean {
if (this.log.isEnabled()) {
this.log.writeLine(`Exec: ${command}`);
this.log.writeLine(`Exec: ${file} ${args.join(" ")}`);
}
try {
const stdout = this.nodeExecSync(command, { ...options, encoding: "utf-8" });
const stdout = this.nodeExecFileSync(file, args, { ...options, encoding: "utf-8" });
if (this.log.isEnabled()) {
this.log.writeLine(` Succeeded. stdout:${indent(sys.newLine, stdout)}`);
}

View File

@ -31,28 +31,35 @@ namespace ts.server.typingsInstaller {
}
/*@internal*/
export function installNpmPackages(npmPath: string, tsVersion: string, packageNames: string[], install: (command: string) => boolean) {
export function installNpmPackages(npmPath: string, tsVersion: string, packageNames: string[], install: (file: string, args: string[]) => boolean) {
let hasError = false;
for (let remaining = packageNames.length; remaining > 0;) {
const result = getNpmCommandForInstallation(npmPath, tsVersion, packageNames, remaining);
remaining = result.remaining;
hasError = install(result.command) || hasError;
hasError = install(result.command[0], result.command[1]) || hasError;
}
return hasError;
}
function getUserAgent(tsVersion: string) {
return `--user-agent="typesInstaller/${tsVersion}"`;
}
const npmInstall = "install", ignoreScripts = "--ignore-scripts", saveDev = "--save-dev";
const commandBaseLength = npmInstall.length + ignoreScripts.length + saveDev.length + getUserAgent("").length + 5;
/*@internal*/
export function getNpmCommandForInstallation(npmPath: string, tsVersion: string, packageNames: string[], remaining: number) {
const sliceStart = packageNames.length - remaining;
let command: string, toSlice = remaining;
let packages: string[], toSlice = remaining;
while (true) {
command = `${npmPath} install --ignore-scripts ${(toSlice === packageNames.length ? packageNames : packageNames.slice(sliceStart, sliceStart + toSlice)).join(" ")} --save-dev --user-agent="typesInstaller/${tsVersion}"`;
if (command.length < 8000) {
packages = toSlice === packageNames.length ? packageNames : packageNames.slice(sliceStart, sliceStart + toSlice);
const commandLength = npmPath.length + commandBaseLength + packages.join(" ").length + tsVersion.length;
if (commandLength < 8000) {
break;
}
toSlice = toSlice - Math.floor(toSlice / 2);
}
const command: [string, string[]] = [npmPath, [npmInstall, ignoreScripts, ...packages, saveDev, getUserAgent(tsVersion)]];
return { command, remaining: remaining - toSlice };
}