fix argv parsing on windows (#8458)

* fix argv parsing on windows

* directly use zig stdlib

* remove debug comments, fix double deinit

* change bun.argv() to return slices, not null-terminated pointers

* fix test on windows to escape file paths correctly

---------

Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
This commit is contained in:
Georgijs
2024-01-24 20:54:01 -08:00
committed by GitHub
parent 0bf8a25d50
commit 80bd3254cc
12 changed files with 49 additions and 59 deletions

View File

@@ -2414,7 +2414,7 @@ pub const Path = struct {
pub const Process = struct {
pub fn getArgv0(globalObject: *JSC.JSGlobalObject) callconv(.C) JSC.JSValue {
return JSC.ZigString.fromUTF8(bun.span(bun.argv()[0])).toValueGC(globalObject);
return JSC.ZigString.fromUTF8(bun.argv()[0]).toValueGC(globalObject);
}
pub fn getExecPath(globalObject: *JSC.JSGlobalObject) callconv(.C) JSC.JSValue {
@@ -2452,8 +2452,7 @@ pub const Process = struct {
var used: usize = 0;
const offset: usize = 1;
for (bun.argv()[@min(bun.argv().len, offset)..]) |arg_| {
const arg = bun.span(arg_);
for (bun.argv()[@min(bun.argv().len, offset)..]) |arg| {
if (arg.len == 0)
continue;

View File

@@ -1374,7 +1374,7 @@ pub fn reloadProcess(
const bun = @This();
const dupe_argv = allocator.allocSentinel(?[*:0]const u8, bun.argv().len, null) catch unreachable;
for (bun.argv(), dupe_argv) |src, *dest| {
dest.* = (allocator.dupeZ(u8, sliceTo(src, 0)) catch unreachable).ptr;
dest.* = (allocator.dupeZ(u8, src) catch unreachable).ptr;
}
const environ_slice = std.mem.span(std.c.environ);
@@ -1391,7 +1391,7 @@ pub fn reloadProcess(
const exec_path = (allocator.dupeZ(u8, std.fs.selfExePathAlloc(allocator) catch unreachable) catch unreachable).ptr;
// we clone argv so that the memory address isn't the same as the libc one
const argv = @as([*:null]?[*:0]const u8, @ptrCast(dupe_argv.ptr));
const newargv = @as([*:null]?[*:0]const u8, @ptrCast(dupe_argv.ptr));
// we clone envp so that the memory address of environment variables isn't the same as the libc one
const envp = @as([*:null]?[*:0]const u8, @ptrCast(environ.ptr));
@@ -1419,7 +1419,7 @@ pub fn reloadProcess(
C.POSIX_SPAWN_SETEXEC |
C.POSIX_SPAWN_SETSIGDEF | C.POSIX_SPAWN_SETSIGMASK,
) catch unreachable;
switch (PosixSpawn.spawnZ(exec_path, actions, attrs, @as([*:null]?[*:0]const u8, @ptrCast(argv)), @as([*:null]?[*:0]const u8, @ptrCast(envp)))) {
switch (PosixSpawn.spawnZ(exec_path, actions, attrs, @as([*:null]?[*:0]const u8, @ptrCast(newargv)), @as([*:null]?[*:0]const u8, @ptrCast(envp)))) {
.err => |err| {
Output.panic("Unexpected error while reloading: {d} {s}", .{ err.errno, @tagName(err.getErrno()) });
},
@@ -1428,7 +1428,7 @@ pub fn reloadProcess(
} else {
const err = std.os.execveZ(
exec_path,
argv,
newargv,
envp,
);
Output.panic("Unexpected error while reloading: {s}", .{@errorName(err)});
@@ -1811,18 +1811,21 @@ const WindowsStat = extern struct {
pub const Stat = if (Environment.isWindows) windows.libuv.uv_stat_t else std.os.Stat;
var _argv: [][:0]u8 = &[_][:0]u8{};
pub inline fn argv() [][:0]u8 {
return _argv;
}
pub fn initArgv(allocator: std.mem.Allocator) !void {
_argv = try std.process.argsAlloc(allocator);
}
pub const posix = struct {
pub const STDIN_FD = toFD(0);
pub const STDOUT_FD = toFD(1);
pub const STDERR_FD = toFD(2);
pub inline fn argv() [][*:0]u8 {
return std.os.argv;
}
pub inline fn setArgv(new_ptr: [][*:0]u8) void {
std.os.argv = new_ptr;
}
pub fn stdio(i: anytype) FileDescriptor {
return switch (i) {
1 => STDOUT_FD,
@@ -1840,14 +1843,6 @@ pub const win32 = struct {
pub var STDERR_FD: FileDescriptor = undefined;
pub var STDIN_FD: FileDescriptor = undefined;
pub inline fn argv() [][*:0]u8 {
return std.os.argv;
}
pub inline fn setArgv(new_ptr: [][*:0]u8) void {
std.os.argv = new_ptr;
}
pub fn stdio(i: anytype) FileDescriptor {
return switch (i) {
0 => STDIN_FD,

View File

@@ -1131,7 +1131,7 @@ pub const Command = struct {
// std.process.args allocates!
const ArgsIterator = struct {
buf: [][*:0]u8 = undefined,
buf: [][:0]u8 = undefined,
i: u32 = 0,
pub fn next(this: *ArgsIterator) ?[]const u8 {
@@ -1140,7 +1140,7 @@ pub const Command = struct {
}
const i = this.i;
this.i += 1;
return std.mem.span(this.buf[i]);
return this.buf[i];
}
pub fn skip(this: *ArgsIterator) bool {
@@ -1197,8 +1197,7 @@ pub const Command = struct {
RootCommandMatcher.case("install"),
=> brk: {
for (args_iter.buf) |arg| {
const span = std.mem.span(arg);
if (span.len > 0 and (strings.eqlComptime(span, "-g") or strings.eqlComptime(span, "--global"))) {
if (arg.len > 0 and (strings.eqlComptime(arg, "-g") or strings.eqlComptime(arg, "--global"))) {
break :brk .AddCommand;
}
}
@@ -1292,13 +1291,11 @@ pub const Command = struct {
};
ctx.args.target = Api.Target.bun;
const argv = try bun.default_allocator.alloc(string, bun.argv().len -| 1);
if (bun.argv().len > 1) {
for (argv, bun.argv()[1..]) |*dest, src| {
dest.* = bun.span(src);
}
ctx.passthrough = bun.argv()[1..];
} else {
ctx.passthrough = &[_]string{};
}
ctx.passthrough = argv;
try @import("./bun_js.zig").Run.bootStandalone(
ctx,
@@ -1510,8 +1507,7 @@ pub const Command = struct {
// iterate over args
// if --help, print help and exit
const print_help = brk: {
for (bun.argv()) |arg_| {
const arg = bun.span(arg_);
for (bun.argv()) |arg| {
if (strings.eqlComptime(arg, "--help")) {
break :brk true;
}
@@ -1595,7 +1591,7 @@ pub const Command = struct {
example_tag != CreateCommandExample.Tag.local_folder;
if (use_bunx) {
const bunx_args = try allocator.alloc([*:0]const u8, args.len - template_name_start);
const bunx_args = try allocator.alloc([:0]const u8, args.len - template_name_start);
bunx_args[0] = try BunxCommand.addCreatePrefix(allocator, template_name);
for (bunx_args[1..], args[template_name_start + 1 ..]) |*dest, src| {
dest.* = src;
@@ -1663,7 +1659,7 @@ pub const Command = struct {
if (ctx.args.entry_points.len == 1) {
if (strings.eqlComptime(extension, ".lockb")) {
for (bun.argv()) |arg| {
if (strings.eqlComptime(std.mem.span(arg), "--hash")) {
if (strings.eqlComptime(arg, "--hash")) {
try PackageManagerCommand.printHash(ctx, ctx.args.entry_points[0]);
return;
}

View File

@@ -162,7 +162,7 @@ pub const BunxCommand = struct {
Global.exit(1);
}
pub fn exec(ctx_: bun.CLI.Command.Context, argv: [][*:0]const u8) !void {
pub fn exec(ctx_: bun.CLI.Command.Context, argv: [][:0]const u8) !void {
var ctx = ctx_;
var requests_buf = bun.PackageManager.UpdateRequest.Array.init(0) catch unreachable;
var run_in_bun = ctx.debug.run_in_bun;
@@ -172,9 +172,7 @@ pub const BunxCommand = struct {
{
var found_subcommand_name = false;
for (argv) |positional_| {
const positional = bun.span(positional_);
for (argv) |positional| {
if (positional.len == 0) continue;
if (positional[0] != '-') {

View File

@@ -89,10 +89,9 @@ pub const InitCommand = struct {
entry_point: string = "",
};
pub fn exec(alloc: std.mem.Allocator, argv: [][*:0]u8) !void {
pub fn exec(alloc: std.mem.Allocator, argv: [][:0]u8) !void {
const print_help = brk: {
for (argv) |arg_| {
const arg = bun.span(arg_);
for (argv) |arg| {
if (strings.eqlComptime(arg, "--help")) {
break :brk true;
}

View File

@@ -138,9 +138,9 @@ pub const InstallCompletionsCommand = struct {
var completions_dir: string = "";
var output_dir: std.fs.Dir = found: {
for (bun.argv(), 0..) |arg, i| {
if (strings.eqlComptime(std.mem.span(arg), "completions")) {
if (strings.eqlComptime(arg, "completions")) {
if (bun.argv().len > i + 1) {
const input = std.mem.span(bun.argv()[i + 1]);
const input = bun.argv()[i + 1];
if (!std.fs.path.isAbsolute(input)) {
completions_dir = resolve_path.joinAbs(

View File

@@ -510,7 +510,7 @@ pub const RunCommand = struct {
// if we are already an absolute path, use that
// if the user started the application via a shebang, it's likely that the path is absolute already
if (bun.argv()[0][0] == '/') {
optional_bun_path.* = bun.span(bun.argv()[0]);
optional_bun_path.* = bun.argv()[0];
argv0 = bun.argv()[0];
} else if (optional_bun_path.len == 0) {
// otherwise, ask the OS for the absolute path
@@ -1176,7 +1176,7 @@ pub const RunCommand = struct {
shebang = std.mem.trim(u8, shebang, " \r\n\t");
if (strings.hasPrefixComptime(shebang, "#!")) {
const first_arg: string = if (bun.argv().len > 0) bun.span(bun.argv()[0]) else "";
const first_arg: string = if (bun.argv().len > 0) bun.argv()[0] else "";
const filename = std.fs.path.basename(first_arg);
// are we attempting to run the script with bun?
if (!strings.contains(shebang, filename)) {

View File

@@ -438,14 +438,14 @@ pub const UpgradeCommand = struct {
const use_canary = brk: {
const default_use_canary = Environment.is_canary;
if (default_use_canary and strings.containsAny(bun.span(bun.argv()), "--stable"))
if (default_use_canary and strings.containsAny(bun.argv(), "--stable"))
break :brk false;
break :brk strings.eqlComptime(env_loader.map.get("BUN_CANARY") orelse "0", "1") or
strings.containsAny(bun.span(bun.argv()), "--canary") or default_use_canary;
strings.containsAny(bun.argv(), "--canary") or default_use_canary;
};
const use_profile = strings.containsAny(bun.span(bun.argv()), "--profile");
const use_profile = strings.containsAny(bun.argv(), "--profile");
if (!use_canary) {
var refresher = std.Progress{};

View File

@@ -50,7 +50,7 @@ pub const OsIterator = struct {
const Error = process.ArgIterator.InitError;
arena: @import("root").bun.ArenaAllocator,
remain: [][*:0]u8,
remain: [][:0]u8,
/// The executable path (this is the first argument passed to the program)
/// TODO: Is it the right choice for this to be null? Maybe `init` should
@@ -73,7 +73,7 @@ pub const OsIterator = struct {
pub fn next(iter: *OsIterator) ?[:0]const u8 {
if (iter.remain.len > 0) {
const res = bun.sliceTo(iter.remain[0], 0);
const res = iter.remain[0];
iter.remain = iter.remain[1..];
return res;
}

View File

@@ -26,6 +26,10 @@ pub fn main() void {
const Output = bun.Output;
const Environment = bun.Environment;
bun.initArgv(bun.default_allocator) catch |err| {
Output.panic("Failed to initialize argv: {s}\n", .{@errorName(err)});
};
if (Environment.isRelease and Environment.isPosix)
CrashReporter.start() catch unreachable;
if (Environment.isWindows) {

View File

@@ -576,11 +576,10 @@ pub const StandaloneModuleGraph = struct {
// and also just makes sense.
const argv = bun.argv();
if (argv.len > 0) {
const argv0_len = bun.len(argv[0]);
if (argv0_len > 0) {
const argv0 = argv[0][0..argv0_len];
if (argv0_len == 3) {
// const argv0_len = bun.len(argv[0]);
const argv0 = argv[0];
if (argv0.len > 0) {
if (argv0.len == 3) {
if (bun.strings.eqlComptimeIgnoreLen(argv0, "bun")) {
return null;
}
@@ -592,7 +591,7 @@ pub const StandaloneModuleGraph = struct {
}
}
if (argv0_len == 4) {
if (argv0.len == 4) {
if (bun.strings.eqlComptimeIgnoreLen(argv0, "bunx")) {
return null;
}
@@ -617,7 +616,7 @@ pub const StandaloneModuleGraph = struct {
&whichbuf,
bun.getenvZ("PATH") orelse return error.FileNotFound,
"",
bun.span(bun.argv()[0]),
bun.argv()[0],
)) |path| {
return bun.toFD((try std.fs.cwd().openFileZ(path, flags)).handle);
}

View File

@@ -76,7 +76,7 @@ test("latin1 & utf16 imports", async () => {
mkdirSync(prefix, { recursive: true });
} catch (e) {}
const entryCode = imports.map(i => `import "${i}";`).join("\n");
let entryCode = imports.map(i => `import ${JSON.stringify(i)};`).join("\n");
await Bun.write(inputPath, entryCode);
for (let importPath of imports) {