git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] revision: avoid work after --max-count is reached

From
Junio C Hamano <gitster@pobox.com>
Date
Jul 13, 2012, 22:12 UTC
Message-ID
<7vwr2771k0.fsf@alter.siamese.dyndns.org>
In-Reply-To
<20120713212050.GB10767@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Yeah, this was my analysis, too. Though reading get_revision-1, it seems
> like we can actually set SHOWN, but I wasn't able to trigger any change
> of behavior in practice. I think it is because we must set both SHOWN
> and BOUNDARY to have an effect, and we do not do so.

In principle, SHOWN is only given when get_revision_internal gives the commit back to be shown, and the parents of the returned commit are painted CHILD_SHOWN. This should be the only place to paint commit as CHILD_SHOWN.

A handful of places set the bit to commits that would be shown if some options that further limit what is shown by topological property (e.g. --left-only, --cherry-pick), which may cause that a parent of a commit that was omitted due to these conditions may later be marked incorrectly as a boundary inside create_boundary_commit_list().

BOUNDARY is only given in create_boundary_commit_list() using CHILD_SHOWN and SHOWN, and that should happen only once when get_revision_1() runs out of the commits.

By the way, cherry_pick_list() pays attention to BOUNDARY, but I think it is written overly defensively to protect itself from future callsites. With the current code structure, it is only called from limit_list() and get_revision_*() functions are never called until limit_list() returns (and again create_boundary_commit_list() that is called from get_revision_internal() is the only place that sets BOUNDARY, so the commits cherry_pick_list() sees would never have that bit set.

Show 5 quoted lines
> So the only questionable thing would be: are there commits with BOUNDARY
> set but not SHOWN that could be affected by calling get_revision_1? For
> that matter, if such a commit existed, would the current behavior even
> be correct? We would not have actually shown the commit, so if such a
> case did exist, I wonder if we would be fixing a bug.
Previous: Jeff KingNext: Jeff King
Message 5 of 6 in “revision: avoid work after --max-count is reached”
  1. revision: avoid work after --max-count is reachedJeff King, Jul 13, 2012
  2. Jeff KingJul 13, 2012
  3. Junio C HamanoJul 13, 2012
  4. Jeff KingJul 13, 2012
  5. Junio C HamanoJul 13, 2012
  6. Jeff KingJul 14, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.