From 9f8f4464353460825856b848dfe2480de20d8d55 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Fri, 9 Oct 2020 16:45:39 -0700 Subject: [PATCH] fixups to previous commit * std.fs.Dir.readFile: add doc comments to explain what it means when the returned slice has the same length as the supplied buffer. * introduce readSmallFile / writeSmallFile to abstract over the decision to use symlink or file contents to store data. --- lib/std/fs.zig | 6 +++++- src/Cache.zig | 24 ++++++++++++++++++++++++ src/Compilation.zig | 14 +++++++++----- src/link.zig | 8 ++++++-- src/link/Coff.zig | 12 ++++++++---- src/link/Elf.zig | 12 ++++++++---- src/link/MachO.zig | 12 ++++++++---- src/link/Wasm.zig | 12 ++++++++---- 8 files changed, 76 insertions(+), 24 deletions(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index bebc954746..0f8c6e0d24 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -1490,7 +1490,11 @@ pub const Dir = struct { return os.windows.ReadLink(self.fd, sub_path_w, buffer); } - /// Read all of file contents using a preallocated buffer + /// Read all of file contents using a preallocated buffer. + /// The returned slice has the same pointer as `buffer`. If the length matches `buffer.len` + /// the situation is ambiguous. It could either mean that the entire file was read, and + /// it exactly fits the buffer, or it could mean the buffer was not big enough for the + /// entire file. pub fn readFile(self: Dir, file_path: []const u8, buffer: []u8) ![]u8 { var file = try self.openFile(file_path, .{}); defer file.close(); diff --git a/src/Cache.zig b/src/Cache.zig index dff6f7e38e..1adf72d16f 100644 --- a/src/Cache.zig +++ b/src/Cache.zig @@ -576,6 +576,30 @@ pub const Manifest = struct { } }; +/// On operating systems that support symlinks, does a readlink. On other operating systems, +/// uses the file contents. Windows supports symlinks but only with elevated privileges, so +/// it is treated as not supporting symlinks. +pub fn readSmallFile(dir: fs.Dir, sub_path: []const u8, buffer: []u8) ![]u8 { + if (std.Target.current.os.tag == .windows) { + return dir.readFile(sub_path, buffer); + } else { + return dir.readLink(sub_path, buffer); + } +} + +/// On operating systems that support symlinks, does a symlink. On other operating systems, +/// uses the file contents. Windows supports symlinks but only with elevated privileges, so +/// it is treated as not supporting symlinks. +/// `data` must be a valid UTF-8 encoded file path and 255 bytes or fewer. +pub fn writeSmallFile(dir: fs.Dir, sub_path: []const u8, data: []const u8) !void { + assert(data.len <= 255); + if (std.Target.current.os.tag == .windows) { + return dir.writeFile(sub_path, data); + } else { + return dir.symLink(data, sub_path, .{}); + } +} + fn hashFile(file: fs.File, bin_digest: []u8) !void { var buf: [1024]u8 = undefined; diff --git a/src/Compilation.zig b/src/Compilation.zig index 0bff2372d9..1fddda42f9 100644 --- a/src/Compilation.zig +++ b/src/Compilation.zig @@ -2605,8 +2605,12 @@ fn updateStage1Module(comp: *Compilation, main_progress_node: *std.Progress.Node // We use an extra hex-encoded byte here to store some flags. var prev_digest_buf: [digest.len + 2]u8 = undefined; - const prev_digest: []u8 = directory.handle.readFile(id_symlink_basename, &prev_digest_buf) catch |err| blk: { - log.debug("stage1 {} new_digest={} readFile error: {}", .{ mod.root_pkg.root_src_path, digest, @errorName(err) }); + const prev_digest: []u8 = Cache.readSmallFile( + directory.handle, + id_symlink_basename, + &prev_digest_buf, + ) catch |err| blk: { + log.debug("stage1 {} new_digest={} error: {}", .{ mod.root_pkg.root_src_path, digest, @errorName(err) }); // Handle this as a cache miss. break :blk prev_digest_buf[0..0]; }; @@ -2777,7 +2781,7 @@ fn updateStage1Module(comp: *Compilation, main_progress_node: *std.Progress.Node const digest = man.final(); - // Update the dangling symlink with the digest. If it fails we can continue; it only + // Update the small file with the digest. If it fails we can continue; it only // means that the next invocation will have an unnecessary cache miss. const stage1_flags_byte = @bitCast(u8, mod.stage1_flags); log.debug("stage1 {} final digest={} flags={x}", .{ @@ -2792,10 +2796,10 @@ fn updateStage1Module(comp: *Compilation, main_progress_node: *std.Progress.Node log.debug("saved digest + flags: '{s}' (byte = {}) have_winmain_crt_startup={}", .{ digest_plus_flags, stage1_flags_byte, mod.stage1_flags.have_winmain_crt_startup, }); - directory.handle.writeFile(id_symlink_basename, &digest_plus_flags) catch |err| { + Cache.writeSmallFile(directory.handle, id_symlink_basename, &digest_plus_flags) catch |err| { log.warn("failed to save stage1 hash digest file: {}", .{@errorName(err)}); }; - // Again failure here only means an unnecessary cache miss. + // Failure here only means an unnecessary cache miss. man.writeManifest() catch |err| { log.warn("failed to write cache manifest when linking: {}", .{@errorName(err)}); }; diff --git a/src/link.zig b/src/link.zig index cd41c54cd4..b7b891b5b1 100644 --- a/src/link.zig +++ b/src/link.zig @@ -466,7 +466,11 @@ pub const File = struct { const digest = ch.final(); var prev_digest_buf: [digest.len]u8 = undefined; - const prev_digest: []u8 = directory.handle.readFile(id_symlink_basename, &prev_digest_buf) catch |err| b: { + const prev_digest: []u8 = Cache.readSmallFile( + directory.handle, + id_symlink_basename, + &prev_digest_buf, + ) catch |err| b: { log.debug("archive new_digest={} readFile error: {}", .{ digest, @errorName(err) }); break :b prev_digest_buf[0..0]; }; @@ -512,7 +516,7 @@ pub const File = struct { const bad = llvm.WriteArchive(full_out_path_z, object_files.items.ptr, object_files.items.len, os_type); if (bad) return error.UnableToWriteArchive; - directory.handle.writeFile(id_symlink_basename, &digest) catch |err| { + Cache.writeSmallFile(directory.handle, id_symlink_basename, &digest) catch |err| { std.log.warn("failed to save archive hash digest file: {}", .{@errorName(err)}); }; diff --git a/src/link/Coff.zig b/src/link/Coff.zig index 5c7975743f..492dbfc8eb 100644 --- a/src/link/Coff.zig +++ b/src/link/Coff.zig @@ -854,8 +854,12 @@ fn linkWithLLD(self: *Coff, comp: *Compilation) !void { _ = try man.hit(); digest = man.final(); var prev_digest_buf: [digest.len]u8 = undefined; - const prev_digest: []u8 = directory.handle.readFile(id_symlink_basename, &prev_digest_buf) catch |err| blk: { - log.debug("COFF LLD new_digest={} readFile error: {}", .{ digest, @errorName(err) }); + const prev_digest: []u8 = Cache.readSmallFile( + directory.handle, + id_symlink_basename, + &prev_digest_buf, + ) catch |err| blk: { + log.debug("COFF LLD new_digest={} error: {}", .{ digest, @errorName(err) }); // Handle this as a cache miss. break :blk prev_digest_buf[0..0]; }; @@ -1180,9 +1184,9 @@ fn linkWithLLD(self: *Coff, comp: *Compilation) !void { } if (!self.base.options.disable_lld_caching) { - // Update the dangling file with the digest. If it fails we can continue; it only + // Update the file with the digest. If it fails we can continue; it only // means that the next invocation will have an unnecessary cache miss. - directory.handle.writeFile(id_symlink_basename, &digest) catch |err| { + Cache.writeSmallFile(directory.handle, id_symlink_basename, &digest) catch |err| { std.log.warn("failed to save linking hash digest file: {}", .{@errorName(err)}); }; // Again failure here only means an unnecessary cache miss. diff --git a/src/link/Elf.zig b/src/link/Elf.zig index 4fcfe78901..9c4029b3cd 100644 --- a/src/link/Elf.zig +++ b/src/link/Elf.zig @@ -1326,8 +1326,12 @@ fn linkWithLLD(self: *Elf, comp: *Compilation) !void { digest = man.final(); var prev_digest_buf: [digest.len]u8 = undefined; - const prev_digest: []u8 = directory.handle.readFile(id_symlink_basename, &prev_digest_buf) catch |err| blk: { - log.debug("ELF LLD new_digest={} readFile error: {}", .{ digest, @errorName(err) }); + const prev_digest: []u8 = Cache.readSmallFile( + directory.handle, + id_symlink_basename, + &prev_digest_buf, + ) catch |err| blk: { + log.debug("ELF LLD new_digest={} error: {}", .{ digest, @errorName(err) }); // Handle this as a cache miss. break :blk prev_digest_buf[0..0]; }; @@ -1647,9 +1651,9 @@ fn linkWithLLD(self: *Elf, comp: *Compilation) !void { } if (!self.base.options.disable_lld_caching) { - // Update the dangling file with the digest. If it fails we can continue; it only + // Update the file with the digest. If it fails we can continue; it only // means that the next invocation will have an unnecessary cache miss. - directory.handle.writeFile(id_symlink_basename, &digest) catch |err| { + Cache.writeSmallFile(directory.handle, id_symlink_basename, &digest) catch |err| { std.log.warn("failed to save linking hash digest file: {}", .{@errorName(err)}); }; // Again failure here only means an unnecessary cache miss. diff --git a/src/link/MachO.zig b/src/link/MachO.zig index 20fda846f5..9700a0bdb4 100644 --- a/src/link/MachO.zig +++ b/src/link/MachO.zig @@ -419,8 +419,12 @@ fn linkWithLLD(self: *MachO, comp: *Compilation) !void { digest = man.final(); var prev_digest_buf: [digest.len]u8 = undefined; - const prev_digest: []u8 = directory.handle.readFile(id_symlink_basename, &prev_digest_buf) catch |err| blk: { - log.debug("MachO LLD new_digest={} readFile error: {}", .{ digest, @errorName(err) }); + const prev_digest: []u8 = Cache.readSmallFile( + directory.handle, + id_symlink_basename, + &prev_digest_buf, + ) catch |err| blk: { + log.debug("MachO LLD new_digest={} error: {}", .{ digest, @errorName(err) }); // Handle this as a cache miss. break :blk prev_digest_buf[0..0]; }; @@ -674,9 +678,9 @@ fn linkWithLLD(self: *MachO, comp: *Compilation) !void { } if (!self.base.options.disable_lld_caching) { - // Update the dangling file with the digest. If it fails we can continue; it only + // Update the file with the digest. If it fails we can continue; it only // means that the next invocation will have an unnecessary cache miss. - directory.handle.writeFile(id_symlink_basename, &digest) catch |err| { + Cache.writeSmallFile(directory.handle, id_symlink_basename, &digest) catch |err| { std.log.warn("failed to save linking hash digest file: {}", .{@errorName(err)}); }; // Again failure here only means an unnecessary cache miss. diff --git a/src/link/Wasm.zig b/src/link/Wasm.zig index 320f90de19..2ab757461c 100644 --- a/src/link/Wasm.zig +++ b/src/link/Wasm.zig @@ -310,8 +310,12 @@ fn linkWithLLD(self: *Wasm, comp: *Compilation) !void { digest = man.final(); var prev_digest_buf: [digest.len]u8 = undefined; - const prev_digest: []u8 = directory.handle.readFile(id_symlink_basename, &prev_digest_buf) catch |err| blk: { - log.debug("WASM LLD new_digest={} readFile error: {}", .{ digest, @errorName(err) }); + const prev_digest: []u8 = Cache.readSmallFile( + directory.handle, + id_symlink_basename, + &prev_digest_buf, + ) catch |err| blk: { + log.debug("WASM LLD new_digest={} error: {}", .{ digest, @errorName(err) }); // Handle this as a cache miss. break :blk prev_digest_buf[0..0]; }; @@ -424,9 +428,9 @@ fn linkWithLLD(self: *Wasm, comp: *Compilation) !void { } if (!self.base.options.disable_lld_caching) { - // Update the dangling file with the digest. If it fails we can continue; it only + // Update the file with the digest. If it fails we can continue; it only // means that the next invocation will have an unnecessary cache miss. - directory.handle.writeFile(id_symlink_basename, &digest) catch |err| { + Cache.writeSmallFile(directory.handle, id_symlink_basename, &digest) catch |err| { std.log.warn("failed to save linking hash digest symlink: {}", .{@errorName(err)}); }; // Again failure here only means an unnecessary cache miss.