Commit 21181181bf

Andrew Kelley <andrew@ziglang.org>
2023-10-03 02:00:45
zig fetch: enhanced error reporting
* Package: use std.tar diagnostics to give detailed error messages * std.tar: add diagnostic for unsupported file type
1 parent ef9966c
Changed files (4)
lib/std/zig/ErrorBundle.zig
@@ -202,7 +202,7 @@ fn renderErrorMessageToWriter(
         try counting_stderr.writeAll(": ");
         // This is the length of the part before the error message:
         // e.g. "file.zig:4:5: error: "
-        const prefix_len = @as(usize, @intCast(counting_stderr.context.bytes_written));
+        const prefix_len: usize = @intCast(counting_stderr.context.bytes_written);
         try ttyconf.setColor(stderr, .reset);
         try ttyconf.setColor(stderr, .bold);
         if (err_msg.count == 1) {
@@ -356,7 +356,7 @@ pub const Wip = struct {
         }
 
         const compile_log_str_index = if (compile_log_text.len == 0) 0 else str: {
-            const str = @as(u32, @intCast(wip.string_bytes.items.len));
+            const str: u32 = @intCast(wip.string_bytes.items.len);
             try wip.string_bytes.ensureUnusedCapacity(gpa, compile_log_text.len + 1);
             wip.string_bytes.appendSliceAssumeCapacity(compile_log_text);
             wip.string_bytes.appendAssumeCapacity(0);
@@ -364,8 +364,8 @@ pub const Wip = struct {
         };
 
         wip.setExtra(0, ErrorMessageList{
-            .len = @as(u32, @intCast(wip.root_list.items.len)),
-            .start = @as(u32, @intCast(wip.extra.items.len)),
+            .len = @intCast(wip.root_list.items.len),
+            .start = @intCast(wip.extra.items.len),
             .compile_log_text = compile_log_str_index,
         });
         try wip.extra.appendSlice(gpa, @as([]const u32, @ptrCast(wip.root_list.items)));
@@ -385,7 +385,7 @@ pub const Wip = struct {
 
     pub fn addString(wip: *Wip, s: []const u8) !u32 {
         const gpa = wip.gpa;
-        const index = @as(u32, @intCast(wip.string_bytes.items.len));
+        const index: u32 = @intCast(wip.string_bytes.items.len);
         try wip.string_bytes.ensureUnusedCapacity(gpa, s.len + 1);
         wip.string_bytes.appendSliceAssumeCapacity(s);
         wip.string_bytes.appendAssumeCapacity(0);
@@ -394,7 +394,7 @@ pub const Wip = struct {
 
     pub fn printString(wip: *Wip, comptime fmt: []const u8, args: anytype) !u32 {
         const gpa = wip.gpa;
-        const index = @as(u32, @intCast(wip.string_bytes.items.len));
+        const index: u32 = @intCast(wip.string_bytes.items.len);
         try wip.string_bytes.writer(gpa).print(fmt, args);
         try wip.string_bytes.append(gpa, 0);
         return index;
@@ -406,15 +406,15 @@ pub const Wip = struct {
     }
 
     pub fn addErrorMessage(wip: *Wip, em: ErrorMessage) !MessageIndex {
-        return @as(MessageIndex, @enumFromInt(try addExtra(wip, em)));
+        return @enumFromInt(try addExtra(wip, em));
     }
 
     pub fn addErrorMessageAssumeCapacity(wip: *Wip, em: ErrorMessage) MessageIndex {
-        return @as(MessageIndex, @enumFromInt(addExtraAssumeCapacity(wip, em)));
+        return @enumFromInt(addExtraAssumeCapacity(wip, em));
     }
 
     pub fn addSourceLocation(wip: *Wip, sl: SourceLocation) !SourceLocationIndex {
-        return @as(SourceLocationIndex, @enumFromInt(try addExtra(wip, sl)));
+        return @enumFromInt(try addExtra(wip, sl));
     }
 
     pub fn addReferenceTrace(wip: *Wip, rt: ReferenceTrace) !void {
@@ -430,7 +430,7 @@ pub const Wip = struct {
         const other_list = other.getMessages();
 
         // The ensureUnusedCapacity call above guarantees this.
-        const notes_start = wip.reserveNotes(@as(u32, @intCast(other_list.len))) catch unreachable;
+        const notes_start = wip.reserveNotes(@intCast(other_list.len)) catch unreachable;
         for (notes_start.., other_list) |note, message| {
             wip.extra.items[note] = @intFromEnum(wip.addOtherMessage(other, message) catch unreachable);
         }
@@ -455,7 +455,7 @@ pub const Wip = struct {
         try wip.extra.ensureUnusedCapacity(wip.gpa, notes_len +
             notes_len * @typeInfo(ErrorBundle.ErrorMessage).Struct.fields.len);
         wip.extra.items.len += notes_len;
-        return @as(u32, @intCast(wip.extra.items.len - notes_len));
+        return @intCast(wip.extra.items.len - notes_len);
     }
 
     fn addOtherMessage(wip: *Wip, other: ErrorBundle, msg_index: MessageIndex) !MessageIndex {
@@ -510,7 +510,7 @@ pub const Wip = struct {
 
     fn addExtraAssumeCapacity(wip: *Wip, extra: anytype) u32 {
         const fields = @typeInfo(@TypeOf(extra)).Struct.fields;
-        const result = @as(u32, @intCast(wip.extra.items.len));
+        const result: u32 = @intCast(wip.extra.items.len);
         wip.extra.items.len += fields.len;
         setExtra(wip, result, extra);
         return result;
lib/std/tar.zig
@@ -29,6 +29,10 @@ pub const Options = struct {
                 file_name: []const u8,
                 link_name: []const u8,
             },
+            unsupported_file_type: struct {
+                file_name: []const u8,
+                file_type: Header.FileType,
+            },
         };
 
         pub fn deinit(d: *Diagnostics) void {
@@ -38,6 +42,9 @@ pub const Options = struct {
                         d.allocator.free(info.file_name);
                         d.allocator.free(info.link_name);
                     },
+                    .unsupported_file_type => |info| {
+                        d.allocator.free(info.file_name);
+                    },
                 }
             }
             d.errors.deinit(d.allocator);
@@ -50,6 +57,7 @@ pub const Header = struct {
     bytes: *const [512]u8,
 
     pub const FileType = enum(u8) {
+        normal_alias = 0,
         normal = '0',
         hard_link = '1',
         symbolic_link = '2',
@@ -105,8 +113,9 @@ pub const Header = struct {
     }
 
     pub fn fileType(header: Header) FileType {
-        const result = @as(FileType, @enumFromInt(header.bytes[156]));
-        return if (result == @as(FileType, @enumFromInt(0))) .normal else result;
+        const result: FileType = @enumFromInt(header.bytes[156]);
+        if (result == .normal_alias) return .normal;
+        return result;
     }
 
     fn str(header: Header, start: usize, end: usize) []const u8 {
@@ -268,18 +277,21 @@ pub fn pipeToFileSystem(dir: std.fs.Dir, reader: anytype, options: Options) !voi
                 const link_name = header.linkName();
 
                 dir.symLink(link_name, file_name, .{}) catch |err| {
-                    if (options.diagnostics) |d| {
-                        try d.errors.append(d.allocator, .{ .unable_to_create_sym_link = .{
-                            .code = err,
-                            .file_name = try d.allocator.dupe(u8, file_name),
-                            .link_name = try d.allocator.dupe(u8, link_name),
-                        } });
-                    } else {
-                        return error.UnableToCreateSymLink;
-                    }
+                    const d = options.diagnostics orelse return error.UnableToCreateSymLink;
+                    try d.errors.append(d.allocator, .{ .unable_to_create_sym_link = .{
+                        .code = err,
+                        .file_name = try d.allocator.dupe(u8, file_name),
+                        .link_name = try d.allocator.dupe(u8, link_name),
+                    } });
                 };
             },
-            else => return error.TarUnsupportedFileType,
+            else => |file_type| {
+                const d = options.diagnostics orelse return error.TarUnsupportedFileType;
+                try d.errors.append(d.allocator, .{ .unsupported_file_type = .{
+                    .file_name = try d.allocator.dupe(u8, unstripped_file_name),
+                    .file_type = file_type,
+                } });
+            },
         }
     }
 }
src/main.zig
@@ -6610,6 +6610,7 @@ fn cmdFetch(
     arena: Allocator,
     args: []const []const u8,
 ) !void {
+    const color: Color = .auto;
     var opt_url: ?[]const u8 = null;
     var override_global_cache_dir: ?[]const u8 = try optionalStringEnvVar(arena, "ZIG_GLOBAL_CACHE_DIR");
 
@@ -6651,10 +6652,17 @@ fn cmdFetch(
     const root_prog_node = progress.start("Fetch", 0);
     defer root_prog_node.end();
 
+    var wip_errors: std.zig.ErrorBundle.Wip = undefined;
+    try wip_errors.init(gpa);
+    defer wip_errors.deinit();
+
     var report: Package.Report = .{
         .ast = null,
-        .directory = undefined,
-        .error_bundle = undefined,
+        .directory = .{
+            .handle = fs.cwd(),
+            .path = null,
+        },
+        .error_bundle = &wip_errors,
     };
 
     var global_cache_directory: Compilation.Directory = l: {
@@ -6697,14 +6705,22 @@ fn cmdFetch(
     };
     defer readable_resource.deinit(gpa);
 
-    var package_location = try readable_resource.unpack(
+    var package_location = readable_resource.unpack(
         gpa,
         &thread_pool,
         global_cache_directory,
         0,
         report,
         root_prog_node,
-    );
+    ) catch |err| {
+        if (wip_errors.root_list.items.len > 0) {
+            var errors = try wip_errors.toOwnedBundle("");
+            defer errors.deinit(gpa);
+            errors.renderToStdErr(renderOptions(color));
+            process.exit(1);
+        }
+        fatal("unable to unpack '{s}': {s}", .{ url, @errorName(err) });
+    };
     defer package_location.deinit(gpa);
 
     const hex_digest = Package.Manifest.hexDigest(package_location.hash);
src/Package.zig
@@ -285,7 +285,8 @@ pub fn fetchAndAddDependencies(
     if (manifest.errors.len > 0) {
         const file_path = try directory.join(arena, &.{Manifest.basename});
         for (manifest.errors) |msg| {
-            try Report.addErrorMessage(ast, file_path, error_bundle, 0, msg);
+            const str = try error_bundle.addString(msg.msg);
+            try Report.addErrorMessage(&ast, file_path, error_bundle, 0, str, msg.tok, msg.off);
         }
         return error.PackageFetchFailed;
     }
@@ -465,20 +466,31 @@ pub const Report = struct {
         comptime fmt_string: []const u8,
         fmt_args: anytype,
     ) error{ PackageFetchFailed, OutOfMemory } {
-        const ast = report.ast orelse main.fatal(fmt_string, fmt_args);
+        const msg = try report.error_bundle.printString(fmt_string, fmt_args);
+        return failMsg(report, tok, msg);
+    }
+
+    fn failMsg(
+        report: Report,
+        tok: std.zig.Ast.TokenIndex,
+        msg: u32,
+    ) error{ PackageFetchFailed, OutOfMemory } {
         const gpa = report.error_bundle.gpa;
 
         const file_path = try report.directory.join(gpa, &.{Manifest.basename});
         defer gpa.free(file_path);
 
-        const msg = try std.fmt.allocPrint(gpa, fmt_string, fmt_args);
-        defer gpa.free(msg);
+        const eb = report.error_bundle;
 
-        try addErrorMessage(ast.*, file_path, report.error_bundle, 0, .{
-            .tok = tok,
-            .off = 0,
-            .msg = msg,
-        });
+        if (report.ast) |ast| {
+            try addErrorMessage(ast, file_path, eb, 0, msg, tok, 0);
+        } else {
+            try eb.addRootErrorMessage(.{
+                .msg = msg,
+                .src_loc = .none,
+                .notes_len = 0,
+            });
+        }
 
         return error.PackageFetchFailed;
     }
@@ -488,31 +500,42 @@ pub const Report = struct {
         notes_len: u32,
         msg: Manifest.ErrorMessage,
     ) error{OutOfMemory}!void {
-        const ast = report.ast orelse main.fatal("{s}", .{msg.msg});
-        const gpa = report.error_bundle.gpa;
-        const file_path = try report.directory.join(gpa, &.{Manifest.basename});
-        defer gpa.free(file_path);
-        return addErrorMessage(ast.*, file_path, report.error_bundle, notes_len, msg);
+        const eb = report.error_bundle;
+        const msg_str = try eb.addString(msg.msg);
+        if (report.ast) |ast| {
+            const gpa = eb.gpa;
+            const file_path = try report.directory.join(gpa, &.{Manifest.basename});
+            defer gpa.free(file_path);
+            return addErrorMessage(ast, file_path, eb, notes_len, msg_str, msg.tok, msg.off);
+        } else {
+            return eb.addRootErrorMessage(.{
+                .msg = msg_str,
+                .src_loc = .none,
+                .notes_len = notes_len,
+            });
+        }
     }
 
     fn addErrorMessage(
-        ast: std.zig.Ast,
+        ast: *const std.zig.Ast,
         file_path: []const u8,
         eb: *std.zig.ErrorBundle.Wip,
         notes_len: u32,
-        msg: Manifest.ErrorMessage,
+        msg_str: u32,
+        msg_tok: std.zig.Ast.TokenIndex,
+        msg_off: u32,
     ) error{OutOfMemory}!void {
         const token_starts = ast.tokens.items(.start);
-        const start_loc = ast.tokenLocation(0, msg.tok);
+        const start_loc = ast.tokenLocation(0, msg_tok);
 
         try eb.addRootErrorMessage(.{
-            .msg = try eb.addString(msg.msg),
+            .msg = msg_str,
             .src_loc = try eb.addSourceLocation(.{
                 .src_path = try eb.addString(file_path),
-                .span_start = token_starts[msg.tok],
-                .span_end = @as(u32, @intCast(token_starts[msg.tok] + ast.tokenSlice(msg.tok).len)),
-                .span_main = token_starts[msg.tok] + msg.off,
-                .line = @as(u32, @intCast(start_loc.line)),
+                .span_start = token_starts[msg_tok],
+                .span_end = @as(u32, @intCast(token_starts[msg_tok] + ast.tokenSlice(msg_tok).len)),
+                .span_main = token_starts[msg_tok] + msg_off,
+                .line = @intCast(start_loc.line),
                 .column = @as(u32, @intCast(start_loc.column)),
                 .source_line = try eb.addString(ast.source[start_loc.line_start..start_loc.line_end]),
             }),
@@ -752,9 +775,9 @@ pub const ReadableResource = struct {
                         };
 
                         switch (try rr.getFileType(dep_location_tok, report)) {
-                            .tar => try unpackTarball(prog_reader.reader(), tmp_directory.handle),
-                            .@"tar.gz" => try unpackTarballCompressed(allocator, prog_reader, tmp_directory.handle, std.compress.gzip),
-                            .@"tar.xz" => try unpackTarballCompressed(allocator, prog_reader, tmp_directory.handle, std.compress.xz),
+                            .tar => try unpackTarball(allocator, prog_reader.reader(), tmp_directory.handle, dep_location_tok, report),
+                            .@"tar.gz" => try unpackTarballCompressed(allocator, prog_reader, tmp_directory.handle, dep_location_tok, report, std.compress.gzip),
+                            .@"tar.xz" => try unpackTarballCompressed(allocator, prog_reader, tmp_directory.handle, dep_location_tok, report, std.compress.xz),
                             .git_pack => try unpackGitPack(allocator, &prog_reader, git.parseOid(rr.path) catch unreachable, tmp_directory.handle),
                         }
                     } else {
@@ -1128,6 +1151,8 @@ fn unpackTarballCompressed(
     gpa: Allocator,
     reader: anytype,
     out_dir: fs.Dir,
+    dep_location_tok: std.zig.Ast.TokenIndex,
+    report: Report,
     comptime Compression: type,
 ) !void {
     var br = std.io.bufferedReaderSize(std.crypto.tls.max_ciphertext_record_len, reader);
@@ -1135,11 +1160,21 @@ fn unpackTarballCompressed(
     var decompress = try Compression.decompress(gpa, br.reader());
     defer decompress.deinit();
 
-    return unpackTarball(decompress.reader(), out_dir);
+    return unpackTarball(gpa, decompress.reader(), out_dir, dep_location_tok, report);
 }
 
-fn unpackTarball(reader: anytype, out_dir: fs.Dir) !void {
+fn unpackTarball(
+    gpa: Allocator,
+    reader: anytype,
+    out_dir: fs.Dir,
+    dep_location_tok: std.zig.Ast.TokenIndex,
+    report: Report,
+) !void {
+    var diagnostics: std.tar.Options.Diagnostics = .{ .allocator = gpa };
+    defer diagnostics.deinit();
+
     try std.tar.pipeToFileSystem(out_dir, reader, .{
+        .diagnostics = &diagnostics,
         .strip_components = 1,
         // TODO: we would like to set this to executable_bit_only, but two
         // things need to happen before that:
@@ -1148,6 +1183,36 @@ fn unpackTarball(reader: anytype, out_dir: fs.Dir) !void {
         //    bit on Windows from the ACLs (see the isExecutable function).
         .mode_mode = .ignore,
     });
+
+    if (diagnostics.errors.items.len > 0) {
+        const notes_len: u32 = @intCast(diagnostics.errors.items.len);
+        try report.addErrorWithNotes(notes_len, .{
+            .tok = dep_location_tok,
+            .off = 0,
+            .msg = "unable to unpack tarball",
+        });
+        const eb = report.error_bundle;
+        const notes_start = try eb.reserveNotes(notes_len);
+        for (diagnostics.errors.items, notes_start..) |item, note_i| {
+            switch (item) {
+                .unable_to_create_sym_link => |info| {
+                    eb.extra.items[note_i] = @intFromEnum(try eb.addErrorMessage(.{
+                        .msg = try eb.printString("unable to create symlink from '{s}' to '{s}': {s}", .{
+                            info.file_name, info.link_name, @errorName(info.code),
+                        }),
+                    }));
+                },
+                .unsupported_file_type => |info| {
+                    eb.extra.items[note_i] = @intFromEnum(try eb.addErrorMessage(.{
+                        .msg = try eb.printString("file '{s}' has unsupported type '{c}'", .{
+                            info.file_name, @intFromEnum(info.file_type),
+                        }),
+                    }));
+                },
+            }
+        }
+        return error.InvalidTarball;
+    }
 }
 
 fn unpackGitPack(