From 116546a996a12a2e68a68ddf7926d2b4e708c52c Mon Sep 17 00:00:00 2001 From: Arya-Elfren <109028294+Arya-Elfren@users.noreply.github.com> Date: Wed, 26 Apr 2023 22:07:20 +0100 Subject: [PATCH 01/15] Clarify `f16` maths - closes #204 --- exercises/060_floats.zig | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/exercises/060_floats.zig b/exercises/060_floats.zig index 8ba51db..0ebd7a2 100644 --- a/exercises/060_floats.zig +++ b/exercises/060_floats.zig @@ -1,7 +1,8 @@ // // Zig has support for IEEE-754 floating-point numbers in these // specific sizes: f16, f32, f64, f80, and f128. Floating point -// literals may be written in scientific notation: +// literals may be written in the same ways as integers but also +// in scientific notation: // // const a1: f32 = 1200.0; // 1,200 // const a2: f32 = 1.2e+3; // 1,200 @@ -26,7 +27,10 @@ // operations with numeric literals, ensure the types match. Zig // does not perform unsafe type coercions behind your back: // -// var foo: f16 = 13.5 * 5; // ERROR! +// fn foo(bar: u16) f16 { return 13.5 * bar; } // ERROR! +// var foo: f16 = 13.5 * @as(u8, 5); // ERROR! +// var foo: f16 = 13.5 * 5; // This is a safe compile-time +// // conversion, so no problem! // var foo: f16 = 13.5 * 5.0; // No problem, both are floats // // Please fix the two float problems with this program and From 18f69f5634c7469042dc601e4c5609af9e0f382c Mon Sep 17 00:00:00 2001 From: Arya-Elfren <109028294+Arya-Elfren@users.noreply.github.com> Date: Wed, 26 Apr 2023 22:47:03 +0100 Subject: [PATCH 02/15] Clarify the methods syntax sugar & a bit more I think it's a bit clearer to show exactly what the syntax sugar of methods is, because that's all it is. Every function in Zig is in a struct (files are structs after all) and methods just simplify their use. I also thought we might use the explicit saturating subtraction as that is why the feature is in Zig. --- exercises/047_methods.zig | 43 ++++++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/exercises/047_methods.zig b/exercises/047_methods.zig index 96d4c8e..7211caa 100644 --- a/exercises/047_methods.zig +++ b/exercises/047_methods.zig @@ -2,9 +2,9 @@ // Help! Evil alien creatures have hidden eggs all over the Earth // and they're starting to hatch! // -// Before you jump into battle, you'll need to know four things: +// Before you jump into battle, you'll need to know three things: // -// 1. You can attach functions to structs: +// 1. You can attach functions to structs (and other "type definitions"): // // const Foo = struct{ // pub fn hello() void { @@ -12,31 +12,34 @@ // } // }; // -// 2. A function that is a member of a struct is a "method" and is -// called with the "dot syntax" like so: +// 2. A function that is a member of a struct is "namespaced" within +// that struct and is called by specifying the "namespace" and then +// using the "dot syntax": // // Foo.hello(); // -// 3. The NEAT feature of methods is the special parameter named -// "self" that takes an instance of that type of struct: +// 3. The NEAT feature of these functions is that if they take either +// an instance of the struct or a pointer to an instance of the struct +// then they have some syntax sugar: // // const Bar = struct{ -// number: u32, -// -// pub fn printMe(self: Bar) void { -// std.debug.print("{}\n", .{self.number}); -// } +// pub fn a(self: Bar) void { _ = self; } +// pub fn b(this: *Bar, other: u8) void { _ = this; _ = other; } +// pub fn c(bar: *const Bar) void { _ = bar; } // }; // -// (Actually, you can name the first parameter anything, but -// please follow convention and use "self".) +// var bar = Bar{}; +// bar.a() // is equivalent to Bar.a(bar) +// bar.b(3) // is equivalent to Bar.b(&bar, 3) +// bar.c() // is equivalent to Bar.c(&bar) // -// 4. Now when you call the method on an INSTANCE of that struct -// with the "dot syntax", the instance will be automatically -// passed as the "self" parameter: +// Notice that the name of the parameter doesn't matter. Some use +// self, others use a lowercase version of the type name, but feel +// free to use whatever is most appropriate. // -// var my_bar = Bar{ .number = 2000 }; -// my_bar.printMe(); // prints "2000" +// Effectively, the method syntax sugar just does this transformation: +// thing.function(args); +// @TypeOf(thing).function(thing, args); // // Okay, you're armed. // @@ -63,7 +66,9 @@ const HeatRay = struct { // We love this method: pub fn zap(self: HeatRay, alien: *Alien) void { - alien.health -= if (self.damage >= alien.health) alien.health else self.damage; + alien.health -|= self.damage; // Saturating inplace substraction + // It subtracts but doesn't go below the + // lowest value for our type (in this case 0) } }; From 3612c67f04e0d902a12c3f71ed52b1de8422804e Mon Sep 17 00:00:00 2001 From: Arya-Elfren <109028294+Arya-Elfren@users.noreply.github.com> Date: Fri, 28 Apr 2023 11:12:42 +0100 Subject: [PATCH 03/15] Simplify methods explanation in 047 --- exercises/047_methods.zig | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/exercises/047_methods.zig b/exercises/047_methods.zig index 7211caa..6b2dbef 100644 --- a/exercises/047_methods.zig +++ b/exercises/047_methods.zig @@ -18,14 +18,14 @@ // // Foo.hello(); // -// 3. The NEAT feature of these functions is that if they take either -// an instance of the struct or a pointer to an instance of the struct -// then they have some syntax sugar: +// 3. The NEAT feature of these functions is that if their first argument +// is an instance of the struct (or a pointer to one) then we can use +// the instance as the namespace instead of the type: // // const Bar = struct{ -// pub fn a(self: Bar) void { _ = self; } -// pub fn b(this: *Bar, other: u8) void { _ = this; _ = other; } -// pub fn c(bar: *const Bar) void { _ = bar; } +// pub fn a(self: Bar) void {} +// pub fn b(this: *Bar, other: u8) void {} +// pub fn c(bar: *const Bar) void {} // }; // // var bar = Bar{}; @@ -37,10 +37,6 @@ // self, others use a lowercase version of the type name, but feel // free to use whatever is most appropriate. // -// Effectively, the method syntax sugar just does this transformation: -// thing.function(args); -// @TypeOf(thing).function(thing, args); -// // Okay, you're armed. // // Now, please zap the alien structs until they're all gone or @@ -66,9 +62,7 @@ const HeatRay = struct { // We love this method: pub fn zap(self: HeatRay, alien: *Alien) void { - alien.health -|= self.damage; // Saturating inplace substraction - // It subtracts but doesn't go below the - // lowest value for our type (in this case 0) + alien.health -= if (self.damage >= alien.health) alien.health else self.damage; } }; From 599bea57051efcfaa7a81cb2a846fe095b9759c0 Mon Sep 17 00:00:00 2001 From: Arya-Elfren <109028294+Arya-Elfren@users.noreply.github.com> Date: Fri, 28 Apr 2023 11:32:45 +0100 Subject: [PATCH 04/15] Simplify `f16` coersion example --- exercises/060_floats.zig | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/exercises/060_floats.zig b/exercises/060_floats.zig index 0ebd7a2..a04a54d 100644 --- a/exercises/060_floats.zig +++ b/exercises/060_floats.zig @@ -23,15 +23,12 @@ // const pi: f16 = 3.1415926535; // rounds to 3.140625 // const av: f16 = 6.02214076e+23; // Avogadro's inf(inity)! // -// A float literal has a decimal point. When performing math +// A float literal doesn't need a decimal point. When performing math // operations with numeric literals, ensure the types match. Zig // does not perform unsafe type coercions behind your back: // -// fn foo(bar: u16) f16 { return 13.5 * bar; } // ERROR! -// var foo: f16 = 13.5 * @as(u8, 5); // ERROR! -// var foo: f16 = 13.5 * 5; // This is a safe compile-time -// // conversion, so no problem! -// var foo: f16 = 13.5 * 5.0; // No problem, both are floats +// var foo: f16 = 5; // NO ERROR +// var foo: f16 = @as(u16, 5); // ERROR // // Please fix the two float problems with this program and // display the result as a whole number. From c2fe843a8a780e77e9875c9f6da8b3c3999915f2 Mon Sep 17 00:00:00 2001 From: Arya-Elfren <109028294+Arya-Elfren@users.noreply.github.com> Date: Fri, 28 Apr 2023 15:11:43 +0100 Subject: [PATCH 05/15] 060 - remove `@as()` --- exercises/060_floats.zig | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/exercises/060_floats.zig b/exercises/060_floats.zig index a04a54d..1320171 100644 --- a/exercises/060_floats.zig +++ b/exercises/060_floats.zig @@ -4,8 +4,8 @@ // literals may be written in the same ways as integers but also // in scientific notation: // -// const a1: f32 = 1200.0; // 1,200 -// const a2: f32 = 1.2e+3; // 1,200 +// const a1: f32 = 1200; // 1,200 +// const a2: f32 = 1.2e+3; // 1,200 // const b1: f32 = -500_000.0; // -500,000 // const b2: f32 = -5.0e+5; // -500,000 // @@ -23,12 +23,14 @@ // const pi: f16 = 3.1415926535; // rounds to 3.140625 // const av: f16 = 6.02214076e+23; // Avogadro's inf(inity)! // -// A float literal doesn't need a decimal point. When performing math -// operations with numeric literals, ensure the types match. Zig -// does not perform unsafe type coercions behind your back: +// When performing math operations with numeric literals, ensure +// the types match. Zig does not perform unsafe type coercions +// behind your back: // // var foo: f16 = 5; // NO ERROR -// var foo: f16 = @as(u16, 5); // ERROR +// +// var foo: u16 = 5; // A literal of a different type +// var bar: f16 = foo; // ERROR // // Please fix the two float problems with this program and // display the result as a whole number. From 74b48192e4ab3f9b8dbc5ac1e40f295f88a976d8 Mon Sep 17 00:00:00 2001 From: Manlio Perillo Date: Tue, 2 May 2023 10:51:59 +0200 Subject: [PATCH 06/15] test: don't run heal during configuration phase In order to simplify the code, the heal function is called during the configuration phase, thus resulting in the function being always called when the build.zig file is run. This behavior unfortunately causes a serious issue when the user fix a broken exercise and, during the next step, the heal function tries to heal the fixed exercise resulting in GNU patch assuming an attempt to reverse a patch, waiting for input from the terminal. Run the heal function from the new HealStep step, so that it is called only during tests. Rename the outdir constant to work_path, for consistency with build.zig. Fixes #272 --- test/tests.zig | 59 ++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 48 insertions(+), 11 deletions(-) diff --git a/test/tests.zig b/test/tests.zig index 8786d91..752ca50 100644 --- a/test/tests.zig +++ b/test/tests.zig @@ -20,15 +20,14 @@ pub fn addCliTests(b: *std.Build, exercises: []const Exercise) *Step { // We should use a temporary path, but it will make the implementation of // `build.zig` more complex. - const outdir = "patches/healed"; + const work_path = "patches/healed"; - fs.cwd().makePath(outdir) catch |err| { - return fail(step, "unable to make '{s}': {s}\n", .{ outdir, @errorName(err) }); - }; - heal(b.allocator, exercises, outdir) catch |err| { - return fail(step, "unable to heal exercises: {s}\n", .{@errorName(err)}); + fs.cwd().makePath(work_path) catch |err| { + return fail(step, "unable to make '{s}': {s}\n", .{ work_path, @errorName(err) }); }; + const heal_step = HealStep.create(b, exercises, work_path); + { // Test that `zig build -Dhealed -Dn=n test` selects the nth exercise. const case_step = createCase(b, "case-1"); @@ -49,6 +48,8 @@ pub fn addCliTests(b: *std.Build, exercises: []const Exercise) *Step { else expectStdErrMatch(cmd, ex.output); + cmd.step.dependOn(&heal_step.step); + case_step.dependOn(&cmd.step); } @@ -72,6 +73,8 @@ pub fn addCliTests(b: *std.Build, exercises: []const Exercise) *Step { cmd.expectStdOutEqual(""); expectStdErrMatch(cmd, b.fmt("{s} skipped", .{ex.main_file})); + cmd.step.dependOn(&heal_step.step); + case_step.dependOn(&cmd.step); } @@ -86,6 +89,7 @@ pub fn addCliTests(b: *std.Build, exercises: []const Exercise) *Step { const cmd = b.addSystemCommand(&.{ b.zig_exe, "build", "-Dhealed" }); cmd.setName("zig build -Dhealed"); cmd.expectExitCode(0); + cmd.step.dependOn(&heal_step.step); const stderr = cmd.captureStdErr(); const verify = CheckStep.create(b, exercises, stderr, true); @@ -107,6 +111,7 @@ pub fn addCliTests(b: *std.Build, exercises: []const Exercise) *Step { ); cmd.setName("zig build -Dhealed -Dn=1 start"); cmd.expectExitCode(0); + cmd.step.dependOn(&heal_step.step); const stderr = cmd.captureStdErr(); const verify = CheckStep.create(b, exercises, stderr, false); @@ -126,14 +131,16 @@ pub fn addCliTests(b: *std.Build, exercises: []const Exercise) *Step { cmd.expectExitCode(1); expectStdErrMatch(cmd, exercises[0].hint); + cmd.step.dependOn(&heal_step.step); + case_step.dependOn(&cmd.step); step.dependOn(case_step); } - // Don't add the cleanup step, since it may delete outdir while a test case - // is running. - //const cleanup = b.addRemoveDirTree(outdir); + // Don't add the cleanup step, since it may delete work_path while a test + // case is running. + //const cleanup = b.addRemoveDirTree(work_path); //step.dependOn(&cleanup.step); return step; @@ -315,8 +322,38 @@ fn fail(step: *Step, comptime format: []const u8, args: anytype) *Step { return step; } +// A step that heals exercises. +const HealStep = struct { + step: Step, + exercises: []const Exercise, + work_path: []const u8, + + pub fn create(owner: *Build, exercises: []const Exercise, work_path: []const u8) *HealStep { + const self = owner.allocator.create(HealStep) catch @panic("OOM"); + self.* = .{ + .step = Step.init(.{ + .id = .custom, + .name = "heal", + .owner = owner, + .makeFn = make, + }), + .exercises = exercises, + .work_path = work_path, + }; + + return self; + } + + fn make(step: *Step, _: *std.Progress.Node) !void { + const b = step.owner; + const self = @fieldParentPtr(HealStep, "step", step); + + return heal(b.allocator, self.exercises, self.work_path); + } +}; + // Heals all the exercises. -fn heal(allocator: Allocator, exercises: []const Exercise, outdir: []const u8) !void { +fn heal(allocator: Allocator, exercises: []const Exercise, work_path: []const u8) !void { const join = fs.path.join; const exercises_path = "exercises"; @@ -331,7 +368,7 @@ fn heal(allocator: Allocator, exercises: []const Exercise, outdir: []const u8) ! const patch_name = try fmt.allocPrint(allocator, "{s}.patch", .{name}); break :b try join(allocator, &.{ patches_path, patch_name }); }; - const output = try join(allocator, &.{ outdir, ex.main_file }); + const output = try join(allocator, &.{ work_path, ex.main_file }); const argv = &.{ "patch", "-i", patch, "-o", output, "-s", file }; From 7a40c4584e9118e1b218ff7c424e524b046abfbc Mon Sep 17 00:00:00 2001 From: Manlio Perillo Date: Tue, 2 May 2023 11:06:58 +0200 Subject: [PATCH 07/15] Restore unit tests --- .github/workflows/ci.yml | 36 ++++++++++++++++++------------------ build.zig | 5 ++--- 2 files changed, 20 insertions(+), 21 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ab821c6..d071d8f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -27,21 +27,21 @@ jobs: - name: Check compatibility with old Zig compilers run: ci/compat.sh -# test: -# name: Unit Tests -# strategy: -# matrix: -# os: [ubuntu-latest, windows-latest, macos-latest] -# runs-on: ${{ matrix.os }} -# timeout-minutes: 30 -# steps: -# - name: Checkout -# uses: actions/checkout@v3 -# -# - name: Setup Zig -# uses: goto-bus-stop/setup-zig@v2 -# with: -# version: master -# -# - name: Run unit tests -# run: zig build test + test: + name: Unit Tests + strategy: + matrix: + os: [ubuntu-latest, windows-latest, macos-latest] + runs-on: ${{ matrix.os }} + timeout-minutes: 30 + steps: + - name: Checkout + uses: actions/checkout@v3 + + - name: Setup Zig + uses: goto-bus-stop/setup-zig@v2 + with: + version: master + + - name: Run unit tests + run: zig build test diff --git a/build.zig b/build.zig index 38890c2..383a231 100644 --- a/build.zig +++ b/build.zig @@ -210,9 +210,8 @@ pub fn build(b: *Build) !void { } ziglings_step.dependOn(prev_step); - // Disabled, see issue 272 - // const test_step = b.step("test", "Run all the tests"); - // // test_step.dependOn(tests.addCliTests(b, &exercises)); + const test_step = b.step("test", "Run all the tests"); + test_step.dependOn(tests.addCliTests(b, &exercises)); } var use_color_escapes = false; From 27b941fdaf5dbc1311e9ad344a16376bfbcf4013 Mon Sep 17 00:00:00 2001 From: Manlio Perillo Date: Mon, 1 May 2023 17:12:05 +0200 Subject: [PATCH 08/15] build: remove the ZiglingStep.makeInternal method Rename the doCompile method to compile and add the run method. The two methods are now called from the make method. Add the help method, since the error handling of compile and run methods are now separate. Remove the obsolete comment for the compile method. --- build.zig | 52 +++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 37 insertions(+), 15 deletions(-) diff --git a/build.zig b/build.zig index 3b59d70..519ab85 100644 --- a/build.zig +++ b/build.zig @@ -240,7 +240,7 @@ const ZiglingStep = struct { return self; } - fn make(step: *Step, prog_node: *std.Progress.Node) anyerror!void { + fn make(step: *Step, prog_node: *std.Progress.Node) !void { const self = @fieldParentPtr(@This(), "step", step); if (self.exercise.skip) { @@ -248,28 +248,33 @@ const ZiglingStep = struct { return; } - self.makeInternal(prog_node) catch { + + const exe_path = self.compile(prog_node) catch { if (self.exercise.hint.len > 0) { print("\n{s}HINT: {s}{s}", .{ bold_text, self.exercise.hint, reset_text }); } - print("\n{s}Edit exercises/{s} and run this again.{s}", .{ red_text, self.exercise.main_file, reset_text }); - print("\n{s}To continue from this zigling, use this command:{s}\n {s}zig build -Dn={s}{s}\n", .{ red_text, reset_text, bold_text, self.exercise.key(), reset_text }); + self.help(); + std.os.exit(1); + }; + + self.run(exe_path, prog_node) catch { + if (self.exercise.hint.len > 0) { + print("\n{s}HINT: {s}{s}", .{ bold_text, self.exercise.hint, reset_text }); + } + + self.help(); std.os.exit(1); }; } - fn makeInternal(self: *@This(), prog_node: *std.Progress.Node) !void { - print("Compiling {s}...\n", .{self.exercise.main_file}); - - const exe_file = try self.doCompile(prog_node); - + fn run(self: *@This(), exe_path: []const u8, _: *std.Progress.Node) !void { resetLine(); print("Checking {s}...\n", .{self.exercise.main_file}); const cwd = self.builder.build_root.path.?; - const argv = [_][]const u8{exe_file}; + const argv = [_][]const u8{exe_path}; var child = std.ChildProcess.init(&argv, self.builder.allocator); @@ -336,9 +341,9 @@ const ZiglingStep = struct { print("{s}PASSED:\n{s}{s}\n\n", .{ green_text, trimOutput, reset_text }); } - // The normal compile step calls os.exit, so we can't use it as a library :( - // This is a stripped down copy of std.build.LibExeObjStep.make. - fn doCompile(self: *@This(), prog_node: *std.Progress.Node) ![]const u8 { + fn compile(self: *@This(), prog_node: *std.Progress.Node) ![]const u8 { + print("Compiling {s}...\n", .{self.exercise.main_file}); + const builder = self.builder; var zig_args = std.ArrayList([]const u8).init(builder.allocator); @@ -362,7 +367,7 @@ const ZiglingStep = struct { const argv = zig_args.items; var code: u8 = undefined; - const file_name = self.eval(argv, &code, prog_node) catch |err| { + const exe_path = self.eval(argv, &code, prog_node) catch |err| { self.printErrors(); switch (err) { @@ -397,7 +402,7 @@ const ZiglingStep = struct { }; self.printErrors(); - return file_name; + return exe_path; } // Code adapted from `std.Build.execAllowFail and `std.Build.Step.evalZigProcess`. @@ -500,6 +505,23 @@ const ZiglingStep = struct { return result orelse return error.ZigIPCError; } + fn help(self: *ZiglingStep) void { + const path = self.exercise.main_file; + const key = self.exercise.key(); + + print("\n{s}Edit exercises/{s} and run this again.{s}", .{ + red_text, path, reset_text, + }); + + const format = + \\ + \\{s}To continue from this zigling, use this command:{s} + \\ {s}zig build -Dn={s}{s} + \\ + ; + print(format, .{ red_text, reset_text, bold_text, key, reset_text }); + } + fn printErrors(self: *ZiglingStep) void { resetLine(); From 40cbee8fa207c4b2d5918741493e015078d9b5fb Mon Sep 17 00:00:00 2001 From: Manlio Perillo Date: Mon, 1 May 2023 17:32:07 +0200 Subject: [PATCH 09/15] build: fix incorrect error handling in ZiglingStep.compile When handling the error from the eval method, some possible errors are ignored. The make method will only print the exercise hint and the help message. Print the unexpected error message, in the else prong. Note that FileNotFound can also be considered unexpected. --- build.zig | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/build.zig b/build.zig index 519ab85..03fad4c 100644 --- a/build.zig +++ b/build.zig @@ -395,7 +395,16 @@ const ZiglingStep = struct { for (argv) |v| print("{s} ", .{v}); print("\n", .{}); }, - else => {}, + else => { + print("{s}{s}: Unexpected error: {s}{s}\n", .{ + red_text, + self.exercise.main_file, + @errorName(err), + reset_text, + }); + for (argv) |v| print("{s} ", .{v}); + print("\n", .{}); + }, } return err; From 11d8468539f489d760098453810ed9b274813169 Mon Sep 17 00:00:00 2001 From: Manlio Perillo Date: Mon, 1 May 2023 19:27:50 +0200 Subject: [PATCH 10/15] build: use Child.exec in ZiglingStep.run Update the run method to use Child.exec, instead of Child.spawn followed by Child.wait. This simplifies the code. --- build.zig | 49 ++++++++++++++++++------------------------------- 1 file changed, 18 insertions(+), 31 deletions(-) diff --git a/build.zig b/build.zig index 03fad4c..234a58f 100644 --- a/build.zig +++ b/build.zig @@ -272,44 +272,31 @@ const ZiglingStep = struct { resetLine(); print("Checking {s}...\n", .{self.exercise.main_file}); - const cwd = self.builder.build_root.path.?; - - const argv = [_][]const u8{exe_path}; - - var child = std.ChildProcess.init(&argv, self.builder.allocator); - - child.cwd = cwd; - child.env_map = self.builder.env_map; - - child.stdin_behavior = .Inherit; - if (self.exercise.check_stdout) { - child.stdout_behavior = .Pipe; - child.stderr_behavior = .Inherit; - } else { - child.stdout_behavior = .Inherit; - child.stderr_behavior = .Pipe; - } - - child.spawn() catch |err| { - print("{s}Unable to spawn {s}: {s}{s}\n", .{ red_text, argv[0], @errorName(err), reset_text }); - return err; - }; + const b = self.step.owner; // Allow up to 1 MB of stdout capture. - const max_output_len = 1 * 1024 * 1024; - const output = if (self.exercise.check_stdout) - try child.stdout.?.reader().readAllAlloc(self.builder.allocator, max_output_len) - else - try child.stderr.?.reader().readAllAlloc(self.builder.allocator, max_output_len); + const max_output_bytes = 1 * 1024 * 1024; - // At this point stdout is closed, wait for the process to terminate. - const term = child.wait() catch |err| { - print("{s}Unable to spawn {s}: {s}{s}\n", .{ red_text, argv[0], @errorName(err), reset_text }); + var result = Child.exec(.{ + .allocator = b.allocator, + .argv = &.{exe_path}, + .cwd = b.build_root.path.?, + .cwd_dir = b.build_root.handle, + .max_output_bytes = max_output_bytes, + }) catch |err| { + print("{s}Unable to spawn {s}: {s}{s}\n", .{ + red_text, exe_path, @errorName(err), reset_text, + }); return err; }; + const output = if (self.exercise.check_stdout) + result.stdout + else + result.stderr; + // Make sure it exited cleanly. - switch (term) { + switch (result.term) { .Exited => |code| { if (code != 0) { print("{s}{s} exited with error code {d} (expected {d}){s}\n", .{ red_text, self.exercise.main_file, code, 0, reset_text }); From 3ec978d73faa9db0fdbb60e4a3c0631f0923be48 Mon Sep 17 00:00:00 2001 From: Manlio Perillo Date: Wed, 3 May 2023 09:56:29 +0200 Subject: [PATCH 11/15] build: remove ZiglingStep.builder field It is not necessary, since the builder is available in self.step.owner. --- build.zig | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/build.zig b/build.zig index 234a58f..7eb45d2 100644 --- a/build.zig +++ b/build.zig @@ -223,7 +223,6 @@ var reset_text: []const u8 = ""; const ZiglingStep = struct { step: Step, exercise: Exercise, - builder: *Build, work_path: []const u8, result_messages: []const u8 = "", @@ -234,7 +233,6 @@ const ZiglingStep = struct { self.* = .{ .step = Step.init(Step.Options{ .id = .custom, .name = exercise.main_file, .owner = builder, .makeFn = make }), .exercise = exercise, - .builder = builder, .work_path = work_path, }; return self; @@ -331,12 +329,12 @@ const ZiglingStep = struct { fn compile(self: *@This(), prog_node: *std.Progress.Node) ![]const u8 { print("Compiling {s}...\n", .{self.exercise.main_file}); - const builder = self.builder; + const b = self.step.owner; - var zig_args = std.ArrayList([]const u8).init(builder.allocator); + var zig_args = std.ArrayList([]const u8).init(b.allocator); defer zig_args.deinit(); - zig_args.append(builder.zig_exe) catch unreachable; + zig_args.append(b.zig_exe) catch unreachable; zig_args.append("build-exe") catch unreachable; // Enable C support for exercises that use C functions @@ -344,11 +342,11 @@ const ZiglingStep = struct { zig_args.append("-lc") catch unreachable; } - const zig_file = join(builder.allocator, &.{ self.work_path, self.exercise.main_file }) catch unreachable; - zig_args.append(builder.pathFromRoot(zig_file)) catch unreachable; + const zig_file = join(b.allocator, &.{ self.work_path, self.exercise.main_file }) catch unreachable; + zig_args.append(b.pathFromRoot(zig_file)) catch unreachable; zig_args.append("--cache-dir") catch unreachable; - zig_args.append(builder.pathFromRoot(builder.cache_root.path.?)) catch unreachable; + zig_args.append(b.pathFromRoot(b.cache_root.path.?)) catch unreachable; zig_args.append("--listen=-") catch unreachable; From feeba51940fa52835c69f22b7694ffeea8f1cc08 Mon Sep 17 00:00:00 2001 From: Manlio Perillo Date: Wed, 3 May 2023 12:34:33 +0200 Subject: [PATCH 12/15] build: don't use @This() in ZiglingStep Use ZiglingStep, instead. This is consistent with the coding style in std.Build. --- build.zig | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/build.zig b/build.zig index 7eb45d2..29d4487 100644 --- a/build.zig +++ b/build.zig @@ -228,8 +228,8 @@ const ZiglingStep = struct { result_messages: []const u8 = "", result_error_bundle: std.zig.ErrorBundle = std.zig.ErrorBundle.empty, - pub fn create(builder: *Build, exercise: Exercise, work_path: []const u8) *@This() { - const self = builder.allocator.create(@This()) catch unreachable; + pub fn create(builder: *Build, exercise: Exercise, work_path: []const u8) *ZiglingStep { + const self = builder.allocator.create(ZiglingStep) catch unreachable; self.* = .{ .step = Step.init(Step.Options{ .id = .custom, .name = exercise.main_file, .owner = builder, .makeFn = make }), .exercise = exercise, @@ -239,7 +239,7 @@ const ZiglingStep = struct { } fn make(step: *Step, prog_node: *std.Progress.Node) !void { - const self = @fieldParentPtr(@This(), "step", step); + const self = @fieldParentPtr(ZiglingStep, "step", step); if (self.exercise.skip) { print("Skipping {s}\n\n", .{self.exercise.main_file}); @@ -266,7 +266,7 @@ const ZiglingStep = struct { }; } - fn run(self: *@This(), exe_path: []const u8, _: *std.Progress.Node) !void { + fn run(self: *ZiglingStep, exe_path: []const u8, _: *std.Progress.Node) !void { resetLine(); print("Checking {s}...\n", .{self.exercise.main_file}); @@ -326,7 +326,7 @@ const ZiglingStep = struct { print("{s}PASSED:\n{s}{s}\n\n", .{ green_text, trimOutput, reset_text }); } - fn compile(self: *@This(), prog_node: *std.Progress.Node) ![]const u8 { + fn compile(self: *ZiglingStep, prog_node: *std.Progress.Node) ![]const u8 { print("Compiling {s}...\n", .{self.exercise.main_file}); const b = self.step.owner; From c6c6a32270d63a2efdedea02305bc284046a63af Mon Sep 17 00:00:00 2001 From: Manlio Perillo Date: Wed, 3 May 2023 14:39:42 +0200 Subject: [PATCH 13/15] build: improve the exercise output check Make the error message consistent with the one in std.Build.RunStep, using the "=" character instead of "-" and correctly aligning the text. --- build.zig | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/build.zig b/build.zig index 29d4487..c9914e0 100644 --- a/build.zig +++ b/build.zig @@ -313,11 +313,11 @@ const ZiglingStep = struct { if (!std.mem.eql(u8, trimOutput, trimExerciseOutput)) { print( \\ - \\{s}----------- Expected this output -----------{s} - \\"{s}" - \\{s}----------- but found -----------{s} - \\"{s}" - \\{s}-----------{s} + \\{s}========= expected this output: =========={s} + \\{s} + \\{s}========= but found: ====================={s} + \\{s} + \\{s}=========================================={s} \\ , .{ red_text, reset_text, trimExerciseOutput, red_text, reset_text, trimOutput, red_text, reset_text }); return error.InvalidOutput; From 771b499cbc2d3519a663dc194023dfbfbf2dda42 Mon Sep 17 00:00:00 2001 From: Manlio Perillo Date: Wed, 3 May 2023 15:25:55 +0200 Subject: [PATCH 14/15] build: use @panic("OOM") instead of unreachable The code in ZiglingStep copied the error handling used in std.Build in the past. Use @panic("OOM") when the error is caused by the allocator failing to allocate memory. --- build.zig | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/build.zig b/build.zig index c9914e0..4ca9275 100644 --- a/build.zig +++ b/build.zig @@ -229,7 +229,7 @@ const ZiglingStep = struct { result_error_bundle: std.zig.ErrorBundle = std.zig.ErrorBundle.empty, pub fn create(builder: *Build, exercise: Exercise, work_path: []const u8) *ZiglingStep { - const self = builder.allocator.create(ZiglingStep) catch unreachable; + const self = builder.allocator.create(ZiglingStep) catch @panic("OOM"); self.* = .{ .step = Step.init(Step.Options{ .id = .custom, .name = exercise.main_file, .owner = builder, .makeFn = make }), .exercise = exercise, @@ -334,21 +334,21 @@ const ZiglingStep = struct { var zig_args = std.ArrayList([]const u8).init(b.allocator); defer zig_args.deinit(); - zig_args.append(b.zig_exe) catch unreachable; - zig_args.append("build-exe") catch unreachable; + zig_args.append(b.zig_exe) catch @panic("OOM"); + zig_args.append("build-exe") catch @panic("OOM"); // Enable C support for exercises that use C functions if (self.exercise.link_libc) { - zig_args.append("-lc") catch unreachable; + zig_args.append("-lc") catch @panic("OOM"); } - const zig_file = join(b.allocator, &.{ self.work_path, self.exercise.main_file }) catch unreachable; - zig_args.append(b.pathFromRoot(zig_file)) catch unreachable; + const zig_file = join(b.allocator, &.{ self.work_path, self.exercise.main_file }) catch @panic("OOM"); + zig_args.append(b.pathFromRoot(zig_file)) catch @panic("OOM"); - zig_args.append("--cache-dir") catch unreachable; - zig_args.append(b.pathFromRoot(b.cache_root.path.?)) catch unreachable; + zig_args.append("--cache-dir") catch @panic("OOM"); + zig_args.append(b.pathFromRoot(b.cache_root.path.?)) catch @panic("OOM"); - zig_args.append("--listen=-") catch unreachable; + zig_args.append("--listen=-") catch @panic("OOM"); const argv = zig_args.items; var code: u8 = undefined; From a5d93c0b2035915dbceb778acb44cb08e5d76684 Mon Sep 17 00:00:00 2001 From: Manlio Perillo Date: Wed, 3 May 2023 17:44:47 +0200 Subject: [PATCH 15/15] build: improve coding style in ZiglingStep - Use an anonymous struct when initializing std.Build.Step. - Rename the builder parameter in the create method to b - Avoid lines too long Additionally: - In the run method, rename output to raw_output in order to make the next variable names shorter. - In the compile method, rename zig_file to path. --- build.zig | 71 +++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 45 insertions(+), 26 deletions(-) diff --git a/build.zig b/build.zig index 4ca9275..64968b9 100644 --- a/build.zig +++ b/build.zig @@ -228,10 +228,15 @@ const ZiglingStep = struct { result_messages: []const u8 = "", result_error_bundle: std.zig.ErrorBundle = std.zig.ErrorBundle.empty, - pub fn create(builder: *Build, exercise: Exercise, work_path: []const u8) *ZiglingStep { - const self = builder.allocator.create(ZiglingStep) catch @panic("OOM"); + pub fn create(b: *Build, exercise: Exercise, work_path: []const u8) *ZiglingStep { + const self = b.allocator.create(ZiglingStep) catch @panic("OOM"); self.* = .{ - .step = Step.init(Step.Options{ .id = .custom, .name = exercise.main_file, .owner = builder, .makeFn = make }), + .step = Step.init(.{ + .id = .custom, + .name = exercise.main_file, + .owner = b, + .makeFn = make, + }), .exercise = exercise, .work_path = work_path, }; @@ -249,7 +254,9 @@ const ZiglingStep = struct { const exe_path = self.compile(prog_node) catch { if (self.exercise.hint.len > 0) { - print("\n{s}HINT: {s}{s}", .{ bold_text, self.exercise.hint, reset_text }); + print("\n{s}HINT: {s}{s}", .{ + bold_text, self.exercise.hint, reset_text, + }); } self.help(); @@ -258,7 +265,9 @@ const ZiglingStep = struct { self.run(exe_path, prog_node) catch { if (self.exercise.hint.len > 0) { - print("\n{s}HINT: {s}{s}", .{ bold_text, self.exercise.hint, reset_text }); + print("\n{s}HINT: {s}{s}", .{ + bold_text, self.exercise.hint, reset_text, + }); } self.help(); @@ -288,7 +297,7 @@ const ZiglingStep = struct { return err; }; - const output = if (self.exercise.check_stdout) + const raw_output = if (self.exercise.check_stdout) result.stdout else result.stderr; @@ -297,20 +306,27 @@ const ZiglingStep = struct { switch (result.term) { .Exited => |code| { if (code != 0) { - print("{s}{s} exited with error code {d} (expected {d}){s}\n", .{ red_text, self.exercise.main_file, code, 0, reset_text }); + print("{s}{s} exited with error code {d} (expected {d}){s}\n", .{ + red_text, self.exercise.main_file, code, 0, reset_text, + }); return error.BadExitCode; } }, else => { - print("{s}{s} terminated unexpectedly{s}\n", .{ red_text, self.exercise.main_file, reset_text }); + print("{s}{s} terminated unexpectedly{s}\n", .{ + red_text, self.exercise.main_file, reset_text, + }); return error.UnexpectedTermination; }, } // Validate the output. - const trimOutput = std.mem.trimRight(u8, output, " \r\n"); - const trimExerciseOutput = std.mem.trimRight(u8, self.exercise.output, " \r\n"); - if (!std.mem.eql(u8, trimOutput, trimExerciseOutput)) { + const output = std.mem.trimRight(u8, raw_output, " \r\n"); + const exercise_output = std.mem.trimRight(u8, self.exercise.output, " \r\n"); + if (!std.mem.eql(u8, output, exercise_output)) { + const red = red_text; + const reset = reset_text; + print( \\ \\{s}========= expected this output: =========={s} @@ -319,17 +335,20 @@ const ZiglingStep = struct { \\{s} \\{s}=========================================={s} \\ - , .{ red_text, reset_text, trimExerciseOutput, red_text, reset_text, trimOutput, red_text, reset_text }); + , .{ red, reset, exercise_output, red, reset, output, red, reset }); return error.InvalidOutput; } - print("{s}PASSED:\n{s}{s}\n\n", .{ green_text, trimOutput, reset_text }); + print("{s}PASSED:\n{s}{s}\n\n", .{ green_text, output, reset_text }); } fn compile(self: *ZiglingStep, prog_node: *std.Progress.Node) ![]const u8 { print("Compiling {s}...\n", .{self.exercise.main_file}); const b = self.step.owner; + const exercise_path = self.exercise.main_file; + const path = join(b.allocator, &.{ self.work_path, exercise_path }) catch + @panic("OOM"); var zig_args = std.ArrayList([]const u8).init(b.allocator); defer zig_args.deinit(); @@ -337,13 +356,12 @@ const ZiglingStep = struct { zig_args.append(b.zig_exe) catch @panic("OOM"); zig_args.append("build-exe") catch @panic("OOM"); - // Enable C support for exercises that use C functions + // Enable C support for exercises that use C functions. if (self.exercise.link_libc) { zig_args.append("-lc") catch @panic("OOM"); } - const zig_file = join(b.allocator, &.{ self.work_path, self.exercise.main_file }) catch @panic("OOM"); - zig_args.append(b.pathFromRoot(zig_file)) catch @panic("OOM"); + zig_args.append(b.pathFromRoot(path)) catch @panic("OOM"); zig_args.append("--cache-dir") catch @panic("OOM"); zig_args.append(b.pathFromRoot(b.cache_root.path.?)) catch @panic("OOM"); @@ -357,35 +375,36 @@ const ZiglingStep = struct { switch (err) { error.FileNotFound => { - print("{s}{s}: Unable to spawn the following command: file not found{s}\n", .{ red_text, self.exercise.main_file, reset_text }); + print("{s}{s}: Unable to spawn the following command: file not found{s}\n", .{ + red_text, self.exercise.main_file, reset_text, + }); for (argv) |v| print("{s} ", .{v}); print("\n", .{}); }, error.ExitCodeFailure => { - print("{s}{s}: The following command exited with error code {}:{s}\n", .{ red_text, self.exercise.main_file, code, reset_text }); + print("{s}{s}: The following command exited with error code {}:{s}\n", .{ + red_text, self.exercise.main_file, code, reset_text, + }); for (argv) |v| print("{s} ", .{v}); print("\n", .{}); }, error.ProcessTerminated => { - print("{s}{s}: The following command terminated unexpectedly:{s}\n", .{ red_text, self.exercise.main_file, reset_text }); + print("{s}{s}: The following command terminated unexpectedly:{s}\n", .{ + red_text, self.exercise.main_file, reset_text, + }); for (argv) |v| print("{s} ", .{v}); print("\n", .{}); }, error.ZigIPCError => { print("{s}{s}: The following command failed to communicate the compilation result:{s}\n", .{ - red_text, - self.exercise.main_file, - reset_text, + red_text, self.exercise.main_file, reset_text, }); for (argv) |v| print("{s} ", .{v}); print("\n", .{}); }, else => { print("{s}{s}: Unexpected error: {s}{s}\n", .{ - red_text, - self.exercise.main_file, - @errorName(err), - reset_text, + red_text, self.exercise.main_file, @errorName(err), reset_text, }); for (argv) |v| print("{s} ", .{v}); print("\n", .{});