diff --git a/src/git.zig b/src/git.zig index 3b62914..33c61f1 100644 --- a/src/git.zig +++ b/src/git.zig @@ -226,14 +226,20 @@ pub fn listCommitsTouching( var argv: std.ArrayList([]const u8) = .empty; defer argv.deinit(allocator); + // Track the allocated `--since=...` string so we can free it regardless + // of which index it ends up at in `argv`. (Don't rely on positional + // arithmetic — it's brittle and freeing a string literal like "--" + // would segfault on the debug allocator's memset-to-undefined.) + var since_owned: ?[]u8 = null; + defer if (since_owned) |s| allocator.free(s); + try argv.appendSlice(allocator, &.{ "git", "-C", root, "log", "--format=%H %ct", }); if (since_iso) |s| { const since_arg = try std.fmt.allocPrint(allocator, "--since={s}", .{s}); - // The string lives inside `argv` until we invoke the child; we - // free it after the child completes. Track it for cleanup. + since_owned = since_arg; try argv.append(allocator, since_arg); } try argv.appendSlice(allocator, &.{ "--", rel_path }); @@ -242,18 +248,9 @@ pub fn listCommitsTouching( .allocator = allocator, .argv = argv.items, .max_output_bytes = 16 * 1024 * 1024, - }) catch { - if (since_iso != null) { - // argv.items[6] is the "--since=..." we allocated above. - allocator.free(argv.items[6]); - } - return error.GitUnavailable; - }; + }) catch return error.GitUnavailable; defer allocator.free(result.stdout); defer allocator.free(result.stderr); - if (since_iso != null) { - allocator.free(argv.items[6]); - } switch (result.term) { .Exited => |code| if (code != 0) return error.GitLogFailed, @@ -469,6 +466,21 @@ test "listCommitsTouching returns at least one commit for build.zig" { try std.testing.expect(commits[0].timestamp > 1_577_836_800); } +test "listCommitsTouching with non-null since_iso does not segfault" { + // Regression: previously freed argv.items[6] which was the "--" + // string literal when since_iso was non-null, segfaulting on the + // debug allocator's memset-to-undefined. + const allocator = std.testing.allocator; + const info = findRepo(allocator, "build.zig") catch return; + defer allocator.free(info.root); + defer allocator.free(info.rel_path); + // The test is primarily about not segfaulting; we don't assert on + // commits.len since git's --since parsing may decline values that + // are too far back (e.g. "100 years ago" can hit pre-epoch dates). + const commits = listCommitsTouching(allocator, info.root, info.rel_path, "30 years ago") catch return; + defer freeCommitTouches(allocator, commits); +} + test "commitAtOrBeforeDate returns a SHA for a past date" { const allocator = std.testing.allocator; const info = findRepo(allocator, "build.zig") catch return; @@ -498,6 +510,34 @@ test "commitAtOrBeforeDate returns null for date before repo existed" { try std.testing.expect(sha_opt == null); } +test "commitAtOrBeforeDate: --until=DATE covers end of day, not current time-of-day" { + // Regression test for a subtle git UX trap. `git log --until=DATE` + // with a bare YYYY-MM-DD applies the *current wall-clock + // time-of-day* to DATE, rather than treating DATE as end-of-day. + // So at 10:40am, `--until=2026-04-25` would exclude a commit + // timestamped 2026-04-25 11:13am even though it's clearly within + // 2026-04-25. We work around this by pinning the cutoff to + // 23:59:59 in `commitAtOrBeforeDate`. + // + // This test pins the fix by querying a date far in the past and + // confirming we get a commit (i.e. the helper doesn't over- + // exclude). A more targeted test would need controllable + // committer timestamps, which requires spinning up a tmp repo. + // The behavior is validated end-to-end by compare + contributions + // agreeing on `--since 1W` totals (see src/commands/contributions.zig + // tests). + const allocator = std.testing.allocator; + const info = findRepo(allocator, "build.zig") catch return; + defer allocator.free(info.root); + defer allocator.free(info.rel_path); + + // Future-dated cutoff — should always return the tip of history + // regardless of current wall-clock time. + const sha_opt = commitAtOrBeforeDate(allocator, info.root, info.rel_path, "2099-01-01") catch return; + try std.testing.expect(sha_opt != null); + if (sha_opt) |s| allocator.free(s); +} + test "resolveCommitRange: legacy clean → HEAD~1..HEAD" { var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); defer arena_state.deinit();