Merge pull request #300 from perillo/simplify-build

Simplify build.zig

build.zig

@@ -64,18 +64,14 @@ pub const Exercise = struct {
     pub fn number(self: Exercise) usize {
         return std.fmt.parseInt(usize, self.key(), 10) catch unreachable;
     }
+};
 
-    /// Returns the CompileStep for this exercise.
-    pub fn addExecutable(self: Exercise, b: *Build, work_path: []const u8) *CompileStep {
-        const path = join(b.allocator, &.{ work_path, self.main_file }) catch
-            @panic("OOM");
-
-        return b.addExecutable(.{
-            .name = self.name(),
-            .root_source_file = .{ .path = path },
-            .link_libc = self.link_libc,
-        });
-    }
+/// Build mode.
+const Mode = enum {
+    /// Normal build mode: `zig build`
+    normal,
+    /// Named build mode: `zig build -Dn=n`
+    named,
 };
 
 pub const logo =
@@ -123,6 +119,9 @@ pub fn build(b: *Build) !void {
         reset_text = "\x1b[0m";
     }
 
+    // Remove the standard install and uninstall steps.
+    b.top_level_steps = .{};
+
     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");
@@ -137,101 +136,37 @@ pub fn build(b: *Build) !void {
 
     const header_step = PrintStep.create(b, logo);
 
-    // If the user pass a number for an exercise
     if (exno) |n| {
+        // Named build mode: verifies a single exercise.
         if (n == 0 or n > exercises.len - 1) {
             print("unknown exercise number: {}\n", .{n});
             std.os.exit(2);
         }
         const ex = exercises[n - 1];
 
-        const build_step = ex.addExecutable(b, work_path);
-
-        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) {
-            test_step.dependOn(&skip_step.step);
-        } else {
-            test_step.dependOn(&run_step.step);
-        }
-
-        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}),
         );
-        zigling_step.dependOn(&verify_step.step);
         b.default_step = zigling_step;
+        zigling_step.dependOn(&header_step.step);
 
-        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| {
-            const nth = exn.number();
-            if (nth > n) {
-                const verify_stepn = ZiglingStep.create(b, exn, work_path);
-                verify_stepn.step.dependOn(&prev_step.step);
+        const verify_step = ZiglingStep.create(b, ex, work_path, .named);
+        verify_step.step.dependOn(&header_step.step);
 
-                prev_step = verify_stepn;
-            }
-        }
-        start_step.dependOn(&prev_step.step);
-
-        return;
-    } else if (healed and false) {
-        // Special case when healed by the eowyn script, where we can make the
-        // code more efficient.
-        //
-        // TODO: this branch is disabled because it prevents the normal case to
-        // be executed.
-        const test_step = b.step("test", "Test the healed exercises");
-        b.default_step = test_step;
-
-        for (exercises) |ex| {
-            const build_step = ex.addExecutable(b, healed_path);
-            b.installArtifact(build_step);
-
-            const run_step = b.addRunArtifact(build_step);
-            if (ex.skip) {
-                const skip_step = SkipStep.create(b, ex);
-                test_step.dependOn(&skip_step.step);
-            } else {
-                test_step.dependOn(&run_step.step);
-            }
-        }
+        zigling_step.dependOn(&verify_step.step);
 
         return;
     }
 
-    // Run all exercises in a row
+    // Normal build mode: verifies all exercises according to the recommended
+    // order.
     const ziglings_step = b.step("ziglings", "Check all ziglings");
     b.default_step = ziglings_step;
 
     var prev_step = &header_step.step;
     for (exercises) |ex| {
-        const build_step = ex.addExecutable(b, work_path);
-
-        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);
+        const verify_stepn = ZiglingStep.create(b, ex, work_path, .normal);
         verify_stepn.step.dependOn(prev_step);
 
         prev_step = &verify_stepn.step;
@@ -252,12 +187,18 @@ const ZiglingStep = struct {
     step: Step,
     exercise: Exercise,
     work_path: []const u8,
+    mode: Mode,
 
     is_testing: bool = false,
     result_messages: []const u8 = "",
     result_error_bundle: std.zig.ErrorBundle = std.zig.ErrorBundle.empty,
 
-    pub fn create(b: *Build, exercise: Exercise, work_path: []const u8) *ZiglingStep {
+    pub fn create(
+        b: *Build,
+        exercise: Exercise,
+        work_path: []const u8,
+        mode: Mode,
+    ) *ZiglingStep {
         const self = b.allocator.create(ZiglingStep) catch @panic("OOM");
         self.* = .{
             .step = Step.init(.{
@@ -268,6 +209,7 @@ const ZiglingStep = struct {
             }),
             .exercise = exercise,
             .work_path = work_path,
+            .mode = mode,
         };
         return self;
     }
@@ -608,23 +550,18 @@ const ZiglingStep = struct {
     }
 
     fn help(self: *ZiglingStep) void {
+        const b = self.step.owner;
+        const key = self.exercise.key();
         const path = self.exercise.main_file;
 
-        print("\n{s}Edit exercises/{s} and run 'zig build' again.{s}\n", .{
-            red_text, path, reset_text,
-        });
+        const cmd = switch (self.mode) {
+            .normal => "zig build",
+            .named => b.fmt("zig build -Dn={s}", .{key}),
+        };
 
-        // NOTE: The README explains this "advanced feature" if anyone wishes to use
-        //       it. Otherwise, beginners are thinking they *have* to do this.
-        //const key = self.exercise.key();
-        //const format =
-        //    \\
-        //    \\{s}To compile only this exercise, you can also use this command:{s}
-        //    \\{s}zig build -Dn={s}{s}
-        //    \\
-        //    \\
-        //;
-        //print(format, .{ red_text, reset_text, bold_text, key, reset_text });
+        print("\n{s}Edit exercises/{s} and run '{s}' again.{s}\n", .{
+            red_text, path, cmd, reset_text,
+        });
     }
 
     fn printErrors(self: *ZiglingStep) void {
@@ -704,35 +641,6 @@ const PrintStep = struct {
     }
 };
 
-/// Skips an exercise.
-const SkipStep = struct {
-    step: Step,
-    exercise: Exercise,
-
-    pub fn create(owner: *Build, exercise: Exercise) *SkipStep {
-        const self = owner.allocator.create(SkipStep) catch @panic("OOM");
-        self.* = .{
-            .step = Step.init(.{
-                .id = .custom,
-                .name = owner.fmt("skip {s}", .{exercise.main_file}),
-                .owner = owner,
-                .makeFn = make,
-            }),
-            .exercise = exercise,
-        };
-
-        return self;
-    }
-
-    fn make(step: *Step, _: *std.Progress.Node) !void {
-        const self = @fieldParentPtr(SkipStep, "step", step);
-
-        if (self.exercise.skip) {
-            print("{s} skipped\n", .{self.exercise.main_file});
-        }
-    }
-};
-
 /// Checks that each exercise number, excluding the last, forms the sequence
 /// `[1, exercise.len)`.
 ///

test/tests.zig

@@ -20,7 +20,7 @@ pub fn addCliTests(b: *std.Build, exercises: []const Exercise) *Step {
     const step = b.step("test-cli", "Test the command line interface");
 
     {
-        // Test that `zig build -Dhealed -Dn=n test` selects the nth exercise.
+        // Test that `zig build -Dhealed -Dn=n` selects the nth exercise.
         const case_step = createCase(b, "case-1");
 
         const tmp_path = makeTempPath(b) catch |err| {
@@ -31,7 +31,6 @@ pub fn addCliTests(b: *std.Build, exercises: []const Exercise) *Step {
 
         for (exercises[0 .. exercises.len - 1]) |ex| {
             const n = ex.number();
-            if (ex.skip) continue;
 
             const cmd = b.addSystemCommand(&.{
                 b.zig_exe,
@@ -39,18 +38,13 @@ pub fn addCliTests(b: *std.Build, exercises: []const Exercise) *Step {
                 "-Dhealed",
                 b.fmt("-Dhealed-path={s}", .{tmp_path}),
                 b.fmt("-Dn={}", .{n}),
-                "test",
             });
-            cmd.setName(b.fmt("zig build -Dhealed -Dn={} test", .{n}));
+            cmd.setName(b.fmt("zig build -Dhealed -Dn={}", .{n}));
             cmd.expectExitCode(0);
             cmd.step.dependOn(&heal_step.step);
 
-            const output = if (ex.check_stdout)
-                cmd.captureStdOut()
-            else
-                cmd.captureStdErr();
-
-            const verify = CheckNamedStep.create(b, ex, output);
+            const stderr = cmd.captureStdErr();
+            const verify = CheckNamedStep.create(b, ex, stderr);
             verify.step.dependOn(&cmd.step);
 
             case_step.dependOn(&verify.step);
@@ -63,7 +57,7 @@ pub fn addCliTests(b: *std.Build, exercises: []const Exercise) *Step {
     }
 
     {
-        // Test that `zig build -Dhealed -Dn=n test` skips disabled esercises.
+        // Test that `zig build -Dhealed` processes all the exercises in order.
         const case_step = createCase(b, "case-2");
 
         const tmp_path = makeTempPath(b) catch |err| {
@@ -71,45 +65,6 @@ pub fn addCliTests(b: *std.Build, exercises: []const Exercise) *Step {
         };
 
         const heal_step = HealStep.create(b, exercises, tmp_path);
-
-        for (exercises[0 .. exercises.len - 1]) |ex| {
-            const n = ex.number();
-            if (!ex.skip) continue;
-
-            const cmd = b.addSystemCommand(&.{
-                b.zig_exe,
-                "build",
-                "-Dhealed",
-                b.fmt("-Dhealed-path={s}", .{tmp_path}),
-                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.addCheck(.{ .expect_stdout_exact = "" });
-            cmd.addCheck(.{ .expect_stderr_match = expect });
-
-            cmd.step.dependOn(&heal_step.step);
-
-            case_step.dependOn(&cmd.step);
-        }
-
-        const cleanup = b.addRemoveDirTree(tmp_path);
-        cleanup.step.dependOn(case_step);
-
-        step.dependOn(&cleanup.step);
-    }
-
-    {
-        // Test that `zig build -Dhealed` process all the exercises in order.
-        const case_step = createCase(b, "case-3");
-
-        const tmp_path = makeTempPath(b) catch |err| {
-            return fail(step, "unable to make tmp path: {s}\n", .{@errorName(err)});
-        };
-
-        const heal_step = HealStep.create(b, exercises, tmp_path);
         heal_step.step.dependOn(case_step);
 
         // TODO: when an exercise is modified, the cache is not invalidated.
@@ -124,7 +79,7 @@ pub fn addCliTests(b: *std.Build, exercises: []const Exercise) *Step {
         cmd.step.dependOn(&heal_step.step);
 
         const stderr = cmd.captureStdErr();
-        const verify = CheckStep.create(b, exercises, stderr, true);
+        const verify = CheckStep.create(b, exercises, stderr);
         verify.step.dependOn(&cmd.step);
 
         const cleanup = b.addRemoveDirTree(tmp_path);
@@ -134,53 +89,29 @@ pub fn addCliTests(b: *std.Build, exercises: []const Exercise) *Step {
     }
 
     {
-        // Test that `zig build -Dhealed -Dn=1 start` process all the exercises
-        // in order.
-        const case_step = createCase(b, "case-4");
-
-        const tmp_path = makeTempPath(b) catch |err| {
-            return fail(step, "unable to make tmp path: {s}\n", .{@errorName(err)});
-        };
-
-        const heal_step = HealStep.create(b, exercises, tmp_path);
-        heal_step.step.dependOn(case_step);
-
-        // TODO: when an exercise is modified, the cache is not invalidated.
-        const cmd = b.addSystemCommand(&.{
-            b.zig_exe,
-            "build",
-            "-Dhealed",
-            b.fmt("-Dhealed-path={s}", .{tmp_path}),
-            "-Dn=1",
-            "start",
-        });
-        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);
-        verify.step.dependOn(&cmd.step);
-
-        const cleanup = b.addRemoveDirTree(tmp_path);
-        cleanup.step.dependOn(&verify.step);
+        // Test that `zig build -Dn=n` prints the hint.
+        const case_step = createCase(b, "case-3");
 
-        step.dependOn(&cleanup.step);
-    }
+        for (exercises[0 .. exercises.len - 1]) |ex| {
+            if (ex.skip) continue;
 
-    {
-        // Test that `zig build -Dn=1` prints the hint.
-        const case_step = createCase(b, "case-5");
+            if (ex.hint) |hint| {
+                const n = ex.number();
 
-        const cmd = b.addSystemCommand(&.{ b.zig_exe, "build", "-Dn=1" });
-        const expect = exercises[0].hint orelse "";
-        cmd.setName("zig build -Dn=1");
-        cmd.expectExitCode(2);
-        cmd.addCheck(.{ .expect_stderr_match = expect });
+                const cmd = b.addSystemCommand(&.{
+                    b.zig_exe,
+                    "build",
+                    b.fmt("-Dn={}", .{n}),
+                });
+                cmd.setName(b.fmt("zig build -Dn={}", .{n}));
+                cmd.expectExitCode(2);
+                cmd.addCheck(.{ .expect_stderr_match = hint });
 
-        cmd.step.dependOn(case_step);
+                case_step.dependOn(&cmd.step);
+            }
+        }
 
-        step.dependOn(&cmd.step);
+        step.dependOn(case_step);
     }
 
     return step;
@@ -197,13 +128,13 @@ fn createCase(b: *Build, name: []const u8) *Step {
     return case_step;
 }
 
-/// Checks the output of `zig build -Dn=n test`.
+/// Checks the output of `zig build -Dn=n`.
 const CheckNamedStep = struct {
     step: Step,
     exercise: Exercise,
-    output: FileSource,
+    stderr: FileSource,
 
-    pub fn create(owner: *Build, exercise: Exercise, output: FileSource) *CheckNamedStep {
+    pub fn create(owner: *Build, exercise: Exercise, stderr: FileSource) *CheckNamedStep {
         const self = owner.allocator.create(CheckNamedStep) catch @panic("OOM");
         self.* = .{
             .step = Step.init(.{
@@ -213,7 +144,7 @@ const CheckNamedStep = struct {
                 .makeFn = make,
             }),
             .exercise = exercise,
-            .output = output,
+            .stderr = stderr,
         };
 
         return self;
@@ -222,34 +153,39 @@ const CheckNamedStep = struct {
     fn make(step: *Step, _: *std.Progress.Node) !void {
         const b = step.owner;
         const self = @fieldParentPtr(CheckNamedStep, "step", step);
+        const ex = self.exercise;
+
+        const stderr_file = try fs.cwd().openFile(
+            self.stderr.getPath(b),
+            .{ .mode = .read_only },
+        );
+        defer stderr_file.close();
 
-        // Allow up to 1 MB of output capture.
-        const max_bytes = 1 * 1024 * 1024;
-        const path = self.output.getPath(b);
-        const raw_output = try fs.cwd().readFileAlloc(b.allocator, path, max_bytes);
+        const stderr = stderr_file.reader();
+        {
+            // Skip the logo.
+            const nlines = mem.count(u8, root.logo, "\n");
+            var buf: [80]u8 = undefined;
 
-        const actual = try root.trimLines(b.allocator, raw_output);
-        const expect = self.exercise.output;
-        if (!mem.eql(u8, expect, actual)) {
-            return step.fail("{s}: expected to see \"{s}\", found \"{s}\"", .{
-                self.exercise.main_file, expect, actual,
-            });
+            var lineno: usize = 0;
+            while (lineno < nlines) : (lineno += 1) {
+                _ = try readLine(stderr, &buf);
+            }
         }
+        try check_output(step, ex, stderr);
     }
 };
 
-/// Checks the output of `zig build` or `zig build -Dn=1 start`.
+/// Checks the output of `zig build`.
 const CheckStep = struct {
     step: Step,
     exercises: []const Exercise,
     stderr: FileSource,
-    has_logo: bool,
 
     pub fn create(
         owner: *Build,
         exercises: []const Exercise,
         stderr: FileSource,
-        has_logo: bool,
     ) *CheckStep {
         const self = owner.allocator.create(CheckStep) catch @panic("OOM");
         self.* = .{
@@ -261,7 +197,6 @@ const CheckStep = struct {
             }),
             .exercises = exercises,
             .stderr = stderr,
-            .has_logo = has_logo,
         };
 
         return self;
@@ -280,7 +215,7 @@ const CheckStep = struct {
 
         const stderr = stderr_file.reader();
         for (exercises) |ex| {
-            if (ex.number() == 1 and self.has_logo) {
+            if (ex.number() == 1) {
                 // Skip the logo.
                 const nlines = mem.count(u8, root.logo, "\n");
                 var buf: [80]u8 = undefined;
@@ -293,75 +228,75 @@ const CheckStep = struct {
             try check_output(step, ex, stderr);
         }
     }
+};
 
-    fn check_output(step: *Step, exercise: Exercise, reader: Reader) !void {
-        const b = step.owner;
-
-        var buf: [1024]u8 = undefined;
-        if (exercise.skip) {
-            {
-                const actual = try readLine(reader, &buf) orelse "EOF";
-                const expect = b.fmt("Skipping {s}", .{exercise.main_file});
-                try check(step, exercise, expect, actual);
-            }
-
-            {
-                const actual = try readLine(reader, &buf) orelse "EOF";
-                try check(step, exercise, "", actual);
-            }
-
-            return;
-        }
+fn check_output(step: *Step, exercise: Exercise, reader: Reader) !void {
+    const b = step.owner;
 
+    var buf: [1024]u8 = undefined;
+    if (exercise.skip) {
         {
             const actual = try readLine(reader, &buf) orelse "EOF";
-            const expect = b.fmt("Compiling {s}...", .{exercise.main_file});
+            const expect = b.fmt("Skipping {s}", .{exercise.main_file});
             try check(step, exercise, expect, actual);
         }
 
         {
             const actual = try readLine(reader, &buf) orelse "EOF";
-            const expect = b.fmt("Checking {s}...", .{exercise.main_file});
-            try check(step, exercise, expect, actual);
+            try check(step, exercise, "", actual);
         }
 
-        {
-            const actual = try readLine(reader, &buf) orelse "EOF";
-            const expect = "PASSED:";
-            try check(step, exercise, expect, actual);
-        }
+        return;
+    }
 
-        // Skip the exercise output.
-        const nlines = 1 + mem.count(u8, exercise.output, "\n") + 1;
-        var lineno: usize = 0;
-        while (lineno < nlines) : (lineno += 1) {
-            _ = try readLine(reader, &buf) orelse @panic("EOF");
-        }
+    {
+        const actual = try readLine(reader, &buf) orelse "EOF";
+        const expect = b.fmt("Compiling {s}...", .{exercise.main_file});
+        try check(step, exercise, expect, actual);
     }
 
-    fn check(
-        step: *Step,
-        exercise: Exercise,
-        expect: []const u8,
-        actual: []const u8,
-    ) !void {
-        if (!mem.eql(u8, expect, actual)) {
-            return step.fail("{s}: expected to see \"{s}\", found \"{s}\"", .{
-                exercise.main_file,
-                expect,
-                actual,
-            });
-        }
+    {
+        const actual = try readLine(reader, &buf) orelse "EOF";
+        const expect = b.fmt("Checking {s}...", .{exercise.main_file});
+        try check(step, exercise, expect, actual);
     }
 
-    fn readLine(reader: fs.File.Reader, buf: []u8) !?[]const u8 {
-        if (try reader.readUntilDelimiterOrEof(buf, '\n')) |line| {
-            return mem.trimRight(u8, line, " \r\n");
-        }
+    {
+        const actual = try readLine(reader, &buf) orelse "EOF";
+        const expect = "PASSED:";
+        try check(step, exercise, expect, actual);
+    }
 
-        return null;
+    // Skip the exercise output.
+    const nlines = 1 + mem.count(u8, exercise.output, "\n") + 1;
+    var lineno: usize = 0;
+    while (lineno < nlines) : (lineno += 1) {
+        _ = try readLine(reader, &buf) orelse @panic("EOF");
     }
-};
+}
+
+fn check(
+    step: *Step,
+    exercise: Exercise,
+    expect: []const u8,
+    actual: []const u8,
+) !void {
+    if (!mem.eql(u8, expect, actual)) {
+        return step.fail("{s}: expected to see \"{s}\", found \"{s}\"", .{
+            exercise.main_file,
+            expect,
+            actual,
+        });
+    }
+}
+
+fn readLine(reader: fs.File.Reader, buf: []u8) !?[]const u8 {
+    if (try reader.readUntilDelimiterOrEof(buf, '\n')) |line| {
+        return mem.trimRight(u8, line, " \r\n");
+    }
+
+    return null;
+}
 
 /// Fails with a custom error message.
 const FailStep = struct {