Commit a4c35a6245

Andrew Kelley <andrew@ziglang.org>
2023-03-06 00:11:04
std.Build: audit use of updateFile
* remove std.Build.updateFile. I noticed some people use it from build.zig (declare phase) when it is intended only for use in the make phase. - This also was incorrectly reporting errors with std.log. * std.Build.InstallArtifactStep - report better errors on failure - report whether the step was cached or not * std.Build.InstallDirStep: report better error on failure * std.Build.InstallFileStep: report better error on failure
1 parent 1e63573
lib/std/Build/CompileStep.zig
@@ -1910,13 +1910,13 @@ fn make(step: *Step, prog_node: *std.Progress.Node) !void {
     const build_output_dir = fs.path.dirname(output_bin_path).?;
 
     if (self.output_dir) |output_dir| {
-        var src_dir = try std.fs.cwd().openIterableDir(build_output_dir, .{});
+        var src_dir = try fs.cwd().openIterableDir(build_output_dir, .{});
         defer src_dir.close();
 
         // Create the output directory if it doesn't exist.
-        try std.fs.cwd().makePath(output_dir);
+        try fs.cwd().makePath(output_dir);
 
-        var dest_dir = try std.fs.cwd().openDir(output_dir, .{});
+        var dest_dir = try fs.cwd().openDir(output_dir, .{});
         defer dest_dir.close();
 
         var it = src_dir.iterate();
lib/std/Build/InstallArtifactStep.zig
@@ -3,6 +3,7 @@ const Step = std.Build.Step;
 const CompileStep = std.Build.CompileStep;
 const InstallDir = std.Build.InstallDir;
 const InstallArtifactStep = @This();
+const fs = std.fs;
 
 pub const base_id = .install_artifact;
 
@@ -77,25 +78,59 @@ fn make(step: *Step, prog_node: *std.Progress.Node) !void {
 
     const dest_sub_path = if (self.dest_sub_path) |sub_path| sub_path else self.artifact.out_filename;
     const full_dest_path = dest_builder.getInstallPath(self.dest_dir, dest_sub_path);
+    const cwd = fs.cwd();
 
-    try src_builder.updateFile(
-        self.artifact.getOutputSource().getPath(src_builder),
-        full_dest_path,
-    );
-    if (self.artifact.isDynamicLibrary() and self.artifact.version != null and self.artifact.target.wantSharedLibSymLinks()) {
+    var all_cached = true;
+
+    {
+        const full_src_path = self.artifact.getOutputSource().getPath(src_builder);
+        const p = fs.Dir.updateFile(cwd, full_src_path, cwd, full_dest_path, .{}) catch |err| {
+            return step.fail("unable to update file from '{s}' to '{s}': {s}", .{
+                full_src_path, full_dest_path, @errorName(err),
+            });
+        };
+        all_cached = all_cached and p == .fresh;
+    }
+
+    if (self.artifact.isDynamicLibrary() and
+        self.artifact.version != null and
+        self.artifact.target.wantSharedLibSymLinks())
+    {
         try CompileStep.doAtomicSymLinks(step, full_dest_path, self.artifact.major_only_filename.?, self.artifact.name_only_filename.?);
     }
-    if (self.artifact.isDynamicLibrary() and self.artifact.target.isWindows() and self.artifact.emit_implib != .no_emit) {
+    if (self.artifact.isDynamicLibrary() and
+        self.artifact.target.isWindows() and
+        self.artifact.emit_implib != .no_emit)
+    {
+        const full_src_path = self.artifact.getOutputLibSource().getPath(src_builder);
         const full_implib_path = dest_builder.getInstallPath(self.dest_dir, self.artifact.out_lib_filename);
-        try src_builder.updateFile(self.artifact.getOutputLibSource().getPath(src_builder), full_implib_path);
+        const p = fs.Dir.updateFile(cwd, full_src_path, cwd, full_implib_path, .{}) catch |err| {
+            return step.fail("unable to update file from '{s}' to '{s}': {s}", .{
+                full_src_path, full_implib_path, @errorName(err),
+            });
+        };
+        all_cached = all_cached and p == .fresh;
     }
     if (self.pdb_dir) |pdb_dir| {
+        const full_src_path = self.artifact.getOutputPdbSource().getPath(src_builder);
         const full_pdb_path = dest_builder.getInstallPath(pdb_dir, self.artifact.out_pdb_filename);
-        try src_builder.updateFile(self.artifact.getOutputPdbSource().getPath(src_builder), full_pdb_path);
+        const p = fs.Dir.updateFile(cwd, full_src_path, cwd, full_pdb_path, .{}) catch |err| {
+            return step.fail("unable to update file from '{s}' to '{s}': {s}", .{
+                full_src_path, full_pdb_path, @errorName(err),
+            });
+        };
+        all_cached = all_cached and p == .fresh;
     }
     if (self.h_dir) |h_dir| {
+        const full_src_path = self.artifact.getOutputHSource().getPath(src_builder);
         const full_h_path = dest_builder.getInstallPath(h_dir, self.artifact.out_h_filename);
-        try src_builder.updateFile(self.artifact.getOutputHSource().getPath(src_builder), full_h_path);
+        const p = fs.Dir.updateFile(cwd, full_src_path, cwd, full_h_path, .{}) catch |err| {
+            return step.fail("unable to update file from '{s}' to '{s}': {s}", .{
+                full_src_path, full_h_path, @errorName(err),
+            });
+        };
+        all_cached = all_cached and p == .fresh;
     }
     self.artifact.installed_path = full_dest_path;
+    step.result_cached = all_cached;
 }
lib/std/Build/InstallDirStep.zig
@@ -89,13 +89,17 @@ fn make(step: *Step, prog_node: *std.Progress.Node) !void {
                     }
                 }
 
-                const prev_status = try fs.Dir.updateFile(
+                const prev_status = fs.Dir.updateFile(
                     src_builder.build_root.handle,
                     src_sub_path,
                     cwd,
                     dest_path,
                     .{},
-                );
+                ) catch |err| {
+                    return step.fail("unable to update file from '{}{s}' to '{s}': {s}", .{
+                        src_builder.build_root, src_sub_path, dest_path, @errorName(err),
+                    });
+                };
                 all_cached = all_cached and prev_status == .fresh;
             },
             else => continue,
lib/std/Build/InstallFileStep.zig
@@ -42,5 +42,11 @@ fn make(step: *Step, prog_node: *std.Progress.Node) !void {
     const dest_builder = self.dest_builder;
     const full_src_path = self.source.getPath2(src_builder, step);
     const full_dest_path = dest_builder.getInstallPath(self.dir, self.dest_rel_path);
-    try dest_builder.updateFile(full_src_path, full_dest_path);
+    const cwd = std.fs.cwd();
+    const prev = std.fs.Dir.updateFile(cwd, full_src_path, cwd, full_dest_path, .{}) catch |err| {
+        return step.fail("unable to update file from '{s}' to '{s}': {s}", .{
+            full_src_path, full_dest_path, @errorName(err),
+        });
+    };
+    step.result_cached = prev == .fresh;
 }
lib/std/Build.zig
@@ -1235,18 +1235,6 @@ pub fn pushInstalledFile(self: *Build, dir: InstallDir, dest_rel_path: []const u
     self.installed_files.append(file.dupe(self)) catch @panic("OOM");
 }
 
-pub fn updateFile(self: *Build, source_path: []const u8, dest_path: []const u8) !void {
-    if (self.verbose) {
-        log.info("cp {s} {s} ", .{ source_path, dest_path });
-    }
-    const cwd = fs.cwd();
-    const prev_status = try fs.Dir.updateFile(cwd, source_path, cwd, dest_path, .{});
-    if (self.verbose) switch (prev_status) {
-        .stale => log.info("# installed", .{}),
-        .fresh => log.info("# up-to-date", .{}),
-    };
-}
-
 pub fn truncateFile(self: *Build, dest_path: []const u8) !void {
     if (self.verbose) {
         log.info("truncate {s}", .{dest_path});