From fb5b04f374c58ba7dbc37694a44bb6e3b628e419 Mon Sep 17 00:00:00 2001 From: Manlio Perillo Date: Sun, 7 May 2023 16:25:35 +0200 Subject: [PATCH 01/10] build: fix `zig build -Dhealed install` The command fails because the path to the exercises directory was incorrectly set to "exercises" instead of `work_path`. The bug was introduced in commit b56bb7b (build: enable full parallelism when -Dhealed is set). Remove the comment about not using multi-object loop, since it is confusing. --- build.zig | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/build.zig b/build.zig index 5168d3d..68f43eb 100644 --- a/build.zig +++ b/build.zig @@ -198,11 +198,9 @@ pub fn build(b: *Build) !void { const ziglings_step = b.step("ziglings", "Check all ziglings"); b.default_step = ziglings_step; - // Don't use the "multi-object for loop" syntax, in order to avoid a syntax - // error with old Zig compilers. var prev_step = &header_step.step; for (exercises) |ex| { - const build_step = ex.addExecutable(b, "exercises"); + const build_step = ex.addExecutable(b, work_path); b.installArtifact(build_step); const verify_stepn = ZiglingStep.create(b, ex, work_path); From 1ed58a1128e7d396afe0f602d6d76e4018337be6 Mon Sep 17 00:00:00 2001 From: Manlio Perillo Date: Sun, 7 May 2023 20:01:02 +0200 Subject: [PATCH 02/10] build: don't install skipped exercises Update the code in `zig build install` and `zig build -Dn=n install`, so that exercises that must be skipped are not installed, since it will cause an error. Ensure that a skip message is printed. --- build.zig | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/build.zig b/build.zig index 68f43eb..e6f2688 100644 --- a/build.zig +++ b/build.zig @@ -133,17 +133,20 @@ pub fn build(b: *Build) !void { print("unknown exercise number: {}\n", .{n}); std.os.exit(1); } - const ex = exercises[n - 1]; const build_step = ex.addExecutable(b, work_path); - b.installArtifact(build_step); + + const skip_step = SkipStep.create(b, ex); + if (!ex.skip) + b.installArtifact(build_step) + else + b.getInstallStep().dependOn(&skip_step.step); const run_step = b.addRunArtifact(build_step); const test_step = b.step("test", b.fmt("Run {s} without checking output", .{ex.main_file})); if (ex.skip) { - const skip_step = SkipStep.create(b, ex); test_step.dependOn(&skip_step.step); } else { test_step.dependOn(&run_step.step); @@ -201,7 +204,12 @@ pub fn build(b: *Build) !void { var prev_step = &header_step.step; for (exercises) |ex| { const build_step = ex.addExecutable(b, work_path); - b.installArtifact(build_step); + + const skip_step = SkipStep.create(b, ex); + if (!ex.skip) + b.installArtifact(build_step) + else + b.getInstallStep().dependOn(&skip_step.step); const verify_stepn = ZiglingStep.create(b, ex, work_path); verify_stepn.step.dependOn(prev_step); From 9c3ea769dc58a8b287a8d2766a9d826e40a306e6 Mon Sep 17 00:00:00 2001 From: Manlio Perillo Date: Sun, 7 May 2023 20:07:26 +0200 Subject: [PATCH 03/10] build: use self when using @fieldParentPtr Update PrintStep and SkipStep to use the `self` variable when getting the parent pointer from Step. This convention is used in `std.Build`. --- build.zig | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/build.zig b/build.zig index e6f2688..0fc7ca5 100644 --- a/build.zig +++ b/build.zig @@ -616,9 +616,9 @@ const PrintStep = struct { fn make(step: *Step, prog_node: *std.Progress.Node) !void { _ = prog_node; - const p = @fieldParentPtr(PrintStep, "step", step); + const self = @fieldParentPtr(PrintStep, "step", step); - print("{s}", .{p.message}); + print("{s}", .{self.message}); } }; @@ -644,10 +644,10 @@ const SkipStep = struct { fn make(step: *Step, prog_node: *std.Progress.Node) !void { _ = prog_node; - const p = @fieldParentPtr(SkipStep, "step", step); + const self = @fieldParentPtr(SkipStep, "step", step); - if (p.exercise.skip) { - print("{s} skipped\n", .{p.exercise.main_file}); + if (self.exercise.skip) { + print("{s} skipped\n", .{self.exercise.main_file}); } } }; From 7cd439b8897842ff331f1f52991c1697121e215f Mon Sep 17 00:00:00 2001 From: Manlio Perillo Date: Sun, 7 May 2023 20:18:24 +0200 Subject: [PATCH 04/10] build: use the blank identifier in the parameter list Instead of marking a parameter as unused inside the function body. --- build.zig | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/build.zig b/build.zig index 0fc7ca5..c0515a3 100644 --- a/build.zig +++ b/build.zig @@ -614,8 +614,7 @@ const PrintStep = struct { return self; } - fn make(step: *Step, prog_node: *std.Progress.Node) !void { - _ = prog_node; + fn make(step: *Step, _: *std.Progress.Node) !void { const self = @fieldParentPtr(PrintStep, "step", step); print("{s}", .{self.message}); @@ -642,8 +641,7 @@ const SkipStep = struct { return self; } - fn make(step: *Step, prog_node: *std.Progress.Node) !void { - _ = prog_node; + fn make(step: *Step, _: *std.Progress.Node) !void { const self = @fieldParentPtr(SkipStep, "step", step); if (self.exercise.skip) { From daac879aae8456cdc644f58708b3bf5f22c50645 Mon Sep 17 00:00:00 2001 From: Manlio Perillo Date: Mon, 8 May 2023 08:56:35 +0200 Subject: [PATCH 05/10] build: fix doc-comments Some functions and custom build steps incorrectly used a normal comment. Use a doc-comment instead. Additionally, use a present tense verb to describe the action of a function or custom build step. --- build.zig | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/build.zig b/build.zig index c0515a3..f8ea9b7 100644 --- a/build.zig +++ b/build.zig @@ -568,13 +568,13 @@ const ZiglingStep = struct { } }; -// Clear the entire line and move the cursor to column zero. -// Used for clearing the compiler and build_runner progress messages. +/// Clears the entire line and move the cursor to column zero. +/// Used for clearing the compiler and build_runner progress messages. fn resetLine() void { if (use_color_escapes) print("{s}", .{"\x1b[2K\r"}); } -/// Remove trailing whitespace for each line in buf, also ensuring that there +/// Removes trailing whitespace for each line in buf, also ensuring that there /// are no trailing LF characters at the end. pub fn trimLines(allocator: std.mem.Allocator, buf: []const u8) ![]const u8 { var list = try std.ArrayList(u8).initCapacity(allocator, buf.len); @@ -594,7 +594,7 @@ pub fn trimLines(allocator: std.mem.Allocator, buf: []const u8) ![]const u8 { return std.mem.trimRight(u8, result, "\n"); } -// Print a message to stderr. +/// Prints a message to stderr. const PrintStep = struct { step: Step, message: []const u8, @@ -621,7 +621,7 @@ const PrintStep = struct { } }; -// Skip an exercise. +/// Skips an exercise. const SkipStep = struct { step: Step, exercise: Exercise, @@ -650,10 +650,10 @@ const SkipStep = struct { } }; -// Check that each exercise number, excluding the last, forms the sequence -// `[1, exercise.len)`. -// -// Additionally check that the output field lines doesn't have trailing whitespace. +/// Checks that each exercise number, excluding the last, forms the sequence +/// `[1, exercise.len)`. +/// +/// Additionally check that the output field lines doesn't have trailing whitespace. fn validate_exercises() bool { // Don't use the "multi-object for loop" syntax, in order to avoid a syntax // error with old Zig compilers. From 6f2ffbbdbcdfa707eea83803fd7521074093bb6e Mon Sep 17 00:00:00 2001 From: Manlio Perillo Date: Mon, 8 May 2023 18:11:29 +0200 Subject: [PATCH 06/10] build: add the dumpArgs function Use it in Zigling.compile, in order to reduce code duplication. --- build.zig | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/build.zig b/build.zig index f8ea9b7..9946410 100644 --- a/build.zig +++ b/build.zig @@ -388,37 +388,32 @@ const ZiglingStep = struct { 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", .{}); + dumpArgs(argv); }, error.ExitCodeFailure => { 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", .{}); + dumpArgs(argv); }, error.ProcessTerminated => { 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", .{}); + dumpArgs(argv); }, error.ZigIPCError => { // Commenting this out for now. It always shows up when compilation fails. //print("{s}{s}: The following command failed to communicate the compilation result:{s}\n", .{ // red_text, self.exercise.main_file, reset_text, //}); - //for (argv) |v| print("{s} ", .{v}); - //print("\n", .{}); + //dumpArgs(argv); }, 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", .{}); + dumpArgs(argv); }, } @@ -568,6 +563,11 @@ const ZiglingStep = struct { } }; +fn dumpArgs(args: []const []const u8) void { + for (args) |arg| print("{s} ", .{arg}); + print("\n", .{}); +} + /// Clears the entire line and move the cursor to column zero. /// Used for clearing the compiler and build_runner progress messages. fn resetLine() void { From 7b9d3ec3dfe8e185fd85ba9b1b72b2ef8aaf0f93 Mon Sep 17 00:00:00 2001 From: Manlio Perillo Date: Mon, 8 May 2023 18:30:52 +0200 Subject: [PATCH 07/10] build: improve code formatting Avoid too long lines or too many line breaks. --- build.zig | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/build.zig b/build.zig index 9946410..465a069 100644 --- a/build.zig +++ b/build.zig @@ -119,7 +119,8 @@ pub fn build(b: *Build) !void { \\ ; - const healed = b.option(bool, "healed", "Run exercises from patches/healed") orelse false; + const healed = b.option(bool, "healed", "Run exercises from patches/healed") orelse + false; const override_healed_path = b.option([]const u8, "healed-path", "Override healed path"); const exno: ?usize = b.option(usize, "n", "Select exercise"); @@ -145,7 +146,10 @@ pub fn build(b: *Build) !void { const run_step = b.addRunArtifact(build_step); - const test_step = b.step("test", b.fmt("Run {s} without checking output", .{ex.main_file})); + const test_step = b.step( + "test", + b.fmt("Run {s} without checking output", .{ex.main_file}), + ); if (ex.skip) { test_step.dependOn(&skip_step.step); } else { @@ -154,11 +158,17 @@ pub fn build(b: *Build) !void { const verify_step = ZiglingStep.create(b, ex, work_path); - const zigling_step = b.step("zigling", b.fmt("Check the solution of {s}", .{ex.main_file})); + const zigling_step = b.step( + "zigling", + b.fmt("Check the solution of {s}", .{ex.main_file}), + ); zigling_step.dependOn(&verify_step.step); b.default_step = zigling_step; - const start_step = b.step("start", b.fmt("Check all solutions starting at {s}", .{ex.main_file})); + const start_step = b.step( + "start", + b.fmt("Check all solutions starting at {s}", .{ex.main_file}), + ); var prev_step = verify_step; for (exercises) |exn| { @@ -665,9 +675,7 @@ fn validate_exercises() bool { if (exno != i and exno != last) { print("exercise {s} has an incorrect number: expected {}, got {s}\n", .{ - ex.main_file, - i, - ex.key(), + ex.main_file, i, ex.key(), }); return false; From 7cf650672756fd9783266bf8d7e8a9877b1012e5 Mon Sep 17 00:00:00 2001 From: Manlio Perillo Date: Mon, 8 May 2023 20:52:23 +0200 Subject: [PATCH 08/10] tests: remove the missing functions from RunStep Use directly the RunStep.addCheck method, instead. --- test/tests.zig | 32 +++++--------------------------- 1 file changed, 5 insertions(+), 27 deletions(-) diff --git a/test/tests.zig b/test/tests.zig index a8a7c4c..702d313 100644 --- a/test/tests.zig +++ b/test/tests.zig @@ -84,10 +84,11 @@ pub fn addCliTests(b: *std.Build, exercises: []const Exercise) *Step { b.fmt("-Dn={}", .{n}), "test", }); + const expect = b.fmt("{s} skipped", .{ex.main_file}); cmd.setName(b.fmt("zig build -Dhealed -Dn={} test", .{n})); cmd.expectExitCode(0); - cmd.expectStdOutEqual(""); - expectStdErrMatch(cmd, b.fmt("{s} skipped", .{ex.main_file})); + cmd.addCheck(.{ .expect_stdout_exact = "" }); + cmd.addCheck(.{ .expect_stderr_match = expect }); cmd.step.dependOn(&heal_step.step); @@ -172,9 +173,10 @@ pub fn addCliTests(b: *std.Build, exercises: []const Exercise) *Step { const case_step = createCase(b, "case-5"); const cmd = b.addSystemCommand(&.{ b.zig_exe, "build", "-Dn=1" }); + const expect = exercises[0].hint orelse ""; cmd.setName("zig build -Dn=1"); cmd.expectExitCode(1); - expectStdErrMatch(cmd, exercises[0].hint orelse ""); + cmd.addCheck(.{ .expect_stderr_match = expect }); cmd.step.dependOn(case_step); @@ -466,27 +468,3 @@ pub fn makeTempPath(b: *Build) ![]const u8 { return path; } - -// -// Missing functions from std.Build.RunStep -// - -/// Adds a check for stderr match. Does not add any other checks. -pub fn expectStdErrMatch(self: *RunStep, bytes: []const u8) void { - const new_check: RunStep.StdIo.Check = .{ - .expect_stderr_match = self.step.owner.dupe(bytes), - }; - self.addCheck(new_check); -} - -/// Adds a check for stdout match as well as a check for exit code 0, if -/// there is not already an expected termination check. -pub fn expectStdOutMatch(self: *RunStep, bytes: []const u8) void { - const new_check: RunStep.StdIo.Check = .{ - .expect_stdout_match = self.step.owner.dupe(bytes), - }; - self.addCheck(new_check); - if (!self.hasTermCheck()) { - self.expectExitCode(0); - } -} From ac3c2b565b6d7c1c42dd6d7225562849f742da63 Mon Sep 17 00:00:00 2001 From: Manlio Perillo Date: Tue, 9 May 2023 11:00:16 +0200 Subject: [PATCH 09/10] build: make literal paths portable Use fs.path.sep_str instead of a slash, in literal paths. --- build.zig | 6 +++++- test/tests.zig | 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/build.zig b/build.zig index 465a069..582350d 100644 --- a/build.zig +++ b/build.zig @@ -124,7 +124,11 @@ pub fn build(b: *Build) !void { const override_healed_path = b.option([]const u8, "healed-path", "Override healed path"); const exno: ?usize = b.option(usize, "n", "Select exercise"); - const healed_path = if (override_healed_path) |path| path else "patches/healed"; + const sep = std.fs.path.sep_str; + const healed_path = if (override_healed_path) |path| + path + else + "patches" ++ sep ++ "healed"; const work_path = if (healed) healed_path else "exercises"; const header_step = PrintStep.create(b, logo); diff --git a/test/tests.zig b/test/tests.zig index 702d313..2fd5ec4 100644 --- a/test/tests.zig +++ b/test/tests.zig @@ -435,10 +435,11 @@ const HealStep = struct { /// Heals all the exercises. fn heal(allocator: Allocator, exercises: []const Exercise, work_path: []const u8) !void { + const sep = std.fs.path.sep_str; const join = fs.path.join; const exercises_path = "exercises"; - const patches_path = "patches/patches"; + const patches_path = "patches" ++ sep ++ "patches"; for (exercises) |ex| { const name = ex.name(); From 6d7a4bbe2b85a2ae36ac9108951e8fd1766104eb Mon Sep 17 00:00:00 2001 From: Manlio Perillo Date: Tue, 9 May 2023 17:15:22 +0200 Subject: [PATCH 10/10] Restore unit tests Commit dbd42bb (Cleaning up zig build output) broke the unit test. Always use exit code 2, instead of 0. This is the exit code used by the build runner to notify the compiler to not report any further diagnostics. Move the Ziglings logo from the `build` function scope to the global scope, and make it public so that tests.zig can use it to find the number of lines to skip, instead of using an hard coded value. Fixes #295 --- build.zig | 40 ++++++++++++++++++++-------------------- test/tests.zig | 5 +++-- 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/build.zig b/build.zig index 582350d..c762457 100644 --- a/build.zig +++ b/build.zig @@ -74,9 +74,22 @@ pub const Exercise = struct { } }; +pub const logo = + \\ _ _ _ + \\ ___(_) __ _| (_)_ __ __ _ ___ + \\ |_ | |/ _' | | | '_ \ / _' / __| + \\ / /| | (_| | | | | | | (_| \__ \ + \\ /___|_|\__, |_|_|_| |_|\__, |___/ + \\ |___/ |___/ + \\ + \\ "Look out! Broken programs below!" + \\ + \\ +; + pub fn build(b: *Build) !void { if (!compat.is_compatible) compat.die(); - if (!validate_exercises()) std.os.exit(1); + if (!validate_exercises()) std.os.exit(2); use_color_escapes = false; if (std.io.getStdErr().supportsAnsiEscapeCodes()) { @@ -106,19 +119,6 @@ pub fn build(b: *Build) !void { reset_text = "\x1b[0m"; } - const logo = - \\ _ _ _ - \\ ___(_) __ _| (_)_ __ __ _ ___ - \\ |_ | |/ _' | | | '_ \ / _' / __| - \\ / /| | (_| | | | | | | (_| \__ \ - \\ /___|_|\__, |_|_|_| |_|\__, |___/ - \\ |___/ |___/ - \\ - \\ "Look out! Broken programs below!" - \\ - \\ - ; - const healed = b.option(bool, "healed", "Run exercises from patches/healed") orelse false; const override_healed_path = b.option([]const u8, "healed-path", "Override healed path"); @@ -136,7 +136,7 @@ pub fn build(b: *Build) !void { if (exno) |n| { if (n == 0 or n > exercises.len - 1) { print("unknown exercise number: {}\n", .{n}); - std.os.exit(1); + std.os.exit(2); } const ex = exercises[n - 1]; @@ -280,10 +280,10 @@ const ZiglingStep = struct { self.help(); - // NOTE: Returning 0 'success' status because the *exercise* failed, - // but Ziglings did not. Otherwise the learner will see this message: - // "error: the following build command failed with exit code 1:..." - std.os.exit(0); + // NOTE: Using exit code 2 will prevent the Zig compiler to print + // the message: + // "error: the following build command failed with exit code 1:..." + std.os.exit(2); }; self.run(exe_path, prog_node) catch { @@ -293,7 +293,7 @@ const ZiglingStep = struct { self.help(); // NOTE: See note above! - std.os.exit(0); + std.os.exit(2); }; } diff --git a/test/tests.zig b/test/tests.zig index 2fd5ec4..6eab08e 100644 --- a/test/tests.zig +++ b/test/tests.zig @@ -175,7 +175,7 @@ pub fn addCliTests(b: *std.Build, exercises: []const Exercise) *Step { const cmd = b.addSystemCommand(&.{ b.zig_exe, "build", "-Dn=1" }); const expect = exercises[0].hint orelse ""; cmd.setName("zig build -Dn=1"); - cmd.expectExitCode(1); + cmd.expectExitCode(2); cmd.addCheck(.{ .expect_stderr_match = expect }); cmd.step.dependOn(case_step); @@ -282,10 +282,11 @@ const CheckStep = struct { for (exercises) |ex| { if (ex.number() == 1 and self.has_logo) { // Skip the logo. + const nlines = mem.count(u8, root.logo, "\n"); var buf: [80]u8 = undefined; var lineno: usize = 0; - while (lineno < 8) : (lineno += 1) { + while (lineno < nlines) : (lineno += 1) { _ = try readLine(stderr, &buf); } }