Skip to content

pr-2169/mmontalbo/mm/line-log-tidy-proto-v1

tagged this 15 Jul 19:29
From: Michael Montalbo <mmontalbo@gmail.com>

get_commit_action() reads as a predicate that decides whether a commit
is shown or ignored, but for a line-level log without parent rewriting
it also calls line_log_process_ranges_arbitrary_commit(), which
mutates the tracked line ranges.  That hidden side effect makes it unsafe
to evaluate ahead of the walk, the way a lookahead would.

get_commit_action() was split out of simplify_commit() in beb5af43a6
(graph API: fix bug in graph_is_interesting(), 2009-08-18) as the
show/ignore decision minus the parent rewriting, so the graph renderer
could reuse it; line-level log later routed its filtering through it as
well, in 3cb9d2b6 (line-log: more responsive, incremental 'git log -L',
2020-05-11).  Besides simplify_commit(), the walk driver,
graph_is_interesting() is its only other caller, and it runs only under
--graph, which sets rewrite_parents and therefore want_ancestry(); the
"-L without ancestry" branch that holds the side effect never fires
there, so it is dormant today.

The line-level processing folds a commit's tracked ranges onto its
parents, which must happen even for a commit that get_commit_action()
filters from the output, or the ranges never reach the parents.  Move it
to simplify_commit() and run it before get_commit_action(), gated by
get_commit_action()'s leading checks (already shown, uninteresting, and
the like) so a commit ignored by those is not folded, as before; factor
those checks out as commit_early_ignore().  get_commit_action() is then
side-effect free.

commit_early_ignore() runs twice on the -L path, once for that gate and
once inside get_commit_action(), but it reads only object flags and pack
membership, disjoint from the TREESAME flag the fold sets, so the repeat
is harmless.

Add a "line-log-peek" subcommand to the revision-walking test helper
that evaluates get_commit_action() on a commit the walk has not reached
yet, plus a t4211 check that the call leaves the commit's flags
unchanged.  The flags are compared rather than the commit list because
add_line_range() merges ranges by union, which is idempotent, so the
side effect never changed which commits a linear -L history shows.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>

Submitted-As: https://lore.kernel.org/git/pull.2169.git.1784143793613.gitgitgadget@gmail.com
Assets 2
Loading