mirror/zig
1
0
Fork 0
mirror of https://github.com/ziglang/zig.git synced 2026-02-12 20:37:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

CLI: fix stdin dumping behavior

Browse Source 
* no need to move `tmpFilePath` around
 * no need for calculating max length of `FileExt` tag name
 * provide a canonical file extension name for `FileExt` so that, e.g.
   the file will be named `stdin.S` instead of
   `stdin.assembly_with_cpp`.
 * move temp file cleanup to a function to reduce defer bloat in a large
   function.
 * fix bug caused by mixing relative and absolute paths in the cleanup
   logic.
 * remove commented out test and dead code
This commit is contained in:
Andrew Kelley 2023-05-16 16:29:20 -07:00
parent 2e692312f1
commit ae119a9a8d
4 changed files with 62 additions and 134 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
lib/std/Build/Cache.zig
View File

@ -31,16 +31,6 @@ pub const Directory = struct {
}
}
pub fn tmpFilePath(self: Directory, ally: Allocator, suffix: []const u8) error{OutOfMemory}![]const u8 {
const s = std.fs.path.sep_str;
const rand_int = std.crypto.random.int(u64);
if (self.path) |p| {
return std.fmt.allocPrint(ally, "{s}" ++ s ++ "tmp" ++ s ++ "{x}-{s}", .{ p, rand_int, suffix });
} else {
return std.fmt.allocPrint(ally, "tmp" ++ s ++ "{x}-{s}", .{ rand_int, suffix });
}
}
/// Whether or not the handle should be closed, or the path should be freed
/// is determined by usage, however this function is provided for convenience
/// if it happens to be what the caller needs.

39
src/Compilation.zig
View File

@ -3981,7 +3981,7 @@ fn updateCObject(comp: *Compilation, c_object: *CObject, c_obj_prog_node: *std.P
// We can't know the digest until we do the C compiler invocation,
// so we need a temporary filename.
const out_obj_path = try comp.local_cache_directory.tmpFilePath(arena, o_basename);
const out_obj_path = try comp.tmpFilePath(arena, o_basename);
var zig_cache_tmp_dir = try comp.local_cache_directory.handle.makeOpenPath("tmp", .{});
defer zig_cache_tmp_dir.close();
@ -4129,6 +4129,16 @@ fn updateCObject(comp: *Compilation, c_object: *CObject, c_obj_prog_node: *std.P
};
}
pub fn tmpFilePath(comp: *Compilation, ally: Allocator, suffix: []const u8) error{OutOfMemory}![]const u8 {
const s = std.fs.path.sep_str;
const rand_int = std.crypto.random.int(u64);
if (comp.local_cache_directory.path) |p| {
return std.fmt.allocPrint(ally, "{s}" ++ s ++ "tmp" ++ s ++ "{x}-{s}", .{ p, rand_int, suffix });
} else {
return std.fmt.allocPrint(ally, "tmp" ++ s ++ "{x}-{s}", .{ rand_int, suffix });
}
}
pub fn addTranslateCCArgs(
comp: *Compilation,
arena: Allocator,
@ -4588,13 +4598,26 @@ pub const FileExt = enum {
};
}
// maximum length of @tagName(ext: FileExt)
pub const max_len = blk: {
var max: u16 = 0;
inline for (std.meta.tags(FileExt)) |ext|
max = std.math.max(@tagName(ext).len, max);
break :blk max;
};
pub fn canonicalName(ext: FileExt, target: Target) [:0]const u8 {
return switch (ext) {
.c => ".c",
.cpp => ".cpp",
.cu => ".cu",
.h => ".h",
.m => ".m",
.mm => ".mm",
.ll => ".ll",
.bc => ".bc",
.assembly => ".s",
.assembly_with_cpp => ".S",
.shared_library => target.dynamicLibSuffix(),
.object => target.ofmt.fileExt(target.cpu.arch),
.static_library => target.staticLibSuffix(),
.zig => ".zig",
.def => ".def",
.unknown => "",
};
}
};
pub fn hasObjectExt(filename: []const u8) bool {

54
src/main.zig
View File

@ -705,6 +705,17 @@ const ArgsIterator = struct {
}
};
fn cleanupTempStdinFile(
temp_stdin_file: ?[]const u8,
local_cache_directory: Compilation.Directory,
) void {
if (temp_stdin_file) |file| {
// Some garbage may stay in the file system if removal fails; this
// is harmless so no warning is needed.
local_cache_directory.handle.deleteFile(file) catch {};
}
}
fn buildOutputType(
gpa: Allocator,
arena: Allocator,
@ -3021,15 +3032,11 @@ fn buildOutputType(
};
var temp_stdin_file: ?[]const u8 = null;
defer {
if (temp_stdin_file) |file| {
// some garbage may stay in the file system if removal fails.
// Alternatively, we could tell the user that the removal failed,
// but it's not as much of a deal: it's a temporary cache directory
// at all.
local_cache_directory.handle.deleteFile(file) catch {};
}
}
// Note that in one of the happy paths, execve() is used to switch to clang
// in which case this cleanup logic does not run and this temp file is
// leaked. Oh well. It's a minor punishment for using `-x c` which nobody
// should be doing.
defer cleanupTempStdinFile(temp_stdin_file, local_cache_directory);
for (c_source_files.items) |*src| {
if (!mem.eql(u8, src.src_path, "-")) continue;
@ -3038,21 +3045,22 @@ fn buildOutputType(
fatal("-E or -x is required when reading from a non-regular file", .{});
// "-" is stdin. Dump it to a real file.
const new_file = blk: {
var buf: ["stdin.".len + Compilation.FileExt.max_len]u8 = undefined;
const fname = try std.fmt.bufPrint(&buf, "stdin.{s}", .{@tagName(ext)});
const new_name = try local_cache_directory.tmpFilePath(arena, fname);
const sub_path = blk: {
const sep = fs.path.sep_str;
const sub_path = try std.fmt.allocPrint(arena, "tmp" ++ sep ++ "{x}-stdin{s}", .{
std.crypto.random.int(u64), ext.canonicalName(target_info.target),
});
try local_cache_directory.handle.makePath("tmp");
var outfile = try local_cache_directory.handle.createFile(new_name, .{});
defer outfile.close();
errdefer local_cache_directory.handle.deleteFile(new_name) catch {};
try outfile.writeFileAll(io.getStdIn(), .{});
break :blk new_name;
var f = try local_cache_directory.handle.createFile(sub_path, .{});
defer f.close();
errdefer local_cache_directory.handle.deleteFile(sub_path) catch {};
try f.writeFileAll(io.getStdIn(), .{});
break :blk sub_path;
};
temp_stdin_file = new_file;
src.src_path = new_file;
// Relative to `local_cache_directory`.
temp_stdin_file = sub_path;
// Relative to current working directory.
src.src_path = try local_cache_directory.join(arena, &.{sub_path});
}
if (build_options.have_llvm and emit_asm != .no) {
@ -3908,7 +3916,7 @@ fn cmdTranslateC(comp: *Compilation, arena: Allocator, fancy_output: ?*Translate
const c_src_basename = fs.path.basename(c_source_file.src_path);
const dep_basename = try std.fmt.allocPrint(arena, "{s}.d", .{c_src_basename});
const out_dep_path = try comp.local_cache_directory.tmpFilePath(arena, dep_basename);
const out_dep_path = try comp.tmpFilePath(arena, dep_basename);
break :blk out_dep_path;
};

93
test/tests.zig
View File

@ -777,52 +777,6 @@ pub fn addCliTests(b: *std.Build) *Step {
step.dependOn(&cleanup.step);
}
// Test `zig cc -x c -`
// Test author was not able to figure out how to start a child process and
// give an fd to it's stdin. fork/exec works, but we are limiting ourselves
// to POSIX.
//
// TODO: the "zig cc <..." step should be a RunStep.create(...)
// However, how do I create a command (RunStep) that invokes a command
// with a specific file descriptor in the stdin?
//if (builtin.os.tag != .windows) {
// const tmp_path = b.makeTempPath();
// var dir = std.fs.cwd().openDir(tmp_path, .{}) catch @panic("unhandled");
// dir.writeFile("truth.c", "int main() { return 42; }") catch @panic("unhandled");
// var infile = dir.openFile("truth.c", .{}) catch @panic("unhandled");
// const outfile = std.fs.path.joinZ(
// b.allocator,
// &[_][]const u8{ tmp_path, "truth" },
// ) catch @panic("unhandled");
// const pid_result = std.os.fork() catch @panic("unhandled");
// if (pid_result == 0) { // child
// std.os.dup2(infile.handle, std.os.STDIN_FILENO) catch @panic("unhandled");
// const argv = &[_:null]?[*:0]const u8{
// b.zig_exe, "cc",
// "-o", outfile,
// "-x", "c",
// "-",
// };
// const envp = &[_:null]?[*:0]const u8{
// std.fmt.allocPrintZ(b.allocator, "ZIG_GLOBAL_CACHE_DIR={s}", .{tmp_path}) catch @panic("unhandled"),
// };
// const err = std.os.execveZ(b.zig_exe, argv, envp);
// std.debug.print("execve error: {any}\n", .{err});
// std.os.exit(1);
// }
// const res = std.os.waitpid(pid_result, 0);
// assert(0 == res.status);
// // run the compiled executable and check if it's telling the truth.
// _ = exec(b.allocator, tmp_path, 42, &[_][]const u8{outfile}) catch @panic("unhandled");
// const cleanup = b.addRemoveDirTree(tmp_path);
// step.dependOn(&cleanup.step);
//}
{
// Test `zig fmt`.
// This test must use a temporary directory rather than a cache
@ -1200,50 +1154,3 @@ pub fn addCases(
check_case_exe,
);
}
fn exec(
allocator: std.mem.Allocator,
cwd: []const u8,
expect_code: u8,
argv: []const []const u8,
) !std.ChildProcess.ExecResult {
const max_output_size = 100 * 1024;
const result = std.ChildProcess.exec(.{
.allocator = allocator,
.argv = argv,
.cwd = cwd,
.max_output_bytes = max_output_size,
}) catch |err| {
std.debug.print("The following command failed:\n", .{});
printCmd(cwd, argv);
return err;
};
switch (result.term) {
.Exited => |code| {
if (code != expect_code) {
std.debug.print(
"The following command exited with error code {}, expected {}:\n",
.{ code, expect_code },
);
printCmd(cwd, argv);
std.debug.print("stderr:\n{s}\n", .{result.stderr});
return error.CommandFailed;
}
},
else => {
std.debug.print("The following command terminated unexpectedly:\n", .{});
printCmd(cwd, argv);
std.debug.print("stderr:\n{s}\n", .{result.stderr});
return error.CommandFailed;
},
}
return result;
}
fn printCmd(cwd: []const u8, argv: []const []const u8) void {
std.debug.print("cd {s} && ", .{cwd});
for (argv) |arg| {
std.debug.print("{s} ", .{arg});
}
std.debug.print("\n", .{});
}