diff --git a/src/Command.zig b/src/Command.zig index a810b16ce..e17c1b370 100644 --- a/src/Command.zig +++ b/src/Command.zig @@ -33,14 +33,17 @@ const EnvMap = std.process.EnvMap; const PreExecFn = fn (*Command) void; -/// Path to the command to run. This must be an absolute path. This -/// library does not do PATH lookup. -path: []const u8, +/// Path to the command to run. This doesn't have to be an absolute path, +/// because use exec functions that search the PATH, if necessary. +/// +/// This field is null-terminated to avoid a copy for the sake of +/// adding a null terminator since POSIX systems are so common. +path: [:0]const u8, /// Command-line arguments. It is the responsibility of the caller to set /// args[0] to the command. If args is empty then args[0] will automatically /// be set to equal path. -args: []const []const u8, +args: []const [:0]const u8, /// Environment variables for the child process. If this is null, inherits /// the environment variables from this process. These are the exact @@ -129,9 +132,8 @@ pub fn start(self: *Command, alloc: Allocator) !void { fn startPosix(self: *Command, arena: Allocator) !void { // Null-terminate all our arguments - const pathZ = try arena.dupeZ(u8, self.path); - const argsZ = try arena.allocSentinel(?[*:0]u8, self.args.len, null); - for (self.args, 0..) |arg, i| argsZ[i] = (try arena.dupeZ(u8, arg)).ptr; + const argsZ = try arena.allocSentinel(?[*:0]const u8, self.args.len, null); + for (self.args, 0..) |arg, i| argsZ[i] = arg.ptr; // Determine our env vars const envp = if (self.env) |env_map| @@ -184,7 +186,9 @@ fn startPosix(self: *Command, arena: Allocator) !void { if (self.pre_exec) |f| f(self); // Finally, replace our process. - _ = posix.execveZ(pathZ, argsZ, envp) catch null; + // Note: we must use the "p"-variant of exec here because we + // do not guarantee our command is looked up already in the path. + _ = posix.execvpeZ(self.path, argsZ, envp) catch null; // If we are executing this code, the exec failed. In that scenario, // we return a very specific error that can be detected to determine diff --git a/src/Surface.zig b/src/Surface.zig index 46fa476f7..89031a1b5 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -518,7 +518,7 @@ pub fn init( }; // The command we're going to execute - const command: ?[]const u8 = if (app.first) + const command: ?configpkg.Command = if (app.first) config.@"initial-command" orelse config.command else config.command; @@ -650,21 +650,19 @@ pub fn init( // title to the command being executed. This allows window managers // to set custom styling based on the command being executed. const v = command orelse break :xdg; - if (v.len > 0) { - const title = alloc.dupeZ(u8, v) catch |err| { - log.warn( - "error copying command for title, title will not be set err={}", - .{err}, - ); - break :xdg; - }; - defer alloc.free(title); - _ = try rt_app.performAction( - .{ .surface = self }, - .set_title, - .{ .title = title }, + const title = v.string(alloc) catch |err| { + log.warn( + "error copying command for title, title will not be set err={}", + .{err}, ); - } + break :xdg; + }; + defer alloc.free(title); + _ = try rt_app.performAction( + .{ .surface = self }, + .set_title, + .{ .title = title }, + ); } // We are no longer the first surface diff --git a/src/apprt/embedded.zig b/src/apprt/embedded.zig index 9ae00ab8e..50b54435d 100644 --- a/src/apprt/embedded.zig +++ b/src/apprt/embedded.zig @@ -636,6 +636,11 @@ pub const Surface = struct { /// The command to run in the new surface. If this is set then /// the "wait-after-command" option is also automatically set to true, /// since this is used for scripting. + /// + /// This command always run in a shell (e.g. via `/bin/sh -c`), + /// despite Ghostty allowing directly executed commands via config. + /// This is a legacy thing and we should probably change it in the + /// future once we have a concrete use case. command: [*:0]const u8 = "", }; @@ -696,7 +701,7 @@ pub const Surface = struct { // If we have a command from the options then we set it. const cmd = std.mem.sliceTo(opts.command, 0); if (cmd.len > 0) { - config.command = cmd; + config.command = .{ .shell = cmd }; config.@"wait-after-command" = true; } diff --git a/src/config.zig b/src/config.zig index a06e19872..fb7359b3e 100644 --- a/src/config.zig +++ b/src/config.zig @@ -14,6 +14,7 @@ pub const formatEntry = formatter.formatEntry; // Field types pub const ClipboardAccess = Config.ClipboardAccess; +pub const Command = Config.Command; pub const ConfirmCloseSurface = Config.ConfirmCloseSurface; pub const CopyOnSelect = Config.CopyOnSelect; pub const CustomShaderAnimation = Config.CustomShaderAnimation; diff --git a/src/config/Config.zig b/src/config/Config.zig index 9cd285d3b..a0d9275e9 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -22,7 +22,6 @@ const inputpkg = @import("../input.zig"); const terminal = @import("../terminal/main.zig"); const internal_os = @import("../os/main.zig"); const cli = @import("../cli.zig"); -const Command = @import("../Command.zig"); const conditional = @import("conditional.zig"); const Conditional = conditional.Conditional; @@ -34,6 +33,7 @@ const KeyValue = @import("key.zig").Value; const ErrorList = @import("ErrorList.zig"); const MetricModifier = fontpkg.Metrics.Modifier; const help_strings = @import("help_strings"); +pub const Command = @import("command.zig").Command; const RepeatableStringMap = @import("RepeatableStringMap.zig"); pub const Path = @import("path.zig").Path; pub const RepeatablePath = @import("path.zig").RepeatablePath; @@ -691,8 +691,17 @@ palette: Palette = .{}, /// * `passwd` entry (user information) /// /// This can contain additional arguments to run the command with. If additional -/// arguments are provided, the command will be executed using `/bin/sh -c`. -/// Ghostty does not do any shell command parsing. +/// arguments are provided, the command will be executed using `/bin/sh -c` +/// to offload shell argument expansion. +/// +/// To avoid shell expansion altogether, prefix the command with `direct:`, +/// e.g. `direct:nvim foo`. This will avoid the roundtrip to `/bin/sh` but will +/// also not support any shell parsing such as arguments with spaces, filepaths +/// with `~`, globs, etc. +/// +/// You can also explicitly prefix the command with `shell:` to always +/// wrap the command in a shell. This can be used to ensure our heuristics +/// to choose the right mode are not used in case they are wrong. /// /// This command will be used for all new terminal surfaces, i.e. new windows, /// tabs, etc. If you want to run a command only for the first terminal surface @@ -702,7 +711,7 @@ palette: Palette = .{}, /// arguments. For example, `ghostty -e fish --with --custom --args`. /// This flag sets the `initial-command` configuration, see that for more /// information. -command: ?[]const u8 = null, +command: ?Command = null, /// This is the same as "command", but only applies to the first terminal /// surface created when Ghostty starts. Subsequent terminal surfaces will use @@ -718,6 +727,10 @@ command: ?[]const u8 = null, /// fish --with --custom --args`. The `-e` flag automatically forces some /// other behaviors as well: /// +/// * Disables shell expansion since the input is expected to already +/// be shell-expanded by the upstream (e.g. the shell used to type in +/// the `ghostty -e` command). +/// /// * `gtk-single-instance=false` - This ensures that a new instance is /// launched and the CLI args are respected. /// @@ -735,7 +748,7 @@ command: ?[]const u8 = null, /// name your binary appropriately or source the shell integration script /// manually. /// -@"initial-command": ?[]const u8 = null, +@"initial-command": ?Command = null, /// Extra environment variables to pass to commands launched in a terminal /// surface. The format is `env=KEY=VALUE`. @@ -2564,21 +2577,17 @@ pub fn loadCliArgs(self: *Config, alloc_gpa: Allocator) !void { // Next, take all remaining args and use that to build up // a command to execute. - var command = std.ArrayList(u8).init(arena_alloc); - errdefer command.deinit(); + var builder = std.ArrayList([:0]const u8).init(arena_alloc); + errdefer builder.deinit(); for (args) |arg_raw| { const arg = std.mem.sliceTo(arg_raw, 0); - try self._replay_steps.append( - arena_alloc, - .{ .arg = try arena_alloc.dupe(u8, arg) }, - ); - - try command.appendSlice(arg); - try command.append(' '); + const copy = try arena_alloc.dupeZ(u8, arg); + try self._replay_steps.append(arena_alloc, .{ .arg = copy }); + try builder.append(copy); } self.@"_xdg-terminal-exec" = true; - self.@"initial-command" = command.items[0 .. command.items.len - 1]; + self.@"initial-command" = .{ .direct = try builder.toOwnedSlice() }; return; } } @@ -3023,7 +3032,7 @@ pub fn finalize(self: *Config) !void { // We don't do this in flatpak because SHELL in Flatpak is always // set to /bin/sh. if (self.command) |cmd| - log.info("shell src=config value={s}", .{cmd}) + log.info("shell src=config value={}", .{cmd}) else shell_env: { // Flatpak always gets its shell from outside the sandbox if (internal_os.isFlatpak()) break :shell_env; @@ -3035,7 +3044,9 @@ pub fn finalize(self: *Config) !void { if (std.process.getEnvVarOwned(alloc, "SHELL")) |value| { log.info("default shell source=env value={s}", .{value}); - self.command = value; + + const copy = try alloc.dupeZ(u8, value); + self.command = .{ .shell = copy }; // If we don't need the working directory, then we can exit now. if (!wd_home) break :command; @@ -3046,7 +3057,7 @@ pub fn finalize(self: *Config) !void { .windows => { if (self.command == null) { log.warn("no default shell found, will default to using cmd", .{}); - self.command = "cmd.exe"; + self.command = .{ .shell = "cmd.exe" }; } if (wd_home) { @@ -3063,7 +3074,7 @@ pub fn finalize(self: *Config) !void { if (self.command == null) { if (pw.shell) |sh| { log.info("default shell src=passwd value={s}", .{sh}); - self.command = sh; + self.command = .{ .shell = sh }; } } @@ -3145,13 +3156,13 @@ pub fn parseManuallyHook( // Build up the command. We don't clean this up because we take // ownership in our allocator. - var command = std.ArrayList(u8).init(alloc); + var command: std.ArrayList([:0]const u8) = .init(alloc); errdefer command.deinit(); while (iter.next()) |param| { - try self._replay_steps.append(alloc, .{ .arg = try alloc.dupe(u8, param) }); - try command.appendSlice(param); - try command.append(' '); + const copy = try alloc.dupeZ(u8, param); + try self._replay_steps.append(alloc, .{ .arg = copy }); + try command.append(copy); } if (command.items.len == 0) { @@ -3167,9 +3178,8 @@ pub fn parseManuallyHook( return false; } - self.@"initial-command" = command.items[0 .. command.items.len - 1]; - // See "command" docs for the implied configurations and why. + self.@"initial-command" = .{ .direct = command.items }; self.@"gtk-single-instance" = .false; self.@"quit-after-last-window-closed" = true; self.@"quit-after-last-window-closed-delay" = null; @@ -3184,7 +3194,7 @@ pub fn parseManuallyHook( // Keep track of our input args for replay try self._replay_steps.append( alloc, - .{ .arg = try alloc.dupe(u8, arg) }, + .{ .arg = try alloc.dupeZ(u8, arg) }, ); // If we didn't find a special case, continue parsing normally @@ -3377,6 +3387,16 @@ fn equalField(comptime T: type, old: T, new: T) bool { [:0]const u8, => return std.mem.eql(u8, old, new), + []const [:0]const u8, + => { + if (old.len != new.len) return false; + for (old, new) |a, b| { + if (!std.mem.eql(u8, a, b)) return false; + } + + return true; + }, + else => {}, } @@ -3412,6 +3432,8 @@ fn equalField(comptime T: type, old: T, new: T) bool { }, .@"union" => |info| { + if (@hasDecl(T, "equal")) return old.equal(new); + const tag_type = info.tag_type.?; const old_tag = std.meta.activeTag(old); const new_tag = std.meta.activeTag(new); @@ -3441,7 +3463,7 @@ fn equalField(comptime T: type, old: T, new: T) bool { const Replay = struct { const Step = union(enum) { /// An argument to parse as if it came from the CLI or file. - arg: []const u8, + arg: [:0]const u8, /// A base path to expand relative paths against. expand: []const u8, @@ -3481,7 +3503,7 @@ const Replay = struct { return switch (self) { .@"-e" => self, .diagnostic => |v| .{ .diagnostic = try v.clone(alloc) }, - .arg => |v| .{ .arg = try alloc.dupe(u8, v) }, + .arg => |v| .{ .arg = try alloc.dupeZ(u8, v) }, .expand => |v| .{ .expand = try alloc.dupe(u8, v) }, .conditional_arg => |v| conditional: { var conds = try alloc.alloc(Conditional, v.conditions.len); @@ -6620,7 +6642,11 @@ test "parse e: command only" { var it: TestIterator = .{ .data = &.{"foo"} }; try testing.expect(!try cfg.parseManuallyHook(alloc, "-e", &it)); - try testing.expectEqualStrings("foo", cfg.@"initial-command".?); + + const cmd = cfg.@"initial-command".?; + try testing.expect(cmd == .direct); + try testing.expectEqual(cmd.direct.len, 1); + try testing.expectEqualStrings(cmd.direct[0], "foo"); } test "parse e: command and args" { @@ -6631,7 +6657,13 @@ test "parse e: command and args" { var it: TestIterator = .{ .data = &.{ "echo", "foo", "bar baz" } }; try testing.expect(!try cfg.parseManuallyHook(alloc, "-e", &it)); - try testing.expectEqualStrings("echo foo bar baz", cfg.@"initial-command".?); + + const cmd = cfg.@"initial-command".?; + try testing.expect(cmd == .direct); + try testing.expectEqual(cmd.direct.len, 3); + try testing.expectEqualStrings(cmd.direct[0], "echo"); + try testing.expectEqualStrings(cmd.direct[1], "foo"); + try testing.expectEqualStrings(cmd.direct[2], "bar baz"); } test "clone default" { diff --git a/src/config/command.zig b/src/config/command.zig new file mode 100644 index 000000000..9efeb199e --- /dev/null +++ b/src/config/command.zig @@ -0,0 +1,322 @@ +const std = @import("std"); +const builtin = @import("builtin"); +const Allocator = std.mem.Allocator; +const ArenaAllocator = std.heap.ArenaAllocator; +const formatterpkg = @import("formatter.zig"); + +/// A command to execute (argv0 and args). +/// +/// A command is specified as a simple string such as "nvim a b c". +/// By default, we expect the downstream to do some sort of shell expansion +/// on this string. +/// +/// If a command is already expanded and the user does NOT want to do +/// shell expansion (because this usually requires a round trip into +/// /bin/sh or equivalent), specify a `direct:`-prefix. e.g. +/// `direct:nvim a b c`. +/// +/// The whitespace before or around the prefix is ignored. For example, +/// ` direct:nvim a b c` and `direct: nvim a b c` are equivalent. +/// +/// If the command is not absolute, it'll be looked up via the PATH. +/// For the shell-expansion case, we let the shell do this. For the +/// direct case, we do this directly. +pub const Command = union(enum) { + const Self = @This(); + + /// Execute a command directly, e.g. via `exec`. The format here + /// is already structured to be ready to passed directly to `exec` + /// with index zero being the command to execute. + /// + /// Index zero is not guaranteed to be an absolute path, and may require + /// PATH lookup. It is up to the downstream to do this, usually via + /// delegation to something like `execvp`. + direct: []const [:0]const u8, + + /// Execute a command via shell expansion. This provides the command + /// as a single string that is expected to be expanded in some way + /// (up to the downstream). Usually `/bin/sh -c`. + shell: [:0]const u8, + + pub fn parseCLI( + self: *Self, + alloc: Allocator, + input_: ?[]const u8, + ) !void { + // Input is required. Whitespace on the edges isn't needed. + // Commands must be non-empty. + const input = input_ orelse return error.ValueRequired; + const trimmed = std.mem.trim(u8, input, " "); + if (trimmed.len == 0) return error.ValueRequired; + + // If we have a `:` then we MIGHT have a prefix to specify what + // tag we should use. + const tag: std.meta.Tag(Self), const str: []const u8 = tag: { + if (std.mem.indexOfScalar(u8, trimmed, ':')) |idx| { + const prefix = trimmed[0..idx]; + if (std.mem.eql(u8, prefix, "direct")) { + break :tag .{ .direct, trimmed[idx + 1 ..] }; + } else if (std.mem.eql(u8, prefix, "shell")) { + break :tag .{ .shell, trimmed[idx + 1 ..] }; + } + } + + break :tag .{ .shell, trimmed }; + }; + + switch (tag) { + .shell => { + // We have a shell command, so we can just dupe it. + const copy = try alloc.dupeZ(u8, std.mem.trim(u8, str, " ")); + self.* = .{ .shell = copy }; + }, + + .direct => { + // We're not shell expanding, so the arguments are naively + // split on spaces. + var builder: std.ArrayListUnmanaged([:0]const u8) = .empty; + var args = std.mem.splitScalar( + u8, + std.mem.trim(u8, str, " "), + ' ', + ); + while (args.next()) |arg| { + const copy = try alloc.dupeZ(u8, arg); + try builder.append(alloc, copy); + } + + self.* = .{ .direct = try builder.toOwnedSlice(alloc) }; + }, + } + } + + /// Creates a command as a single string, joining arguments as + /// necessary with spaces. Its not guaranteed that this is a valid + /// command; it is only meant to be human readable. + pub fn string( + self: *const Self, + alloc: Allocator, + ) Allocator.Error![:0]const u8 { + return switch (self.*) { + .shell => |v| try alloc.dupeZ(u8, v), + .direct => |v| try std.mem.joinZ(alloc, " ", v), + }; + } + + /// Get an iterator over the arguments array. This may allocate + /// depending on the active tag of the command. + /// + /// For direct commands, this is very cheap and just iterates over + /// the array. There is no allocation. + /// + /// For shell commands, this will use Zig's ArgIteratorGeneral as + /// a best effort shell string parser. This is not guaranteed to be + /// 100% accurate, but it works for common cases. This requires allocation. + pub fn argIterator( + self: *const Self, + alloc: Allocator, + ) Allocator.Error!ArgIterator { + return switch (self.*) { + .direct => |v| .{ .direct = .{ .args = v } }, + .shell => |v| .{ .shell = try .init(alloc, v) }, + }; + } + + /// Iterates over each argument in the command. + pub const ArgIterator = union(enum) { + shell: std.process.ArgIteratorGeneral(.{}), + direct: struct { + i: usize = 0, + args: []const [:0]const u8, + }, + + /// Return the next argument. This may or may not be a copy + /// depending on the active tag. If you want to ensure that every + /// argument is a copy, use the `clone` method first. + pub fn next(self: *ArgIterator) ?[:0]const u8 { + return switch (self.*) { + .shell => |*v| v.next(), + .direct => |*v| { + if (v.i >= v.args.len) return null; + defer v.i += 1; + return v.args[v.i]; + }, + }; + } + + pub fn deinit(self: *ArgIterator) void { + switch (self.*) { + .shell => |*v| v.deinit(), + .direct => {}, + } + } + }; + + pub fn clone( + self: *const Self, + alloc: Allocator, + ) Allocator.Error!Self { + return switch (self.*) { + .shell => |v| .{ .shell = try alloc.dupeZ(u8, v) }, + .direct => |v| direct: { + const copy = try alloc.alloc([:0]const u8, v.len); + for (v, 0..) |arg, i| copy[i] = try alloc.dupeZ(u8, arg); + break :direct .{ .direct = copy }; + }, + }; + } + + pub fn formatEntry(self: Self, formatter: anytype) !void { + switch (self) { + .shell => |v| try formatter.formatEntry([]const u8, v), + + .direct => |v| { + var buf: [4096]u8 = undefined; + var fbs = std.io.fixedBufferStream(&buf); + const writer = fbs.writer(); + writer.writeAll("direct:") catch return error.OutOfMemory; + for (v) |arg| { + writer.writeAll(arg) catch return error.OutOfMemory; + writer.writeByte(' ') catch return error.OutOfMemory; + } + + const written = fbs.getWritten(); + try formatter.formatEntry( + []const u8, + written[0..@intCast(written.len - 1)], + ); + }, + } + } + + test "Command: parseCLI errors" { + const testing = std.testing; + var arena = ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + const alloc = arena.allocator(); + + var v: Self = undefined; + try testing.expectError(error.ValueRequired, v.parseCLI(alloc, null)); + try testing.expectError(error.ValueRequired, v.parseCLI(alloc, "")); + try testing.expectError(error.ValueRequired, v.parseCLI(alloc, " ")); + } + + test "Command: parseCLI shell expanded" { + const testing = std.testing; + var arena = ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + const alloc = arena.allocator(); + + var v: Self = undefined; + try v.parseCLI(alloc, "echo hello"); + try testing.expect(v == .shell); + try testing.expectEqualStrings(v.shell, "echo hello"); + + // Spaces are stripped + try v.parseCLI(alloc, " echo hello "); + try testing.expect(v == .shell); + try testing.expectEqualStrings(v.shell, "echo hello"); + } + + test "Command: parseCLI direct" { + const testing = std.testing; + var arena = ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + const alloc = arena.allocator(); + + var v: Self = undefined; + try v.parseCLI(alloc, "direct:echo hello"); + try testing.expect(v == .direct); + try testing.expectEqual(v.direct.len, 2); + try testing.expectEqualStrings(v.direct[0], "echo"); + try testing.expectEqualStrings(v.direct[1], "hello"); + + // Spaces around the prefix + try v.parseCLI(alloc, " direct: echo hello"); + try testing.expect(v == .direct); + try testing.expectEqual(v.direct.len, 2); + try testing.expectEqualStrings(v.direct[0], "echo"); + try testing.expectEqualStrings(v.direct[1], "hello"); + } + + test "Command: argIterator shell" { + const testing = std.testing; + const alloc = testing.allocator; + + var v: Self = .{ .shell = "echo hello world" }; + var it = try v.argIterator(alloc); + defer it.deinit(); + + try testing.expectEqualStrings(it.next().?, "echo"); + try testing.expectEqualStrings(it.next().?, "hello"); + try testing.expectEqualStrings(it.next().?, "world"); + try testing.expect(it.next() == null); + } + + test "Command: argIterator direct" { + const testing = std.testing; + const alloc = testing.allocator; + + var v: Self = .{ .direct = &.{ "echo", "hello world" } }; + var it = try v.argIterator(alloc); + defer it.deinit(); + + try testing.expectEqualStrings(it.next().?, "echo"); + try testing.expectEqualStrings(it.next().?, "hello world"); + try testing.expect(it.next() == null); + } + + test "Command: string shell" { + const testing = std.testing; + const alloc = testing.allocator; + + var v: Self = .{ .shell = "echo hello world" }; + const str = try v.string(alloc); + defer alloc.free(str); + try testing.expectEqualStrings(str, "echo hello world"); + } + + test "Command: string direct" { + const testing = std.testing; + const alloc = testing.allocator; + + var v: Self = .{ .direct = &.{ "echo", "hello world" } }; + const str = try v.string(alloc); + defer alloc.free(str); + try testing.expectEqualStrings(str, "echo hello world"); + } + + test "Command: formatConfig shell" { + const testing = std.testing; + var arena = ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + const alloc = arena.allocator(); + + var buf = std.ArrayList(u8).init(alloc); + defer buf.deinit(); + + var v: Self = undefined; + try v.parseCLI(alloc, "echo hello"); + try v.formatEntry(formatterpkg.entryFormatter("a", buf.writer())); + try std.testing.expectEqualSlices(u8, "a = echo hello\n", buf.items); + } + + test "Command: formatConfig direct" { + const testing = std.testing; + var arena = ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + const alloc = arena.allocator(); + + var buf = std.ArrayList(u8).init(alloc); + defer buf.deinit(); + + var v: Self = undefined; + try v.parseCLI(alloc, "direct: echo hello"); + try v.formatEntry(formatterpkg.entryFormatter("a", buf.writer())); + try std.testing.expectEqualSlices(u8, "a = direct:echo hello\n", buf.items); + } +}; + +test { + _ = Command; +} diff --git a/src/os/passwd.zig b/src/os/passwd.zig index c12214ee4..e9bbff066 100644 --- a/src/os/passwd.zig +++ b/src/os/passwd.zig @@ -25,9 +25,9 @@ const c = if (builtin.os.tag != .windows) @cImport({ // Entry that is retrieved from the passwd API. This only contains the fields // we care about. pub const Entry = struct { - shell: ?[]const u8 = null, - home: ?[]const u8 = null, - name: ?[]const u8 = null, + shell: ?[:0]const u8 = null, + home: ?[:0]const u8 = null, + name: ?[:0]const u8 = null, }; /// Get the passwd entry for the currently executing user. @@ -117,30 +117,27 @@ pub fn get(alloc: Allocator) !Entry { // Shell and home are the last two entries var it = std.mem.splitBackwardsScalar(u8, std.mem.trimRight(u8, output, " \r\n"), ':'); - result.shell = it.next() orelse null; - result.home = it.next() orelse null; + result.shell = if (it.next()) |v| try alloc.dupeZ(u8, v) else null; + result.home = if (it.next()) |v| try alloc.dupeZ(u8, v) else null; return result; } if (pw.pw_shell) |ptr| { const source = std.mem.sliceTo(ptr, 0); - const sh = try alloc.alloc(u8, source.len); - @memcpy(sh, source); - result.shell = sh; + const value = try alloc.dupeZ(u8, source); + result.shell = value; } if (pw.pw_dir) |ptr| { const source = std.mem.sliceTo(ptr, 0); - const dir = try alloc.alloc(u8, source.len); - @memcpy(dir, source); - result.home = dir; + const value = try alloc.dupeZ(u8, source); + result.home = value; } if (pw.pw_name) |ptr| { const source = std.mem.sliceTo(ptr, 0); - const name = try alloc.alloc(u8, source.len); - @memcpy(name, source); - result.name = name; + const value = try alloc.dupeZ(u8, source); + result.name = value; } return result; diff --git a/src/termio/Exec.zig b/src/termio/Exec.zig index 61b501258..abe49a47b 100644 --- a/src/termio/Exec.zig +++ b/src/termio/Exec.zig @@ -24,6 +24,7 @@ const SegmentedPool = @import("../datastruct/main.zig").SegmentedPool; const ptypkg = @import("../pty.zig"); const Pty = ptypkg.Pty; const EnvMap = std.process.EnvMap; +const PasswdEntry = internal_os.passwd.Entry; const windows = internal_os.windows; const log = std.log.scoped(.io_exec); @@ -725,7 +726,7 @@ pub const ThreadData = struct { }; pub const Config = struct { - command: ?[]const u8 = null, + command: ?configpkg.Command = null, env: EnvMap, env_override: configpkg.RepeatableStringMap = .{}, shell_integration: configpkg.Config.ShellIntegration = .detect, @@ -746,7 +747,7 @@ const Subprocess = struct { arena: std.heap.ArenaAllocator, cwd: ?[]const u8, env: ?EnvMap, - args: [][]const u8, + args: []const [:0]const u8, grid_size: renderer.GridSize, screen_size: renderer.ScreenSize, pty: ?Pty = null, @@ -892,18 +893,29 @@ const Subprocess = struct { env.remove("VTE_VERSION"); // Setup our shell integration, if we can. - const integrated_shell: ?shell_integration.Shell, const shell_command: []const u8 = shell: { - const default_shell_command = cfg.command orelse switch (builtin.os.tag) { - .windows => "cmd.exe", - else => "sh", - }; + const shell_command: configpkg.Command = shell: { + const default_shell_command: configpkg.Command = + cfg.command orelse .{ .shell = switch (builtin.os.tag) { + .windows => "cmd.exe", + else => "sh", + } }; const force: ?shell_integration.Shell = switch (cfg.shell_integration) { .none => { - // Even if shell integration is none, we still want to set up the feature env vars - try shell_integration.setupFeatures(&env, cfg.shell_integration_features); - break :shell .{ null, default_shell_command }; + // Even if shell integration is none, we still want to + // set up the feature env vars + try shell_integration.setupFeatures( + &env, + cfg.shell_integration_features, + ); + + // This is a source of confusion for users despite being + // opt-in since it results in some Ghostty features not + // working. We always want to log it. + log.info("shell integration disabled by configuration", .{}); + break :shell default_shell_command; }, + .detect => null, .bash => .bash, .elvish => .elvish, @@ -911,9 +923,9 @@ const Subprocess = struct { .zsh => .zsh, }; - const dir = cfg.resources_dir orelse break :shell .{ - null, - default_shell_command, + const dir = cfg.resources_dir orelse { + log.warn("no resources dir set, shell integration disabled", .{}); + break :shell default_shell_command; }; const integration = try shell_integration.setup( @@ -923,19 +935,18 @@ const Subprocess = struct { &env, force, cfg.shell_integration_features, - ) orelse break :shell .{ null, default_shell_command }; + ) orelse { + log.warn("shell could not be detected, no automatic shell integration will be injected", .{}); + break :shell default_shell_command; + }; - break :shell .{ integration.shell, integration.command }; - }; - - if (integrated_shell) |shell| { log.info( "shell integration automatically injected shell={}", - .{shell}, + .{integration.shell}, ); - } else if (cfg.shell_integration != .none) { - log.warn("shell could not be detected, no automatic shell integration will be injected", .{}); - } + + break :shell integration.command; + }; // Add the environment variables that override any others. { @@ -947,134 +958,29 @@ const Subprocess = struct { } // Build our args list - const args = args: { - const cap = 9; // the most we'll ever use - var args = try std.ArrayList([]const u8).initCapacity(alloc, cap); - defer args.deinit(); + const args: []const [:0]const u8 = execCommand( + alloc, + shell_command, + internal_os.passwd, + ) catch |err| switch (err) { + // If we fail to allocate space for the command we want to + // execute, we'd still like to try to run something so + // Ghostty can launch (and maybe the user can debug this further). + // Realistically, if you're getting OOM, I think other stuff is + // about to crash, but we can try. + error.OutOfMemory => oom: { + log.warn("failed to allocate space for command args, falling back to basic shell", .{}); - // If we're on macOS, we have to use `login(1)` to get all of - // the proper environment variables set, a login shell, and proper - // hushlogin behavior. - if (comptime builtin.target.os.tag.isDarwin()) darwin: { - const passwd = internal_os.passwd.get(alloc) catch |err| { - log.warn("failed to read passwd, not using a login shell err={}", .{err}); - break :darwin; + // The comptime here is important to ensure the full slice + // is put into the binary data and not the stack. + break :oom comptime switch (builtin.os.tag) { + .windows => &.{"cmd.exe"}, + else => &.{"/bin/sh"}, }; + }, - const username = passwd.name orelse { - log.warn("failed to get username, not using a login shell", .{}); - break :darwin; - }; - - const hush = if (passwd.home) |home| hush: { - var dir = std.fs.openDirAbsolute(home, .{}) catch |err| { - log.warn( - "failed to open home dir, not checking for hushlogin err={}", - .{err}, - ); - break :hush false; - }; - defer dir.close(); - - break :hush if (dir.access(".hushlogin", .{})) true else |_| false; - } else false; - - const cmd = try std.fmt.allocPrint( - alloc, - "exec -l {s}", - .{shell_command}, - ); - - // The reason for executing login this way is unclear. This - // comment will attempt to explain but prepare for a truly - // unhinged reality. - // - // The first major issue is that on macOS, a lot of users - // put shell configurations in ~/.bash_profile instead of - // ~/.bashrc (or equivalent for another shell). This file is only - // loaded for a login shell so macOS users expect all their terminals - // to be login shells. No other platform behaves this way and its - // totally braindead but somehow the entire dev community on - // macOS has cargo culted their way to this reality so we have to - // do it... - // - // To get a login shell, you COULD just prepend argv0 with a `-` - // but that doesn't fully work because `getlogin()` C API will - // return the wrong value, SHELL won't be set, and various - // other login behaviors that macOS users expect. - // - // The proper way is to use `login(1)`. But login(1) forces - // the working directory to change to the home directory, - // which we may not want. If we specify "-l" then we can avoid - // this behavior but now the shell isn't a login shell. - // - // There is another issue: `login(1)` on macOS 14.3 and earlier - // checked for ".hushlogin" in the working directory. This means - // that if we specify "-l" then we won't get hushlogin honored - // if its in the home directory (which is standard). To get - // around this, we check for hushlogin ourselves and if present - // specify the "-q" flag to login(1). - // - // So to get all the behaviors we want, we specify "-l" but - // execute "bash" (which is built-in to macOS). We then use - // the bash builtin "exec" to replace the process with a login - // shell ("-l" on exec) with the command we really want. - // - // We use "bash" instead of other shells that ship with macOS - // because as of macOS Sonoma, we found with a microbenchmark - // that bash can `exec` into the desired command ~2x faster - // than zsh. - // - // To figure out a lot of this logic I read the login.c - // source code in the OSS distribution Apple provides for - // macOS. - // - // Awesome. - try args.append("/usr/bin/login"); - if (hush) try args.append("-q"); - try args.append("-flp"); - - // We execute bash with "--noprofile --norc" so that it doesn't - // load startup files so that (1) our shell integration doesn't - // break and (2) user configuration doesn't mess this process - // up. - try args.append(username); - try args.append("/bin/bash"); - try args.append("--noprofile"); - try args.append("--norc"); - try args.append("-c"); - try args.append(cmd); - break :args try args.toOwnedSlice(); - } - - if (comptime builtin.os.tag == .windows) { - // We run our shell wrapped in `cmd.exe` so that we don't have - // to parse the command line ourselves if it has arguments. - - // Note we don't free any of the memory below since it is - // allocated in the arena. - const windir = try std.process.getEnvVarOwned(alloc, "WINDIR"); - const cmd = try std.fs.path.join(alloc, &[_][]const u8{ - windir, - "System32", - "cmd.exe", - }); - - try args.append(cmd); - try args.append("/C"); - } else { - // We run our shell wrapped in `/bin/sh` so that we don't have - // to parse the command line ourselves if it has arguments. - // Additionally, some environments (NixOS, I found) use /bin/sh - // to setup some environment variables that are important to - // have set. - try args.append("/bin/sh"); - if (internal_os.isFlatpak()) try args.append("-l"); - try args.append("-c"); - } - - try args.append(shell_command); - break :args try args.toOwnedSlice(); + // This logs on its own, this is a bad error. + error.SystemError => return err, }; // We have to copy the cwd because there is no guarantee that @@ -1562,3 +1468,320 @@ pub const ReadThread = struct { } } }; + +/// Builds the argv array for the process we should exec for the +/// configured command. This isn't as straightforward as it seems since +/// we deal with shell-wrapping, macOS login shells, etc. +/// +/// The passwdpkg comptime argument is expected to have a single function +/// `get(Allocator)` that returns a passwd entry. This is used by macOS +/// to determine the username and home directory for the login shell. +/// It is unused on other platforms. +/// +/// Memory ownership: +/// +/// The allocator should be an arena, since the returned value may or +/// may not be allocated and args may or may not be allocated (or copied). +/// Pointers in the return value may point to pointers in the command +/// struct. +fn execCommand( + alloc: Allocator, + command: configpkg.Command, + comptime passwdpkg: type, +) (Allocator.Error || error{SystemError})![]const [:0]const u8 { + // If we're on macOS, we have to use `login(1)` to get all of + // the proper environment variables set, a login shell, and proper + // hushlogin behavior. + if (comptime builtin.target.os.tag.isDarwin()) darwin: { + const passwd = passwdpkg.get(alloc) catch |err| { + log.warn("failed to read passwd, not using a login shell err={}", .{err}); + break :darwin; + }; + + const username = passwd.name orelse { + log.warn("failed to get username, not using a login shell", .{}); + break :darwin; + }; + + const hush = if (passwd.home) |home| hush: { + var dir = std.fs.openDirAbsolute(home, .{}) catch |err| { + log.warn( + "failed to open home dir, not checking for hushlogin err={}", + .{err}, + ); + break :hush false; + }; + defer dir.close(); + + break :hush if (dir.access(".hushlogin", .{})) true else |_| false; + } else false; + + // If we made it this far we're going to start building + // the actual command. + var args: std.ArrayList([:0]const u8) = try .initCapacity( + alloc, + + // This capacity is chosen based on what we'd need to + // execute a shell command (very common). We can/will + // grow if necessary for a longer command (uncommon). + 9, + ); + defer args.deinit(); + + // The reason for executing login this way is unclear. This + // comment will attempt to explain but prepare for a truly + // unhinged reality. + // + // The first major issue is that on macOS, a lot of users + // put shell configurations in ~/.bash_profile instead of + // ~/.bashrc (or equivalent for another shell). This file is only + // loaded for a login shell so macOS users expect all their terminals + // to be login shells. No other platform behaves this way and its + // totally braindead but somehow the entire dev community on + // macOS has cargo culted their way to this reality so we have to + // do it... + // + // To get a login shell, you COULD just prepend argv0 with a `-` + // but that doesn't fully work because `getlogin()` C API will + // return the wrong value, SHELL won't be set, and various + // other login behaviors that macOS users expect. + // + // The proper way is to use `login(1)`. But login(1) forces + // the working directory to change to the home directory, + // which we may not want. If we specify "-l" then we can avoid + // this behavior but now the shell isn't a login shell. + // + // There is another issue: `login(1)` on macOS 14.3 and earlier + // checked for ".hushlogin" in the working directory. This means + // that if we specify "-l" then we won't get hushlogin honored + // if its in the home directory (which is standard). To get + // around this, we check for hushlogin ourselves and if present + // specify the "-q" flag to login(1). + // + // So to get all the behaviors we want, we specify "-l" but + // execute "bash" (which is built-in to macOS). We then use + // the bash builtin "exec" to replace the process with a login + // shell ("-l" on exec) with the command we really want. + // + // We use "bash" instead of other shells that ship with macOS + // because as of macOS Sonoma, we found with a microbenchmark + // that bash can `exec` into the desired command ~2x faster + // than zsh. + // + // To figure out a lot of this logic I read the login.c + // source code in the OSS distribution Apple provides for + // macOS. + // + // Awesome. + try args.append("/usr/bin/login"); + if (hush) try args.append("-q"); + try args.append("-flp"); + try args.append(username); + + switch (command) { + // Direct args can be passed directly to login, since + // login uses execvp we don't need to worry about PATH + // searching. + .direct => |v| try args.appendSlice(v), + + .shell => |v| { + // Use "exec" to replace the bash process with + // our intended command so we don't have a parent + // process hanging around. + const cmd = try std.fmt.allocPrintZ( + alloc, + "exec -l {s}", + .{v}, + ); + + // We execute bash with "--noprofile --norc" so that it doesn't + // load startup files so that (1) our shell integration doesn't + // break and (2) user configuration doesn't mess this process + // up. + try args.append("/bin/bash"); + try args.append("--noprofile"); + try args.append("--norc"); + try args.append("-c"); + try args.append(cmd); + }, + } + + return try args.toOwnedSlice(); + } + + return switch (command) { + .direct => |v| v, + + .shell => |v| shell: { + var args: std.ArrayList([:0]const u8) = try .initCapacity(alloc, 4); + defer args.deinit(); + + if (comptime builtin.os.tag == .windows) { + // We run our shell wrapped in `cmd.exe` so that we don't have + // to parse the command line ourselves if it has arguments. + + // Note we don't free any of the memory below since it is + // allocated in the arena. + const windir = std.process.getEnvVarOwned( + alloc, + "WINDIR", + ) catch |err| { + log.warn("failed to get WINDIR, cannot run shell command err={}", .{err}); + return error.SystemError; + }; + const cmd = try std.fs.path.joinZ(alloc, &[_][]const u8{ + windir, + "System32", + "cmd.exe", + }); + + try args.append(cmd); + try args.append("/C"); + } else { + // We run our shell wrapped in `/bin/sh` so that we don't have + // to parse the command line ourselves if it has arguments. + // Additionally, some environments (NixOS, I found) use /bin/sh + // to setup some environment variables that are important to + // have set. + try args.append("/bin/sh"); + if (internal_os.isFlatpak()) try args.append("-l"); + try args.append("-c"); + } + + try args.append(v); + break :shell try args.toOwnedSlice(); + }, + }; +} + +test "execCommand darwin: shell command" { + if (comptime !builtin.os.tag.isDarwin()) return error.SkipZigTest; + + const testing = std.testing; + var arena = ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + const alloc = arena.allocator(); + + const result = try execCommand(alloc, .{ .shell = "foo bar baz" }, struct { + fn get(_: Allocator) !PasswdEntry { + return .{ + .name = "testuser", + }; + } + }); + + try testing.expectEqual(8, result.len); + try testing.expectEqualStrings(result[0], "/usr/bin/login"); + try testing.expectEqualStrings(result[1], "-flp"); + try testing.expectEqualStrings(result[2], "testuser"); + try testing.expectEqualStrings(result[3], "/bin/bash"); + try testing.expectEqualStrings(result[4], "--noprofile"); + try testing.expectEqualStrings(result[5], "--norc"); + try testing.expectEqualStrings(result[6], "-c"); + try testing.expectEqualStrings(result[7], "exec -l foo bar baz"); +} + +test "execCommand darwin: direct command" { + if (comptime !builtin.os.tag.isDarwin()) return error.SkipZigTest; + + const testing = std.testing; + var arena = ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + const alloc = arena.allocator(); + + const result = try execCommand(alloc, .{ .direct = &.{ + "foo", + "bar baz", + } }, struct { + fn get(_: Allocator) !PasswdEntry { + return .{ + .name = "testuser", + }; + } + }); + + try testing.expectEqual(5, result.len); + try testing.expectEqualStrings(result[0], "/usr/bin/login"); + try testing.expectEqualStrings(result[1], "-flp"); + try testing.expectEqualStrings(result[2], "testuser"); + try testing.expectEqualStrings(result[3], "foo"); + try testing.expectEqualStrings(result[4], "bar baz"); +} + +test "execCommand: shell command, empty passwd" { + if (comptime builtin.os.tag == .windows) return error.SkipZigTest; + + const testing = std.testing; + var arena = ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + const alloc = arena.allocator(); + + const result = try execCommand( + alloc, + .{ .shell = "foo bar baz" }, + struct { + fn get(_: Allocator) !PasswdEntry { + // Empty passwd entry means we can't construct a macOS + // login command and falls back to POSIX behavior. + return .{}; + } + }, + ); + + try testing.expectEqual(3, result.len); + try testing.expectEqualStrings(result[0], "/bin/sh"); + try testing.expectEqualStrings(result[1], "-c"); + try testing.expectEqualStrings(result[2], "foo bar baz"); +} + +test "execCommand: shell command, error passwd" { + if (comptime builtin.os.tag == .windows) return error.SkipZigTest; + + const testing = std.testing; + var arena = ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + const alloc = arena.allocator(); + + const result = try execCommand( + alloc, + .{ .shell = "foo bar baz" }, + struct { + fn get(_: Allocator) !PasswdEntry { + // Failed passwd entry means we can't construct a macOS + // login command and falls back to POSIX behavior. + return error.Fail; + } + }, + ); + + try testing.expectEqual(3, result.len); + try testing.expectEqualStrings(result[0], "/bin/sh"); + try testing.expectEqualStrings(result[1], "-c"); + try testing.expectEqualStrings(result[2], "foo bar baz"); +} + +test "execCommand: direct command, error passwd" { + if (comptime builtin.os.tag == .windows) return error.SkipZigTest; + + const testing = std.testing; + var arena = ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + const alloc = arena.allocator(); + + const result = try execCommand(alloc, .{ + .direct = &.{ + "foo", + "bar baz", + }, + }, struct { + fn get(_: Allocator) !PasswdEntry { + // Failed passwd entry means we can't construct a macOS + // login command and falls back to POSIX behavior. + return error.Fail; + } + }); + + try testing.expectEqual(2, result.len); + try testing.expectEqualStrings(result[0], "foo"); + try testing.expectEqualStrings(result[1], "bar baz"); +} diff --git a/src/termio/shell_integration.zig b/src/termio/shell_integration.zig index ae8d5b67c..2cf809694 100644 --- a/src/termio/shell_integration.zig +++ b/src/termio/shell_integration.zig @@ -27,7 +27,7 @@ pub const ShellIntegration = struct { /// bash in particular it may be different. /// /// The memory is allocated in the arena given to setup. - command: []const u8, + command: config.Command, }; /// Set up the command execution environment for automatic @@ -41,7 +41,7 @@ pub const ShellIntegration = struct { pub fn setup( alloc_arena: Allocator, resource_dir: []const u8, - command: []const u8, + command: config.Command, env: *EnvMap, force_shell: ?Shell, features: config.ShellIntegrationFeatures, @@ -51,14 +51,24 @@ pub fn setup( .elvish => "elvish", .fish => "fish", .zsh => "zsh", - } else exe: { - // The command can include arguments. Look for the first space - // and use the basename of the first part as the command's exe. - const idx = std.mem.indexOfScalar(u8, command, ' ') orelse command.len; - break :exe std.fs.path.basename(command[0..idx]); + } else switch (command) { + .direct => |v| std.fs.path.basename(v[0]), + .shell => |v| exe: { + // Shell strings can include spaces so we want to only + // look up to the space if it exists. No shell that we integrate + // has spaces. + const idx = std.mem.indexOfScalar(u8, v, ' ') orelse v.len; + break :exe std.fs.path.basename(v[0..idx]); + }, }; - const result = try setupShell(alloc_arena, resource_dir, command, env, exe); + const result = try setupShell( + alloc_arena, + resource_dir, + command, + env, + exe, + ); // Setup our feature env vars try setupFeatures(env, features); @@ -69,7 +79,7 @@ pub fn setup( fn setupShell( alloc_arena: Allocator, resource_dir: []const u8, - command: []const u8, + command: config.Command, env: *EnvMap, exe: []const u8, ) !?ShellIntegration { @@ -83,7 +93,10 @@ fn setupShell( // we're using Apple's Bash because /bin is non-writable // on modern macOS due to System Integrity Protection. if (comptime builtin.target.os.tag.isDarwin()) { - if (std.mem.eql(u8, "/bin/bash", command)) { + if (std.mem.eql(u8, "/bin/bash", switch (command) { + .direct => |v| v[0], + .shell => |v| v, + })) { return null; } } @@ -104,7 +117,7 @@ fn setupShell( try setupXdgDataDirs(alloc_arena, resource_dir, env); return .{ .shell = .elvish, - .command = try alloc_arena.dupe(u8, command), + .command = try command.clone(alloc_arena), }; } @@ -112,7 +125,7 @@ fn setupShell( try setupXdgDataDirs(alloc_arena, resource_dir, env); return .{ .shell = .fish, - .command = try alloc_arena.dupe(u8, command), + .command = try command.clone(alloc_arena), }; } @@ -120,7 +133,7 @@ fn setupShell( try setupZsh(resource_dir, env); return .{ .shell = .zsh, - .command = try alloc_arena.dupe(u8, command), + .command = try command.clone(alloc_arena), }; } @@ -139,7 +152,14 @@ test "force shell" { inline for (@typeInfo(Shell).@"enum".fields) |field| { const shell = @field(Shell, field.name); - const result = try setup(alloc, ".", "sh", &env, shell, .{}); + const result = try setup( + alloc, + ".", + .{ .shell = "sh" }, + &env, + shell, + .{}, + ); try testing.expectEqual(shell, result.?.shell); } } @@ -215,25 +235,21 @@ test "setup features" { /// enables the integration or null if integration failed. fn setupBash( alloc: Allocator, - command: []const u8, + command: config.Command, resource_dir: []const u8, env: *EnvMap, -) !?[]const u8 { - // Accumulates the arguments that will form the final shell command line. - // We can build this list on the stack because we're just temporarily - // referencing other slices, but we can fall back to heap in extreme cases. - var args_alloc = std.heap.stackFallback(1024, alloc); - var args = try std.ArrayList([]const u8).initCapacity(args_alloc.get(), 2); +) !?config.Command { + var args = try std.ArrayList([:0]const u8).initCapacity(alloc, 2); defer args.deinit(); // Iterator that yields each argument in the original command line. // This will allocate once proportionate to the command line length. - var iter = try std.process.ArgIteratorGeneral(.{}).init(alloc, command); + var iter = try command.argIterator(alloc); defer iter.deinit(); // Start accumulating arguments with the executable and `--posix` mode flag. if (iter.next()) |exe| { - try args.append(exe); + try args.append(try alloc.dupeZ(u8, exe)); } else return null; try args.append("--posix"); @@ -267,17 +283,17 @@ fn setupBash( if (std.mem.indexOfScalar(u8, arg, 'c') != null) { return null; } - try args.append(arg); + try args.append(try alloc.dupeZ(u8, arg)); } else if (std.mem.eql(u8, arg, "-") or std.mem.eql(u8, arg, "--")) { // All remaining arguments should be passed directly to the shell // command. We shouldn't perform any further option processing. - try args.append(arg); + try args.append(try alloc.dupeZ(u8, arg)); while (iter.next()) |remaining_arg| { - try args.append(remaining_arg); + try args.append(try alloc.dupeZ(u8, remaining_arg)); } break; } else { - try args.append(arg); + try args.append(try alloc.dupeZ(u8, arg)); } } try env.put("GHOSTTY_BASH_INJECT", inject.slice()); @@ -310,30 +326,36 @@ fn setupBash( ); try env.put("ENV", integ_dir); - // Join the accumulated arguments to form the final command string. - return try std.mem.join(alloc, " ", args.items); + // Since we built up a command line, we don't need to wrap it in + // ANOTHER shell anymore and can do a direct command. + return .{ .direct = try args.toOwnedSlice() }; } test "bash" { const testing = std.testing; - const alloc = testing.allocator; + var arena = ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + const alloc = arena.allocator(); var env = EnvMap.init(alloc); defer env.deinit(); - const command = try setupBash(alloc, "bash", ".", &env); - defer if (command) |c| alloc.free(c); + const command = try setupBash(alloc, .{ .shell = "bash" }, ".", &env); - try testing.expectEqualStrings("bash --posix", command.?); + try testing.expectEqual(2, command.?.direct.len); + try testing.expectEqualStrings("bash", command.?.direct[0]); + try testing.expectEqualStrings("--posix", command.?.direct[1]); try testing.expectEqualStrings("./shell-integration/bash/ghostty.bash", env.get("ENV").?); try testing.expectEqualStrings("1", env.get("GHOSTTY_BASH_INJECT").?); } test "bash: unsupported options" { const testing = std.testing; - const alloc = testing.allocator; + var arena = ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + const alloc = arena.allocator(); - const cmdlines = [_][]const u8{ + const cmdlines = [_][:0]const u8{ "bash --posix", "bash --rcfile script.sh --posix", "bash --init-file script.sh --posix", @@ -345,7 +367,7 @@ test "bash: unsupported options" { var env = EnvMap.init(alloc); defer env.deinit(); - try testing.expect(try setupBash(alloc, cmdline, ".", &env) == null); + try testing.expect(try setupBash(alloc, .{ .shell = cmdline }, ".", &env) == null); try testing.expect(env.get("GHOSTTY_BASH_INJECT") == null); try testing.expect(env.get("GHOSTTY_BASH_RCFILE") == null); try testing.expect(env.get("GHOSTTY_BASH_UNEXPORT_HISTFILE") == null); @@ -354,17 +376,20 @@ test "bash: unsupported options" { test "bash: inject flags" { const testing = std.testing; - const alloc = testing.allocator; + var arena = ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + const alloc = arena.allocator(); // bash --norc { var env = EnvMap.init(alloc); defer env.deinit(); - const command = try setupBash(alloc, "bash --norc", ".", &env); - defer if (command) |c| alloc.free(c); + const command = try setupBash(alloc, .{ .shell = "bash --norc" }, ".", &env); - try testing.expectEqualStrings("bash --posix", command.?); + try testing.expectEqual(2, command.?.direct.len); + try testing.expectEqualStrings("bash", command.?.direct[0]); + try testing.expectEqualStrings("--posix", command.?.direct[1]); try testing.expectEqualStrings("1 --norc", env.get("GHOSTTY_BASH_INJECT").?); } @@ -373,52 +398,55 @@ test "bash: inject flags" { var env = EnvMap.init(alloc); defer env.deinit(); - const command = try setupBash(alloc, "bash --noprofile", ".", &env); - defer if (command) |c| alloc.free(c); + const command = try setupBash(alloc, .{ .shell = "bash --noprofile" }, ".", &env); - try testing.expectEqualStrings("bash --posix", command.?); + try testing.expectEqual(2, command.?.direct.len); + try testing.expectEqualStrings("bash", command.?.direct[0]); + try testing.expectEqualStrings("--posix", command.?.direct[1]); try testing.expectEqualStrings("1 --noprofile", env.get("GHOSTTY_BASH_INJECT").?); } } test "bash: rcfile" { const testing = std.testing; - const alloc = testing.allocator; + var arena = ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + const alloc = arena.allocator(); var env = EnvMap.init(alloc); defer env.deinit(); // bash --rcfile { - const command = try setupBash(alloc, "bash --rcfile profile.sh", ".", &env); - defer if (command) |c| alloc.free(c); - - try testing.expectEqualStrings("bash --posix", command.?); + const command = try setupBash(alloc, .{ .shell = "bash --rcfile profile.sh" }, ".", &env); + try testing.expectEqual(2, command.?.direct.len); + try testing.expectEqualStrings("bash", command.?.direct[0]); + try testing.expectEqualStrings("--posix", command.?.direct[1]); try testing.expectEqualStrings("profile.sh", env.get("GHOSTTY_BASH_RCFILE").?); } // bash --init-file { - const command = try setupBash(alloc, "bash --init-file profile.sh", ".", &env); - defer if (command) |c| alloc.free(c); - - try testing.expectEqualStrings("bash --posix", command.?); + const command = try setupBash(alloc, .{ .shell = "bash --init-file profile.sh" }, ".", &env); + try testing.expectEqual(2, command.?.direct.len); + try testing.expectEqualStrings("bash", command.?.direct[0]); + try testing.expectEqualStrings("--posix", command.?.direct[1]); try testing.expectEqualStrings("profile.sh", env.get("GHOSTTY_BASH_RCFILE").?); } } test "bash: HISTFILE" { const testing = std.testing; - const alloc = testing.allocator; + var arena = ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + const alloc = arena.allocator(); // HISTFILE unset { var env = EnvMap.init(alloc); defer env.deinit(); - const command = try setupBash(alloc, "bash", ".", &env); - defer if (command) |c| alloc.free(c); - + _ = try setupBash(alloc, .{ .shell = "bash" }, ".", &env); try testing.expect(std.mem.endsWith(u8, env.get("HISTFILE").?, ".bash_history")); try testing.expectEqualStrings("1", env.get("GHOSTTY_BASH_UNEXPORT_HISTFILE").?); } @@ -430,9 +458,7 @@ test "bash: HISTFILE" { try env.put("HISTFILE", "my_history"); - const command = try setupBash(alloc, "bash", ".", &env); - defer if (command) |c| alloc.free(c); - + _ = try setupBash(alloc, .{ .shell = "bash" }, ".", &env); try testing.expectEqualStrings("my_history", env.get("HISTFILE").?); try testing.expect(env.get("GHOSTTY_BASH_UNEXPORT_HISTFILE") == null); } @@ -440,25 +466,35 @@ test "bash: HISTFILE" { test "bash: additional arguments" { const testing = std.testing; - const alloc = testing.allocator; + var arena = ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + const alloc = arena.allocator(); var env = EnvMap.init(alloc); defer env.deinit(); // "-" argument separator { - const command = try setupBash(alloc, "bash - --arg file1 file2", ".", &env); - defer if (command) |c| alloc.free(c); - - try testing.expectEqualStrings("bash --posix - --arg file1 file2", command.?); + const command = try setupBash(alloc, .{ .shell = "bash - --arg file1 file2" }, ".", &env); + try testing.expectEqual(6, command.?.direct.len); + try testing.expectEqualStrings("bash", command.?.direct[0]); + try testing.expectEqualStrings("--posix", command.?.direct[1]); + try testing.expectEqualStrings("-", command.?.direct[2]); + try testing.expectEqualStrings("--arg", command.?.direct[3]); + try testing.expectEqualStrings("file1", command.?.direct[4]); + try testing.expectEqualStrings("file2", command.?.direct[5]); } // "--" argument separator { - const command = try setupBash(alloc, "bash -- --arg file1 file2", ".", &env); - defer if (command) |c| alloc.free(c); - - try testing.expectEqualStrings("bash --posix -- --arg file1 file2", command.?); + const command = try setupBash(alloc, .{ .shell = "bash -- --arg file1 file2" }, ".", &env); + try testing.expectEqual(6, command.?.direct.len); + try testing.expectEqualStrings("bash", command.?.direct[0]); + try testing.expectEqualStrings("--posix", command.?.direct[1]); + try testing.expectEqualStrings("--", command.?.direct[2]); + try testing.expectEqualStrings("--arg", command.?.direct[3]); + try testing.expectEqualStrings("file1", command.?.direct[4]); + try testing.expectEqualStrings("file2", command.?.direct[5]); } }