Commit 43c2ce10a1

Andrew Kelley <andrew@ziglang.org>
2020-10-16 02:13:32
fixups
* extract logic into a `os_can_execve` and use it in the other place that we execve * outdent some code by introducing `run_or_test` variable * delete unnecessary and wasteful memory management logic * better error message for when execve fails * add comment to explain why we do not execve for `zig test`
1 parent 1c36680
Changed files (1)
src/main.zig
@@ -2,7 +2,6 @@ const std = @import("std");
 const assert = std.debug.assert;
 const io = std.io;
 const fs = std.fs;
-const os = std.os;
 const mem = std.mem;
 const process = std.process;
 const Allocator = mem.Allocator;
@@ -115,15 +114,15 @@ pub fn main() anyerror!void {
     return mainArgs(gpa, arena, args);
 }
 
+const os_can_execve = std.builtin.os.tag != .windows;
+
 pub fn mainArgs(gpa: *Allocator, arena: *Allocator, args: []const []const u8) !void {
     if (args.len <= 1) {
         std.log.info("{}", .{usage});
         fatal("expected command argument", .{});
     }
 
-    if (std.Target.current.os.tag != .windows and
-        std.os.getenvZ("ZIG_IS_DETECTING_LIBC_PATHS") != null)
-    {
+    if (os_can_execve and std.os.getenvZ("ZIG_IS_DETECTING_LIBC_PATHS") != null) {
         // In this case we have accidentally invoked ourselves as "the system C compiler"
         // to figure out where libc is installed. This is essentially infinite recursion
         // via child process execution due to the CC environment variable pointing to Zig.
@@ -1711,87 +1710,91 @@ fn buildOutputType(
         warn("--watch is not recommended with the stage1 backend; it leaks memory and is not capable of incremental compilation", .{});
     }
 
-    switch (arg_mode) {
-        .run, .zig_test => run: {
-            const exe_loc = emit_bin_loc orelse break :run;
-            const exe_directory = exe_loc.directory orelse comp.bin_file.options.emit.?.directory;
-            const exe_path = try fs.path.join(arena, &[_][]const u8{
-                exe_directory.path orelse ".", exe_loc.basename,
-            });
-
-            var argv = std.ArrayList([]const u8).init(gpa);
-            defer argv.deinit();
-
-            if (test_exec_args.items.len == 0) {
-                if (!std.Target.current.canExecBinariesOf(target_info.target)) {
-                    switch (arg_mode) {
-                        .zig_test => {
-                            warn("created {s} but skipping execution because it is non-native", .{exe_path});
-                            if (!watch) return cleanExit();
-                            break :run;
-                        },
-                        .run => fatal("unable to execute {s}: non-native", .{exe_path}),
-                        else => unreachable,
-                    }
-                }
-                try argv.append(exe_path);
-            } else {
-                for (test_exec_args.items) |arg| {
-                    try argv.append(arg orelse exe_path);
-                }
-            }
-            if (runtime_args_start) |i| {
-                try argv.appendSlice(all_args[i..]);
-            }
-            if (std.builtin.os.tag != .windows and arg_mode == .run and !watch) {
-                var env_vars = try process.getEnvMap(gpa);
-                const err = os.execvpe(gpa, argv.items, &env_vars);
-                env_vars.deinit(); // it would cause a memory leak because a defer would be unreachable because of fatal
-                fatal("There was an error with `zig run`: {}", .{@errorName(err)});
-            } else {
-                const child = try std.ChildProcess.init(argv.items, gpa);
-                defer child.deinit();
+    const run_or_test = switch (arg_mode) {
+        .run, .zig_test => true,
+        else => false,
+    };
+    if (run_or_test) run: {
+        const exe_loc = emit_bin_loc orelse break :run;
+        const exe_directory = exe_loc.directory orelse comp.bin_file.options.emit.?.directory;
+        const exe_path = try fs.path.join(arena, &[_][]const u8{
+            exe_directory.path orelse ".", exe_loc.basename,
+        });
 
-                child.stdin_behavior = .Inherit;
-                child.stdout_behavior = .Inherit;
-                child.stderr_behavior = .Inherit;
+        var argv = std.ArrayList([]const u8).init(gpa);
+        defer argv.deinit();
 
-                const term = try child.spawnAndWait();
+        if (test_exec_args.items.len == 0) {
+            if (!std.Target.current.canExecBinariesOf(target_info.target)) {
                 switch (arg_mode) {
-                    .run => {
-                        switch (term) {
-                            .Exited => |code| {
-                                if (code == 0) {
-                                    if (!watch) return cleanExit();
-                                } else {
-                                    // TODO https://github.com/ziglang/zig/issues/6342
-                                    process.exit(1);
-                                }
-                            },
-                            else => process.exit(1),
-                        }
-                    },
                     .zig_test => {
-                        switch (term) {
-                            .Exited => |code| {
-                                if (code == 0) {
-                                    if (!watch) return cleanExit();
-                                } else {
-                                    const cmd = try argvCmd(arena, argv.items);
-                                    fatal("the following test command failed with exit code {}:\n{}", .{ code, cmd });
-                                }
-                            },
-                            else => {
-                                const cmd = try argvCmd(arena, argv.items);
-                                fatal("the following test command crashed:\n{}", .{cmd});
-                            },
-                        }
+                        warn("created {s} but skipping execution because it is non-native", .{exe_path});
+                        if (!watch) return cleanExit();
+                        break :run;
                     },
+                    .run => fatal("unable to execute {s}: non-native", .{exe_path}),
                     else => unreachable,
                 }
             }
-        },
-        else => {},
+            try argv.append(exe_path);
+        } else {
+            for (test_exec_args.items) |arg| {
+                try argv.append(arg orelse exe_path);
+            }
+        }
+        if (runtime_args_start) |i| {
+            try argv.appendSlice(all_args[i..]);
+        }
+        // We do not execve for tests because if the test fails we want to print the error message and
+        // invocation below.
+        if (os_can_execve and arg_mode == .run and !watch) {
+            // TODO improve the std lib so that we don't need a call to getEnvMap here.
+            var env_vars = try process.getEnvMap(arena);
+            const err = std.os.execvpe(gpa, argv.items, &env_vars);
+            const cmd = try argvCmd(arena, argv.items);
+            fatal("the following command failed to execve with '{s}':\n{s}", .{ @errorName(err), cmd });
+        } else {
+            const child = try std.ChildProcess.init(argv.items, gpa);
+            defer child.deinit();
+
+            child.stdin_behavior = .Inherit;
+            child.stdout_behavior = .Inherit;
+            child.stderr_behavior = .Inherit;
+
+            const term = try child.spawnAndWait();
+            switch (arg_mode) {
+                .run => {
+                    switch (term) {
+                        .Exited => |code| {
+                            if (code == 0) {
+                                if (!watch) return cleanExit();
+                            } else {
+                                // TODO https://github.com/ziglang/zig/issues/6342
+                                process.exit(1);
+                            }
+                        },
+                        else => process.exit(1),
+                    }
+                },
+                .zig_test => {
+                    switch (term) {
+                        .Exited => |code| {
+                            if (code == 0) {
+                                if (!watch) return cleanExit();
+                            } else {
+                                const cmd = try argvCmd(arena, argv.items);
+                                fatal("the following test command failed with exit code {}:\n{}", .{ code, cmd });
+                            }
+                        },
+                        else => {
+                            const cmd = try argvCmd(arena, argv.items);
+                            fatal("the following test command crashed:\n{}", .{cmd});
+                        },
+                    }
+                },
+                else => unreachable,
+            }
+        }
     }
 
     const stdin = std.io.getStdIn().inStream();