Skip to content

Commit

Permalink
Refactor run() into two ops.
Browse files Browse the repository at this point in the history 
Removes the ergonomics, since they were making it difficult to
implement a correct API. They can be added later.
  • Loading branch information
@ry
ry committed Nov 12, 2018
1 parent 56406f8 commit 292b752
Show file tree
Hide file tree
Showing 6 changed files with 191 additions and 247 deletions.
194 changes: 40 additions & 154 deletions js/command.ts
View file
Original file line number Diff line number Diff line change
@@ -1,168 +1,40 @@
// Copyright 2018 the Deno authors. All rights reserved. MIT license.
import * as dispatch from "./dispatch";
import { DenoError, ErrorKind } from "./errors";
import * as flatbuffers from "./flatbuffers";
import * as msg from "gen/msg_generated";
import { assert, isObject, unreachable } from "./util";
import { assert, unreachable } from "./util";
import { close } from "./files";

export interface CommandOptions {
argv?: string[];
export interface RunOptions {
argv?: string[]; // TODO rename to args
cwd?: string | null;
throw?: boolean;
}

export interface Command extends CommandOptions {
argv: [string, ...string[]];
}

interface ExitCode {
success: boolean;
code: number;
signal: null;
}

interface ExitSignal {
success: false;
code: null;
signal: number; // TODO: Make this a string, e.g. 'SIGTERM'.
}

export type ExitStatus = ExitCode | ExitSignal;

// TODO: Move some of these small helper types to types.ts. Typescript won't
// allow it for some reason: "Import or export declaration in an ambient module
// declaration cannot reference module through relative module name."

// Exclude from T those types that are assignable to U.
export type Exclude<T, U> = T extends U ? never : T;
// An array or arraylike type with at least one element.
export type NonEmptyList<T> = [T, ...T[]];
// Remove the 'optional' modifier from all properties of T.
export type Required<T> = { [P in keyof T]-?: T[P] };

// Make a tuple type longer by prepending an element type.
// Unfortunately `type Prepend<S, T extends Array> = [S, ...T[]]` doesn't work.
// prettier-ignore
type Prepend<S, T extends Array<unknown>> =
((first: S, ...rest: T) => void) extends ((...all: infer R) => void)
? R
: never;

// This type helps us define the call signature for `run()`, which accepts a
// variable number of arguments followed by an optional options object.
// prettier-ignore
type ArgsAndOptions<
T extends Array<unknown>,
Item, Opts
> = T &
// GrowList starts with a 1-element tuple and grows it until its length is
// compatible with T's length. We'll never match a 0-length tuple; don't try.
GrowArgsAndOptionsList<Item, Opts, Exclude<T["length"], 0>> &
// We don't want to match any T with only an options object no other items.
{ 0: Item };

// This type grows a tuple that has as many elements as TLen. It iterates,
// prepending one item each round, until its length is a supertype of TLen.
// prettier-ignore
type GrowArgsAndOptionsList<
Item, Opts, TLen extends number,
// Initialize the list with the type of the last argument of our variadic
// function, which is where the options object might go.
List extends Array<unknown> = [Item | Opts]
> = {
continue: GrowArgsAndOptionsList<
Item, Opts,
// Drop the lengths that we've covered so far from TLen.
Exclude<TLen, List["length"]>,
// Insert another item at the beginning of the list.
Prepend<Item, List>
>;
return: List;
// Keep iterating until we've made a tuple for every length in TLen.
}[TLen extends List["length"] ? "return" : "continue"];

// Other command running functions should use the same options object,
// but they may use different defaults.
const defaultOptionsForRun = {
cwd: null,
throw: true
};

/** Run an external process.
*
* import { run } from "deno";
* run("curl", "http://deno.land/", "-o", "cool");
* run("ninja", "all", { cwd: "target/debug" });
* run({ argv: ["git", "status"], throw: false});
*/
export function run(command: Command): Promise<ExitStatus>;
export function run(...argv: NonEmptyList<string>): Promise<ExitStatus>;
export function run<T extends Array<unknown>>(
...argvAndOptions: ArgsAndOptions<T, string, CommandOptions>
): Promise<ExitStatus>;

export async function run(
...p: [Command] | NonEmptyList<string | CommandOptions>
): Promise<ExitStatus> {
const last = p[p.length - 1];

// If the last parameter is an object, it must be the options object;
// remove it from `p`. Otherwise assign it a default value.
// After this `p` only contains strings.
const partialOptions: CommandOptions = isObject(last) ? (p.pop(), last) : {};
export class Child {
constructor(readonly rid: number, readonly pid: number) {}

// Merge the argv from the options object with those specified separately.
// The options object is owned by the caller, so we cant't mutate it.
const argv = p as string[];
if (partialOptions.argv) {
argv.push(...partialOptions.argv);
async status(): Promise<ExitStatus> {
return await runStatus(this.rid);
}
if (!("0" in argv) /* Funky syntax is needed to satisfy typescript. */) {
throw new TypeError("Run: missing argv.");
}

// Combine defaults, options and merged argv.
const command: Required<Command> = {
...defaultOptionsForRun,
...partialOptions,
argv
};

// Start the process and wait for it it exit.
const status: ExitStatus = res(await dispatch.sendAsync(...req(command)));

// Throw if the `throw` option is enabled and the process didn't exit cleanly.
if (!status.success && command.throw) {
// Wrap arguments that contain spaces and those that are empty in unusual
// quotation marks, so the separation between them is clear, but without
// suggesting that argv was actually quoted in a certain way.
const pretty = command.argv
.map(arg => (/[\s'"]|^$/.test(arg) ? `\u2039${arg}\u203A` : arg))
.join(" ");
const description = status.signal
? `killed with signal ${status.signal}`
: `exit code ${status.code}`;
const error = new DenoError(
ErrorKind.CommandFailed,
`Command ${pretty} failed: ${description}`
);
// Attach command info and exit status information to the error object.
// TODO: use class CommandError so this works cleanly with typescript.
Object.assign(error, { command, status });
throw error;
close(): void {
close(this.rid);
}
}

return status;
export interface ExitStatus {
success: boolean;
code?: number;
signal?: number; // TODO: Make this a string, e.g. 'SIGTERM'.
}

function req({
argv,
cwd
}: Required<Command>): [flatbuffers.Builder, msg.Any, flatbuffers.Offset] {
export function run(opt: RunOptions): Child {
const { argv, cwd } = opt;

const builder = flatbuffers.createBuilder();
const argvOffset = msg.Run.createArgvVector(
builder,
argv.map(a => builder.createString(a))
argv!.map(a => builder.createString(a))
);
const cwdOffset = cwd == null ? -1 : builder.createString(cwd);
msg.Run.startRun(builder);
Expand All @@ -171,22 +43,36 @@ function req({
msg.Run.addCwd(builder, cwdOffset);
}
const inner = msg.Run.endRun(builder);
return [builder, msg.Any.Run, inner];
}

function res(baseRes: null | msg.Base): ExitStatus {
const baseRes = dispatch.sendSync(builder, msg.Any.Run, inner);
assert(baseRes != null);
assert(msg.Any.RunRes === baseRes!.innerType());
const res = new msg.RunRes();
assert(baseRes!.inner(res) != null);

switch (res.status()) {
return new Child(res.rid(), res.pid());
}

export async function runStatus(rid: number): Promise<ExitStatus> {
const builder = flatbuffers.createBuilder();
msg.RunStatus.startRunStatus(builder);
msg.RunStatus.addRid(builder, rid);
const inner = msg.RunStatus.endRunStatus(builder);

const baseRes = await dispatch.sendAsync(builder, msg.Any.RunStatus, inner);
assert(baseRes != null);
assert(msg.Any.RunStatusRes === baseRes!.innerType());
const res = new msg.RunStatusRes();
assert(baseRes!.inner(res) != null);

let status = res.status();

switch (status) {
case msg.ExitStatus.ExitedWithCode:
const code = res.exitCode();
return { code, signal: null, success: code === 0 };
return { code, success: code === 0 };
case msg.ExitStatus.ExitedWithSignal:
const signal = res.exitSignal();
return { code: null, signal, success: false };
return { signal, success: false };
default:
return unreachable();
}
Expand Down
109 changes: 29 additions & 80 deletions js/command_test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,62 +1,48 @@
// Copyright 2018 the Deno authors. All rights reserved. MIT license.
import { test, testPerm, assert, assertEqual } from "./test_util.ts";
import {
run,
Command,
CommandOptions,
DenoError,
ErrorKind,
ExitStatus
} from "deno";
import { run, runStatus, DenoError, ErrorKind, ExitStatus } from "deno";
import * as deno from "deno";

testPerm({ write: true }, async function runSuccess() {
const status = await run("python", "-c", "print('hello world')");
const child = run({
argv: ["python", "-c", "print('hello world')"]
});
const status = await child.status();
assertEqual(status.success, true);
assertEqual(status.code, 0);
assertEqual(status.signal, null);
assertEqual(status.signal, undefined);
child.close();
});

testPerm({ write: true }, async function runCommandFailedWithCode() {
let error: DenoError<ErrorKind.CommandFailed> & {
command: Command;
status: ExitStatus;
};
try {
await run("python", "-c", "import sys;sys.exit(41 + 1)");
} catch (e) {
error = e;
}
assert(error !== undefined);
console.log(error.stack);
assert(error instanceof DenoError);
assertEqual(error.kind, ErrorKind.CommandFailed);
assertEqual(error.status.success, false);
assertEqual(error.status.code, 42);
assertEqual(error.status.signal, null);
assertEqual(error.command.argv[0], "python");
assert(/python.*import.*41.*42/.test(error.message));
let child = run({
argv: ["python", "-c", "import sys;sys.exit(41 + 1)"]
});
let status = await child.status();
assertEqual(status.success, false);
assertEqual(status.code, 42);
assertEqual(status.signal, undefined);
child.close();
});

testPerm({ write: true }, async function runCommandFailedWithSignal() {
if (deno.platform.os === "win") {
return; // No signals on windows.
}
const status = await run(
"python",
"-c",
"import os;os.kill(os.getpid(), 9)",
{ throw: false }
);
const child = run({
argv: ["python", "-c", "import os;os.kill(os.getpid(), 9)"]
});
const status = await child.status();
assertEqual(status.success, false);
assertEqual(status.code, null);
assertEqual(status.code, undefined);
assertEqual(status.signal, 9);
child.close();
});

testPerm({ write: true }, async function runNotFound() {
let error;
try {
await run({ argv: ["this file hopefully doesn't exist"] });
run({ argv: ["this file hopefully doesn't exist"] });
} catch (e) {
error = e;
}
Expand Down Expand Up @@ -88,56 +74,19 @@ while True:
`;

deno.writeFileSync(`${cwd}/${pyProgramFile}.py`, enc.encode(pyProgram));
const promise = run("python", `${pyProgramFile}.py`, { cwd, throw: false });
const child = run({
cwd,
argv: ["python", `${pyProgramFile}.py`]
});

// Write the expected exit code *after* starting python.
// This is how we verify that `run()` is actually asynchronous.
const code = 84;
deno.writeFileSync(`${cwd}/${exitCodeFile}`, enc.encode(`${code}`));

const status = await promise;
const status = await child.status();
assertEqual(status.success, false);
assertEqual(status.code, code);
assertEqual(status.signal, null);
});

declare function fakeGetType<T>(): T;

test(function runTypeSignature() {
// This test doesn't actually run anything; it just gets type checked.
// This conditional serves to confuse typescript and not make it think that it
// is dead code.
if (typeof fakeGetType === "undefined") {
return;
}

// The following should be accepted by the type checker.
run({ argv: ["hello world"] });
run("x");
run("x", {});
run("x", "x");
run("x", "x", {});
run("x", "x", "x");
run("x", "x", "x", {});
run(...fakeGetType<["a", "b"] | ["a", "b", {}]>());
run(...fakeGetType<["a", {}] | ["a", "b", {}]>());
run(...fakeGetType<[string, ...string[]]>());

// The following should be rejected by the type checker.
// TODO: is there a way to automatically test this?
/*
run();
run({});
run({ argv: [] });
run({ argv: "waddup" });
run("a", "b", {}, "d");
run("a", "b", 11, "d");
run("a", "b", "c", undefined);
run("a", "b", "c", {}, undefined);
run(undefined, "a", "b", "c");
run(...fakeGetType<["a", {}, "c"] | ["a", "b", {}]>());
run(...fakeGetType<string[]>());
run(...fakeGetType<CommandOptions[]>());
run(...fakeGetType<[string, string] | []>());
*/
assertEqual(status.signal, undefined);
child.close();
});
2 changes: 1 addition & 1 deletion js/deno.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export { FileInfo } from "./file_info";
export { connect, dial, listen, Listener, Conn } from "./net";
export { metrics } from "./metrics";
export { resources } from "./resources";
export { run, Command, CommandOptions, ExitStatus } from "./command";
export { run, runStatus, Child, RunOptions, ExitStatus } from "./command";
export const args: string[] = [];

// Provide the compiler API in an obfuscated way
Expand Down
Loading

0 comments on commit 292b752

Please sign in to comment.