Fix not using system shell on posix (#9449)

* Use system shell + add to bunfig

* Update CMakeLists.txt

* Fix tests + flags

* Use execPath

* windows

* Propagate exit code

* Add test for default shell in use

* Update bun-run-bunfig.test.ts

* Update bun-run-bunfig.test.ts

---------

Co-authored-by: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com>
This commit is contained in:
Jarred Sumner
2024-03-15 23:00:53 -07:00
committed by GitHub
parent b23eb60277
commit e89c8d2eaa
11 changed files with 324 additions and 39 deletions

View File

@@ -2,7 +2,7 @@ cmake_minimum_required(VERSION 3.22)
cmake_policy(SET CMP0091 NEW)
cmake_policy(SET CMP0067 NEW)
set(Bun_VERSION "1.0.31")
set(Bun_VERSION "1.0.32")
set(WEBKIT_TAG 089023cc9078b3aa173869fd6685f3e7bed2a994)
if(APPLE AND DEFINED ENV{CI})

View File

@@ -426,4 +426,56 @@ editor = "code"
# - "nvim", "neovim"
# - "vim","vi"
# - "emacs"
``` -->
```
-->
## `bun run`
The `bun run` command can be configured under the `[run]` section. These apply to the `bun run` command and the `bun` command when running a file or executable or script.
### `run.shell`: `"system" | "bun"`
The shell to use when running package.json scripts via `bun run` or `bun`. On Windows, this defaults to `"bun"` and on other platforms it defaults to `"system"`.
Always use the system shell instead of Bun's shell (default behavior unless Windows):
```toml
[run]
shell = "system"
```
Always use Bun's shell instead of the system shell:
```toml
[run]
shell = "bun"
```
### `run.bun`: `boolean`
When `true`, this adds a `node` symlink to `$PATH` which points to the `bun` binary for all scripts or executables invoked by `bun run` or `bun`.
By default, this is enabled if `node` is not already in your `$PATH`.
```toml
[run]
bun = true
```
You can test this by running:
```sh
$ bun --bun which node # /path/to/bun
$ bun which node # /path/to/node
```
This option is equivalent to prefixing all `bun run` commands with `--bun`:
```sh
bun --bun run dev
bun --bun dev
bun run --bun dev
```
If set to `false`, this will disable the `node` symlink.

View File

@@ -4483,7 +4483,7 @@ declare module "bun" {
*/
cmd: string[];
onExit: never;
onExit?: never;
},
): SpawnOptions.OptionsToSyncSubprocess<Opts>;

View File

@@ -123,7 +123,7 @@ pub const Run = struct {
vm.global.vm().holdAPILock(&run, callback);
}
fn bootBunShell(ctx: *const Command.Context, entry_path: []const u8) !void {
fn bootBunShell(ctx: *const Command.Context, entry_path: []const u8) !bun.shell.ExitCode {
@setCold(true);
// this is a hack: make dummy bundler so we can use its `.runEnvLoader()` function to populate environment variables probably should split out the functionality
@@ -136,8 +136,7 @@ pub const Run = struct {
try bundle.runEnvLoader();
const mini = JSC.MiniEventLoop.initGlobal(bundle.env);
mini.top_level_dir = ctx.args.absolute_working_dir orelse "";
try bun.shell.Interpreter.initAndRunFromFile(mini, entry_path);
return;
return try bun.shell.Interpreter.initAndRunFromFile(mini, entry_path);
}
pub fn boot(ctx_: Command.Context, entry_path: string) !void {
@@ -146,8 +145,8 @@ pub const Run = struct {
bun.JSC.initialize();
if (strings.endsWithComptime(entry_path, ".bun.sh")) {
try bootBunShell(&ctx, entry_path);
Global.exit(0);
const exit_code = try bootBunShell(&ctx, entry_path);
Global.exitWide(exit_code);
return;
}

View File

@@ -506,6 +506,38 @@ pub const Bunfig = struct {
}
}
}
if (json.get("run")) |run_expr| {
if (run_expr.get("silent")) |silent| {
if (silent.asBool()) |value| {
this.ctx.debug.silent = value;
} else {
try this.addError(silent.loc, "Expected boolean");
}
}
if (run_expr.get("shell")) |shell| {
if (shell.asString(allocator)) |value| {
if (strings.eqlComptime(value, "bun")) {
this.ctx.debug.use_system_shell = false;
} else if (strings.eqlComptime(value, "system")) {
this.ctx.debug.use_system_shell = true;
} else {
try this.addError(shell.loc, "Invalid shell, only 'bun' and 'system' are supported");
}
} else {
try this.addError(shell.loc, "Expected string");
}
}
if (run_expr.get("bun")) |bun_flag| {
if (bun_flag.asBool()) |value| {
this.ctx.debug.run_in_bun = value;
} else {
try this.addError(bun_flag.loc, "Expected boolean");
}
}
}
}
if (json.get("bundle")) |_bun| {

View File

@@ -182,23 +182,22 @@ pub const Arguments = struct {
clap.parseParam("--conditions <STR>... Pass custom conditions to resolve") catch unreachable,
};
const auto_or_run_params = [_]ParamType{
clap.parseParam("-b, --bun Force a script or package to use Bun's runtime instead of Node.js (via symlinking node)") catch unreachable,
clap.parseParam("--shell <STR> Control the shell used for package.json scripts. Supports either 'bun' or 'system'") catch unreachable,
};
const auto_only_params = [_]ParamType{
// clap.parseParam("--all") catch unreachable,
clap.parseParam("-b, --bun Force a script or package to use Bun's runtime instead of Node.js (via symlinking node)") catch unreachable,
clap.parseParam("--silent Don't print the script command") catch unreachable,
clap.parseParam("-v, --version Print version and exit") catch unreachable,
clap.parseParam("--revision Print version with revision and exit") catch unreachable,
};
} ++ auto_or_run_params;
const auto_params = auto_only_params ++ runtime_params_ ++ transpiler_params_ ++ base_params_;
const run_only_params = [_]ParamType{
clap.parseParam("--silent Don't print the script command") catch unreachable,
clap.parseParam("-b, --bun Force a script or package to use Bun's runtime instead of Node.js (via symlinking node)") catch unreachable,
} ++ if (Environment.isWindows) [_]ParamType{
clap.parseParam("--system-shell Use cmd.exe to interpret package.json scripts") catch unreachable,
} else .{
clap.parseParam("--bun-shell Use Bun Shell to interpret package.json scripts") catch unreachable,
};
} ++ auto_or_run_params;
pub const run_params = run_only_params ++ runtime_params_ ++ transpiler_params_ ++ base_params_;
const bunx_commands = [_]ParamType{
@@ -663,7 +662,8 @@ pub const Arguments = struct {
else => invalidTarget(&diag, _target),
};
ctx.debug.run_in_bun = opts.target.? == .bun;
if (opts.target.? == .bun)
ctx.debug.run_in_bun = opts.target.? == .bun;
}
if (args.flag("--watch")) {
@@ -784,7 +784,10 @@ pub const Arguments = struct {
const react_fast_refresh = true;
if (cmd == .AutoCommand or cmd == .RunCommand) {
ctx.debug.silent = args.flag("--silent");
// "run.silent" in bunfig.toml
if (args.flag("--silent")) {
ctx.debug.silent = true;
}
if (opts.define) |define| {
if (define.keys.len > 0)
@@ -793,7 +796,10 @@ pub const Arguments = struct {
}
if (cmd == .RunCommand or cmd == .AutoCommand or cmd == .BunxCommand) {
ctx.debug.run_in_bun = args.flag("--bun") or ctx.debug.run_in_bun;
// "run.bun" in bunfig.toml
if (args.flag("--bun")) {
ctx.debug.run_in_bun = true;
}
}
opts.resolve = Api.ResolveMode.lazy;
@@ -858,11 +864,17 @@ pub const Arguments = struct {
if (output_file != null)
ctx.debug.output_file = output_file.?;
if (cmd == .RunCommand) {
ctx.debug.use_system_shell = if (Environment.isWindows)
args.flag("--system-shell")
else
!args.flag("--bun-shell");
if (cmd == .RunCommand or cmd == .AutoCommand) {
if (args.option("--shell")) |shell| {
if (strings.eqlComptime(shell, "bun")) {
ctx.debug.use_system_shell = false;
} else if (strings.eqlComptime(shell, "system")) {
ctx.debug.use_system_shell = true;
} else {
Output.errGeneric("Expected --shell to be one of 'bun' or 'system'. Received: \"{s}\"", .{shell});
Global.exit(1);
}
}
}
return opts;
@@ -1062,7 +1074,7 @@ pub const Command = struct {
run_in_bun: bool = false,
loaded_bunfig: bool = false,
/// Disables using bun.shell.Interpreter for `bun run`, instead spawning cmd.exe
use_system_shell: bool = false,
use_system_shell: bool = !bun.Environment.isWindows,
// technical debt
macros: MacroOptions = MacroOptions.unspecified,

View File

@@ -265,7 +265,7 @@ pub const RunCommand = struct {
const log = Output.scoped(.RUN, false);
pub fn runPackageScriptForeground(
fn runPackageScriptForeground(
allocator: std.mem.Allocator,
original_script: string,
name: string,
@@ -316,15 +316,23 @@ pub const RunCommand = struct {
}
const mini = bun.JSC.MiniEventLoop.initGlobal(env);
bun.shell.Interpreter.initAndRunFromSource(mini, name, combined_script) catch |err| {
const code = bun.shell.Interpreter.initAndRunFromSource(mini, name, combined_script) catch |err| {
if (!silent) {
Output.prettyErrorln("<r><red>error<r>: Failed to run script <b>{s}<r> due to error <b>{s}<r>", .{ name, @errorName(err) });
}
Output.flush();
Global.exit(1);
};
if (code > 0) {
if (code != 2 and !silent) {
Output.prettyErrorln("<r><red>error<r><d>:<r> script <b>\"{s}\"<r> exited with code {d}<r>", .{ name, code });
Output.flush();
}
Global.exitWide(code);
}
return true;
}
@@ -1155,12 +1163,11 @@ pub const RunCommand = struct {
;
Output.pretty(intro_text ++ "\n\n", .{});
Output.flush();
Output.pretty("<b>Flags:<r>", .{});
Output.flush();
clap.simpleHelp(&Arguments.run_params);
Output.pretty("\n\n" ++ examples_text, .{});
Output.flush();
if (package_json) |pkg| {
if (pkg.scripts) |scripts| {
@@ -1184,16 +1191,15 @@ pub const RunCommand = struct {
// Output.prettyln("\n<d>{d} scripts<r>", .{scripts.count()});
Output.prettyln("\n", .{});
Output.flush();
} else {
Output.prettyln("\n<r><yellow>No \"scripts\" found in package.json.<r>\n", .{});
Output.flush();
}
} else {
Output.prettyln("\n<r><yellow>No \"scripts\" found in package.json.<r>\n", .{});
Output.flush();
}
}
Output.flush();
}
pub fn exec(

View File

@@ -73,7 +73,7 @@ pub fn assert(cond: bool, comptime msg: []const u8) void {
}
}
const ExitCode = if (bun.Environment.isWindows) u16 else u16;
pub const ExitCode = u16;
pub const StateKind = enum(u8) {
script,
@@ -612,6 +612,7 @@ pub const Interpreter = struct {
started: std.atomic.Value(bool) = std.atomic.Value(bool).init(false),
done: ?*bool = null,
exit_code: ?*ExitCode = null,
const InterpreterChildPtr = StatePtrUnion(.{
Script,
@@ -1156,7 +1157,7 @@ pub const Interpreter = struct {
return .{ .result = interpreter };
}
pub fn initAndRunFromFile(mini: *JSC.MiniEventLoop, path: []const u8) !void {
pub fn initAndRunFromFile(mini: *JSC.MiniEventLoop, path: []const u8) !bun.shell.ExitCode {
var arena = bun.ArenaAllocator.init(bun.default_allocator);
const src = src: {
var file = try std.fs.cwd().openFile(path, .{});
@@ -1196,10 +1197,12 @@ pub const Interpreter = struct {
var interp = switch (ThisInterpreter.init(.{ .mini = mini }, bun.default_allocator, &arena, script_heap, jsobjs)) {
.err => |*e| {
throwShellErr(e, .{ .mini = mini });
return;
return 1;
},
.result => |i| i,
};
var exit_code: ExitCode = 1;
const IsDone = struct {
done: bool = false,
@@ -1210,11 +1213,13 @@ pub const Interpreter = struct {
};
var is_done: IsDone = .{};
interp.done = &is_done.done;
interp.exit_code = &exit_code;
try interp.run();
mini.tick(&is_done, @as(fn (*anyopaque) bool, IsDone.isDone));
return exit_code;
}
pub fn initAndRunFromSource(mini: *JSC.MiniEventLoop, path_for_errors: []const u8, src: []const u8) !void {
pub fn initAndRunFromSource(mini: *JSC.MiniEventLoop, path_for_errors: []const u8, src: []const u8) !ExitCode {
var arena = bun.ArenaAllocator.init(bun.default_allocator);
defer arena.deinit();
@@ -1242,7 +1247,7 @@ pub const Interpreter = struct {
var interp = switch (ThisInterpreter.init(.{ .mini = mini }, bun.default_allocator, &arena, script_heap, jsobjs)) {
.err => |*e| {
throwShellErr(e, .{ .mini = mini });
return;
return 1;
},
.result => |i| i,
};
@@ -1255,10 +1260,13 @@ pub const Interpreter = struct {
}
};
var is_done: IsDone = .{};
var exit_code: ExitCode = 1;
interp.done = &is_done.done;
interp.exit_code = &exit_code;
try interp.run();
mini.tick(&is_done, @as(fn (*anyopaque) bool, IsDone.isDone));
interp.deinitEverything();
return exit_code;
}
pub fn run(this: *ThisInterpreter) !void {
@@ -1305,11 +1313,13 @@ pub const Interpreter = struct {
fn finish(this: *ThisInterpreter, exit_code: ExitCode) void {
log("finish", .{});
defer decrPendingActivityFlag(&this.has_pending_activity);
if (this.event_loop == .js) {
defer this.deinitAfterJSRun();
_ = this.resolve.call(&.{JSValue.jsNumberFromU16(exit_code)});
} else {
this.done.?.* = true;
this.exit_code.?.* = exit_code;
}
}

View File

@@ -3192,3 +3192,4 @@ pub fn needsEscapeUtf8AsciiLatin1Slow(str: []const u8) bool {
}
return false;
}
pub const ExitCode = eval.ExitCode;

View File

@@ -0,0 +1,140 @@
import { describe, expect, test } from "bun:test";
import { realpathSync, chmodSync } from "fs";
import { bunEnv, bunExe, isWindows, tempDirWithFiles, toTOMLString } from "harness";
import { join } from "path";
describe.each(["bun run", "bun"])(`%s`, cmd => {
const runCmd = cmd === "bun" ? ["-c=bunfig.toml", "run"] : ["-c=bunfig.toml"];
const node = Bun.which("node")!;
const execPath = process.execPath;
describe.each(["--bun", "without --bun"])("%s", cmd2 => {
test("which node", async () => {
const bun = cmd2 === "--bun";
const bunFlag = bun ? ["--bun"] : [];
const bunfig = toTOMLString({
run: {
bun,
},
});
const which = "which";
const cwd = tempDirWithFiles("run.where.node." + cmd2, {
"bunfig.toml": bunfig,
"package.json": JSON.stringify(
{
scripts: {
"where-node": `${which} node`,
},
},
null,
2,
),
});
const result = Bun.spawnSync({
cmd: [bunExe(), "--silent", ...bunFlag, ...runCmd, "where-node"],
env: bunEnv,
stderr: "inherit",
stdout: "pipe",
stdin: "ignore",
cwd,
});
const nodeBin = result.stdout.toString().trim();
if (bun) {
if (isWindows) {
expect(realpathSync(nodeBin)).toContain("\\bun-node-");
} else {
expect(realpathSync(nodeBin)).toBe(realpathSync(execPath));
}
} else {
expect(realpathSync(nodeBin)).toBe(realpathSync(node));
}
expect(result.success).toBeTrue();
});
});
describe.each(["bun", "system", "default"])(`run.shell = "%s"`, shellStr => {
const shell = shellStr === "default" ? (isWindows ? "bun" : "system") : shellStr;
const command_not_found =
isWindows && shell === "system" ? "is not recognized as an internal or external command" : "command not found";
test.each(["true", "false"])('run.silent = "%s"', silentStr => {
const silent = silentStr === "true";
const bunfig = toTOMLString({
run: {
shell: shellStr === "default" ? undefined : shell,
silent,
},
});
const cwd = tempDirWithFiles(Bun.hash(bunfig).toString(36), {
"bunfig.toml": bunfig,
"package.json": JSON.stringify(
{
scripts: {
startScript: "echo 1",
},
},
null,
2,
),
});
const result = Bun.spawnSync({
cmd: [bunExe(), ...runCmd, "startScript"],
env: bunEnv,
stderr: "pipe",
stdout: "ignore",
stdin: "ignore",
cwd,
});
if (silent) {
expect(result.stderr.toString().trim()).toBe("");
} else {
expect(result.stderr.toString().trim()).toContain("$ echo 1");
}
expect(result.success).toBeTrue();
});
test("command not found", async () => {
const bunfig = toTOMLString({
run: {
shell,
},
});
const cwd = tempDirWithFiles("run.shell.system-" + Bun.hash(bunfig).toString(32), {
"bunfig.toml": bunfig,
"package.json": JSON.stringify(
{
scripts: {
start: "this-should-start-with-bun-in-the-error-message",
},
},
null,
2,
),
});
const result = Bun.spawnSync({
cmd: [bunExe(), "--silent", ...runCmd, "start"],
env: bunEnv,
stderr: "pipe",
stdout: "inherit",
stdin: "ignore",
cwd,
});
const err = result.stderr.toString().trim();
if (shell === "bun") {
expect(err).toStartWith("bun: ");
} else {
expect(err).not.toStartWith("bun: ");
}
expect(err).toContain(command_not_found);
expect(err).toContain("this-should-start-with-bun-in-the-error-message");
expect(result.success).toBeFalse();
});
});
});

View File

@@ -524,3 +524,36 @@ export function fillRepeating(dstBuffer: NodeJS.TypedArray, start: number, end:
sLen <<= 1; // double length for next segment
}
}
function makeFlatPropertyMap(opts: object) {
// return all properties of opts as paths for nested objects with dot notation
// like { a: { b: 1 } } => { "a.b": 1 }
// combining names of nested objects with dot notation
// infinitely deep
const ret: any = {};
function recurse(obj: object, path = "") {
for (const [key, value] of Object.entries(obj)) {
if (value === undefined) continue;
if (value && typeof value === "object") {
recurse(value, path ? `${path}.${key}` : key);
} else {
ret[path ? `${path}.${key}` : key] = value;
}
}
}
recurse(opts);
return ret;
}
export function toTOMLString(opts: object) {
// return a TOML string of the given options
const props = makeFlatPropertyMap(opts);
let ret = "";
for (const [key, value] of Object.entries(props)) {
if (value === undefined) continue;
ret += `${key} = ${JSON.stringify(value)}` + "\n";
}
return ret;
}