On Tue, May 13, 2008 at 10:15 PM, Stephen R. van den Berg <srb@xxxxxxx> wrote: > 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. True > - 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). True > - 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. True > - 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. Except for the case where the first parent had been already SEEN; then it would continue to test the next parents until one was found which was not already SEEN and _that_ parent would be treated as if it was first. And as Nanako showed, a simple `git rev-list HEAD^..HEAD` marks both HEAD and HEAD^ as seen. When combined with --first-parent, the result (with your patch) is that HEAD^2 is treated as the first parent. With my patch on top of yours, the walk stops as HEAD^, which is what we probably both want. > - 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. I don't think so. My code should neither follow nor mark as SEEN any parent but the first (but I could obviously be wrong). > - 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). I agree with your reasoning, and your patch with mine on top seems to achieve that goal. -- larsh -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
