Re: [PATCH v6 1/4] last-modified: new subcommand to show when files were last modified

[Patrick Steinhardt <ps@pks.im>]

On Fri, Aug 01, 2025 at 06:22:50PM +0200, Toon Claes wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> > On Wed, Jul 30, 2025 at 07:55:07PM +0200, Toon Claes wrote:
> >> diff --git a/builtin/last-modified.c b/builtin/last-modified.c
> >> new file mode 100644
> >> index 0000000000..e4c73464c7
> >> --- /dev/null
> >> +++ b/builtin/last-modified.c
> >> +static int last_modified_run(struct last_modified *lm)
> >> +{
> >> +	struct last_modified_callback_data data = { .lm = lm };
> >> +
> >> +	lm->rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
> >> +	lm->rev.diffopt.format_callback = last_modified_diff;
> >> +	lm->rev.diffopt.format_callback_data = &data;
> >> +
> >> +	prepare_revision_walk(&lm->rev);
> >> +
> >> +	while (hashmap_get_size(&lm->paths)) {
> >> +		data.commit = get_revision(&lm->rev);
> >> +		if (!data.commit)
> >> +			break;
> >
> > So in this case we have reached the end of our commit range. I assume we
> > simply print the oldest commit of that range in this case?
> 
> Looking at this more in detail, I feel we should be calling BUG here.
> When we've hit the boundary commit, we should be printing the remaining
> paths with that commit, but with a caret `^` prepended. If we hit this
> condition it means we went beyond the boundary, but still have paths
> remaining. That's a bug.
> 
> But... As a matter of fact. I had a test failing (on the commit using
> bloom filters). It didn't print remaining paths with the boundary commit
> with a caret. This happens only when having GIT_TEST_COMMIT_GRAPH and
> GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS set. And it's perfectly explainable
> now:
> 
> With those set, we hit this exit condition. This happens because
> maybe_changed_path() was called in previous loop, returning false. Then
> we hit this exit, and un-printed paths remain. Big thanks for this hint.

Nice :)

Patrick