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

Re: [PATCH v2] revision.c: really honor --first-parent

From
Stephen R. van den Berg <srb@cuci.nl>
Date
May 13, 2008, 20:15 UTC
Message-ID
<20080513201522.GA11485@cuci.nl>
In-Reply-To
<1210605156-22926-1-git-send-email-hjemli@gmail.com>
Lars Hjemli wrote:
>In add_parents_to_list, if any parent of a revision had already been
>SEEN, the current code would continue with the next parent, skipping
>the test for --first-parent. This patch inverts the test for SEEN so
>that the test for --first-parent is always performed.
Let's put it this way:
- If there would have been only one path to any particular point in the
  tree, then the --first-parent flag makes no differences, because the
  tree wouldn't contain any merges to begin with.
- If a tree contains *any* merges (i.e. a commit with multiple parents),
  then there are always multiple paths to some common ancestor, and
  therefore depending on which path you travel up first, you sometimes get
  clashes with the SEEN flag (unpredictable by definition).
- It would seem logical and sufficient to avoid this unpredictability by
  utilising the --first-parent flag to present and walk a tree of commits
  AS IF there were no merges.
- My original patch did just that, it simplified the code to make sure
  that all other parents beside the first parent are ignored when
  walking the tree.
- Your code now doesn't simplify the (IMO) convoluted walk, and still
  marks things as seen, even though in the first-parent case, these
  commits are not really seen at all.  It implies that your code
  generates differing output, depending on the merges present.
- The question now is, do we want the output of --first-parent to be
  immutable with respect to merges being present (but hidden from sight
  during a --first-parent run), or do we want the output of
  --first-parent to actually change depending on variations in parents
  other than the first parent?

I'd say it's better to keep the code simpler, and to make sure the output does *not* depend on any parents other than the first (as implemented in my original patch).

>This is a slightly different approach which I think is less ugly.

Your patch is smaller, and therefore (perhaps) less ugly; the resulting code and logic of my original patch is simpler (IMHO), and therefore cleaner (but it all depends on (the lack of) consensus over the points above).

-- 
Sincerely,                                                          srb@cuci.nl
           Stephen R. van den Berg.

"If I had to live my life again, I'd make the same mistakes, only sooner."
Previous: Lars HjemliNext: Lars Hjemli
Message 5 of 10 in “"git log --first-parent" shows parents that are not first”
  1. しらいしななこMay 11, 2008
  2. Junio C HamanoMay 11, 2008
  3. revision.c: really honor --first-parentLars Hjemli, May 11, 2008
  4. revision.c: really honor --first-parentLars Hjemli, May 12, 2008
  5. Stephen R. van den BergMay 13, 2008
  6. Lars HjemliMay 13, 2008
  7. Junio C HamanoMay 13, 2008
  8. Stephen R. van den BergMay 14, 2008
  9. Lars HjemliMay 14, 2008
  10. Stephen R. van den BergMay 14, 2008

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.