From 185a40eb75689005ace380c7dc9501db4cafe33a 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 4ae67ebf1b7027cf33ecd96317d745ccf54987fb 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 bb42451b0f6c6ce9b53ee7959a52cfc6cef0e19c 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 dd5df9f7ccce03786793b8dae4d1e7c4800f6647 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 9de89f6d604753a97afbf739f575fcd21cbd85a3 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 ea4144a41662fe42ebf2f8f5b089a6defbfaed79 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 14545778b2432bdaa2d75ce4fb9c82a3d1ef4045 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 728402c64ff1a4ed1e16c4adac704193e973dc37 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 e4e096c680fc2c8b577cd64bbffc26baf250551a 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 7aa0737929a5de8420e4a90654ed49309add1ec2 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); } }