Thread (53 messages) 53 messages, 6 authors, 4d ago

Re: [PATCH v6 2/3] revision: add peek functions for lookahead

From: Junio C Hamano <hidden>
Date: 2026-06-20 15:15:07

Junio C Hamano [off-list ref] writes:
I _think_ this is OK, as "--graph" sets .rewrite_parents bit (as
well as .topo_order bit) on, which makes want_ancestry() to return
true.  Which in turn means even if -L is in effect, we will not call
line_log_process_ranges_arbitrary_commit() that is the only source
of side effect in this function.  Somebody needs to sanity check
this, but we may want to leave an in-code comment to warn future
developers not to call get_commit_action() on random commits outside
of the normal history traversal under what condition (namely, -L
without rewrite_parents).

Even better, perhaps add

	if (revs->line_level_traverse && !want_ancestry(revs))
		BUG("do not call this");

at the beginning of revision_has_commits_after() function, and
describe why in the header file comment for this function below?
Having said all that, in the longer term we might be better off if
we fix the line-log code so that get_commit_action() becomes a pure
function again.

It might be a very simple change to move the "if we are doing -L and
!want_ancestry(), call the line_log_process_ranges_arbitrary_commit()"
to simplify_commit() before it calls get_commit_action(), but I
haven't thought things through.

[jc: Michael Cc'ed as there are a few topics on line-log recently
from him; SZEDER Cc'ed as his 3cb9d2b6 (line-log: more responsive,
incremental 'git log -L', 2020-05-11) introduced this side effect
there.]

In any case, this is a remote tangent that does not affect how we
want to proceed with this series ;-).
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help