diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a525b5d54..0e988704b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -142,37 +142,18 @@ Zig cannot detect by itself. ### On Linux -On Linux the recommended tool to check for memory leaks is Valgrind. We supply -a file containing suppressions for false positives and known leaks in 3rd party -libraries. The recommended way to run Valgrind is: +On Linux the recommended tool to check for memory leaks is Valgrind. The +recommended way to run Valgrind is via `zig build`: -``` -zig build -Dcpu=baseline -Doptimize=Debug -valgrind \ - --leak-check=full \ - --num-callers=50 \ - --suppressions=valgrind.supp \ - ./zig-out/bin/ghostty +```sh +zig build run-valgrind ``` -> [!NOTE] -> -> `-Dcpu=baseline` may not be needed depending on your CPU, but Valgrind cannot -> deal with some instructions on certain newer CPUs so using `-Dcpu=baseline` -> doesn't hurt. +This builds a Ghostty executable with Valgrind support and runs Valgrind +with the proper flags to ensure we're suppressing known false positives. -Any leaks found by Valgrind should be investigated. - -If you use Nix, you can use the following commands to run Valgrind so that you -don't need to look up the Valgrind invocation every time: - -``` -nix develop -nix run .#valgrind -- --config-default-files=true -``` - -You can add any Ghostty CLI arguments after the `--` and they will be passed to -the invocation of Ghostty. +You can combine the same build args with `run-valgrind` that you can with +`run`, such as specifying additional configurations after a trailing `--`. ## Input Stack Testing diff --git a/build.zig b/build.zig index 4acca53cc..38cfd0e56 100644 --- a/build.zig +++ b/build.zig @@ -19,7 +19,15 @@ pub fn build(b: *std.Build) !void { // All our steps which we'll hook up later. The steps are shown // up here just so that they are more self-documenting. const run_step = b.step("run", "Run the app"); - const test_step = b.step("test", "Run all tests"); + const run_valgrind_step = b.step( + "run-valgrind", + "Run the app under valgrind", + ); + const test_step = b.step("test", "Run tests"); + const test_valgrind_step = b.step( + "test-valgrind", + "Run tests under valgrind", + ); const translations_step = b.step( "update-translations", "Update translation files", @@ -77,9 +85,11 @@ pub fn build(b: *std.Build) !void { // Runtime "none" is libghostty, anything else is an executable. if (config.app_runtime != .none) { - exe.install(); - resources.install(); - if (i18n) |v| v.install(); + if (config.emit_exe) { + exe.install(); + resources.install(); + if (i18n) |v| v.install(); + } } else { // Libghostty // @@ -181,6 +191,31 @@ pub fn build(b: *std.Build) !void { } } + // Valgrind + if (config.app_runtime != .none) { + // We need to rebuild Ghostty with a baseline CPU target. + const valgrind_exe = exe: { + var valgrind_config = config; + valgrind_config.target = valgrind_config.baselineTarget(); + break :exe try buildpkg.GhosttyExe.init( + b, + &valgrind_config, + &deps, + ); + }; + + const run_cmd = b.addSystemCommand(&.{ + "valgrind", + "--leak-check=full", + "--num-callers=50", + b.fmt("--suppressions={s}", .{b.pathFromRoot("valgrind.supp")}), + "--gen-suppressions=all", + }); + run_cmd.addArtifactArg(valgrind_exe.exe); + if (b.args) |args| run_cmd.addArgs(args); + run_valgrind_step.dependOn(&run_cmd.step); + } + // Tests { const test_exe = b.addTest(.{ @@ -188,7 +223,7 @@ pub fn build(b: *std.Build) !void { .filters = if (test_filter) |v| &.{v} else &.{}, .root_module = b.createModule(.{ .root_source_file = b.path("src/main.zig"), - .target = config.target, + .target = config.baselineTarget(), .optimize = .Debug, .strip = false, .omit_frame_pointer = false, @@ -198,8 +233,21 @@ pub fn build(b: *std.Build) !void { if (config.emit_test_exe) b.installArtifact(test_exe); _ = try deps.add(test_exe); + + // Normal test running const test_run = b.addRunArtifact(test_exe); test_step.dependOn(&test_run.step); + + // Valgrind test running + const valgrind_run = b.addSystemCommand(&.{ + "valgrind", + "--leak-check=full", + "--num-callers=50", + b.fmt("--suppressions={s}", .{b.pathFromRoot("valgrind.supp")}), + "--gen-suppressions=all", + }); + valgrind_run.addArtifactArg(test_exe); + test_valgrind_step.dependOn(&valgrind_run.step); } // update-translations does what it sounds like and updates the "pot" diff --git a/flake.nix b/flake.nix index 1c36b64ac..7cf58b27c 100644 --- a/flake.nix +++ b/flake.nix @@ -94,19 +94,6 @@ x11-gnome = runVM ./nix/vm/x11-gnome.nix; x11-plasma6 = runVM ./nix/vm/x11-plasma6.nix; x11-xfce = runVM ./nix/vm/x11-xfce.nix; - valgrind = let - script = pkgs.writeShellScript "valgrind" '' - zig build -Dcpu=baseline -Doptimize=Debug - valgrind \ - --leak-check=full \ - --num-callers=50 \ - --suppressions=valgrind.supp \ - ./zig-out/bin/ghostty "$@" - ''; - in { - type = "app"; - program = "${script}"; - }; }; } # Our supported systems are the same supported systems as the Zig binaries. diff --git a/src/build/Config.zig b/src/build/Config.zig index 175745dc6..fd892f16c 100644 --- a/src/build/Config.zig +++ b/src/build/Config.zig @@ -53,6 +53,7 @@ patch_rpath: ?[]const u8 = null, flatpak: bool = false, emit_bench: bool = false, emit_docs: bool = false, +emit_exe: bool = false, emit_helpgen: bool = false, emit_macos_app: bool = false, emit_terminfo: bool = false, @@ -286,6 +287,12 @@ pub fn init(b: *std.Build) !Config { //--------------------------------------------------------------- // Artifacts to Emit + config.emit_exe = b.option( + bool, + "emit-exe", + "Build and install main executables with 'build'", + ) orelse true; + config.emit_test_exe = b.option( bool, "emit-test-exe", @@ -460,6 +467,22 @@ pub fn addOptions(self: *const Config, step: *std.Build.Step.Options) !void { ); } +/// Returns a baseline CPU target retaining all the other CPU configs. +pub fn baselineTarget(self: *const Config) std.Build.ResolvedTarget { + // Set our cpu model as baseline. There may need to be other modifications + // we need to make such as resetting CPU features but for now this works. + var q = self.target.query; + q.cpu_model = .baseline; + + // Same logic as build.resolveTargetQuery but we don't need to + // handle the native case. + return .{ + .query = q, + .result = std.zig.system.resolveTargetQuery(q) catch + @panic("unable to resolve baseline query"), + }; +} + /// Rehydrate our Config from the comptime options. Note that not all /// options are available at comptime, so look closely at this implementation /// to see what is and isn't available.