{"thread":{"id":"53185","subject":"[PATCH] revision: --include-diversions adds helpful merges","startedAt":"2020-04-08T01:22:10Z","lastAt":"2020-04-10T21:43:12Z","messageCount":23,"participants":["Derrick Stolee via GitGitGadget","Junio C Hamano","Derrick Stolee","brian m. carlson","Jeff King","Philip Oakley"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"395037","messageId":"pull.599.git.1586308923544.gitgitgadget@gmail.com","threadId":"53185","inReplyTo":null,"subject":"[PATCH] revision: --include-diversions adds helpful merges","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-04-08T01:22:03Z","receivedAt":"2020-04-08T01:22:10Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee <dstolee@microsoft.com>\n\nThe default file history simplification of \"git log -- <path>\" or\n\"git rev-list -- <path>\" focuses on providing the smallest set of\ncommits that first contributed a change. The revision walk greatly\nrestricts the set of walked commits by visiting only the first\nTREESAME parent of a merge commit, when one exists. This means\nthat portions of the commit-graph are not walked, which can be a\nperformance benefit, but can also \"hide\" commits that added changes\nbut were ignored by a merge resolution.\n\nThe --full-history option modifies this by walking all commits and\nreporting a merge commit as \"interesting\" if it has _any_ parent\nthat is not TREESAME. This tends to be an over-representation of\nimportant commits, especially in an environment where most merge\ncommits are created by pull request completion.\n\nSuppose we have a commit A and we create a commit B on top that\nchanges our file. When we merge the pull request, we create a merge\ncommit M. If no one else changed the file in the first-parent\nhistory between M and A, then M will not be TREESAME to its first\nparent, but will be TREESAME to B. Thus, the simplified history\nwill be \"B\". However, M will appear in the --full-history mode.\n\nHowever, suppose that a number of topics T1, T2, ..., Tn were\ncreated based on commits C1, C2, ..., Cn between A and M as\nfollows:\n\n  A----C1----C2--- ... ---Cn----M------P1---P2--- ... ---Pn\n   \\     \\     \\            \\  /      /    /            /\n    \\     \\__.. \\            \\/ ..__T1    /           Tn\n     \\           \\__..       /\\     ..__T2           /\n      \\_____________________B  \\____________________/\n\nIf the commits T1, T2, ... Tn did not change the file, then all of\nP1 through Pn will be TREESAME to their first parent, but not\nTREESAME to their second. This means that all of those merge commits\nappear in the --full-history view, with edges that immediately\ncollapse into the lower history without introducing interesting\nsingle-parent commits.\n\nThe --simplify-merges option was introduced to remove these extra\nmerge commits. By noticing that the rewritten parents are reachable\nfrom their first parents, those edges can be simplified away. Finally,\nthe commits now look like single-parent commits that are TREESAME to\ntheir \"only\" parent. Thus, they are removed and this issue does not\ncause issues anymore. However, this also ends up removing the commit\nM from the history view! Even worse, the --simplify-merges option\nrequires walking the entire history before returning a single result.\n\nMany Git users are using Git alongside a Git service that provides\ncode storage alongside a code review tool commonly called \"Pull\nRequests\" or \"Merge Requests\" against a target branch.  When these\nrequests are accepted and merged, they typically create a merge\ncommit whose first parent is the previous branch tip and the second\nparent is the tip of the topic branch used for the request. This\npresents a valuable order to the parents, but also makes that merge\ncommit slightly special. Users may want to see not only which\ncommits changed a file, but which pull requests merged those commits\ninto their branch. In the previous example, this would mean the\nusers want to see the merge commit \"M\" in addition to the single-\nparent commit \"C\".\n\nUsers are even more likely to want these merge commits when they\nuse pull requests to merge into a feature branch before merging that\nfeature branch into their trunk.\n\nIn some sense, users are asking for the \"first\" merge commit to\nbring in the change to their branch. As long as the parent order is\nconsistent, this can be handled with the following rule:\n\n  Include a merge commit if it is not TREESAME to its first\n  parent, but is TREESAME to a later parent.\n\nI call such merge commits \"diversions\" because they divert the\nhistory walk away from the first-parent history. As such, this\nchange adds the \"--include-diversions\" option to rev-list and log.\nTo test these options, extend the standard test example to include\na merge commit that is not TREESAME to its first parent. It is\nsurprising that that option was not already in the example, as it\nis instructive.\n\nIn particular, this extension demonstrates a common issue with file\nhistory simplification. When a user resolves a merge conflict using\n\"-Xours\" or otherwise ignoring one side of the conflict, they create\na TREESAME edge that probably should not be TREESAME. This leads\nusers to become frustrated and complain that \"my change disappeared!\"\nIn my experience, showing them history with --full-history and\n--simplify-merges quickly reveals the problematic merge. As mentioned,\nthis option is expensive to compute. The --include-diversions option\n_might_ show the merge commit (usually titled \"resolving conflicts\")\nmore quickly. Of course, this depends on the user having the correct\nparent order, which is backwards when using \"git pull\".\n\nSigned-off-by: Derrick Stolee <dstolee@microsoft.com>\n---\n    Add a new history mode\n    \n    This --include-diversions option could use a better name.\n    \n    An experienced developer in the Windows OS Engineering Systems team\n    pointed out how hard it is to find out when a change was \"introduced\" in\n    the Windows OS repo. Due to their multi-leveled, long-lived branch\n    organization, a commit could be part of hundreds of pull requests as the\n    branches are merged across the organization.\n    \n    My default answer was \"this is complicated not because of Git, but\n    because of how you are branching.\" I then tried to explain how finding\n    the \"first merge\" to include a commit is incredibly difficult and\n    requires performing multiple reachability queries. As I was working it\n    out on paper, I realized that was true if we relied only on the\n    commit-graph shape to inform our qurey.\n    \n    If we use the TREESAME information, then suddenly we get a much clearer\n    picture! Let's simply pick out those merge commits that \"introduced a\n    change\" because they are TREESAME to a non-first-parent (and not\n    TREESAME to the first parent).\n    \n    My name of \"diversions\" could probably use some work, but I like the\n    basic concept of this option.\n    \n    I welcome any and all feedback. Thanks!\n    \n    -Stolee\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-599%2Fderrickstolee%2Fnew-history-mode-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-599/derrickstolee/new-history-mode-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/599\n\n Documentation/rev-list-options.txt |  6 ++++\n revision.c                         | 16 ++++++++++-\n revision.h                         |  1 +\n t/t6012-rev-list-simplify.sh       | 44 ++++++++++++++++++++++++++++++\n 4 files changed, 66 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex bfd02ade991..0c878be94a9 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -342,6 +342,12 @@ Default mode::\n \tbranches if the end result is the same (i.e. merging branches\n \twith the same content)\n \n+--include-diversions::\n+\tInclude all commits from the default mode, but also any merge\n+\tcommits that are not TREESAME to the first parent but are\n+\tTREESAME to a later parent. This mode is helpful for showing\n+\tthe merge commits that \"first introduced\" a change to a branch.\n+\n --full-history::\n \tSame as the default mode, but does not prune some history.\n \ndiff --git a/revision.c b/revision.c\nindex 8136929e236..915d8febdc4 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -870,7 +870,19 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)\n \t\t\t}\n \t\t\tparent->next = NULL;\n \t\t\tcommit->parents = parent;\n-\t\t\tcommit->object.flags |= TREESAME;\n+\n+\t\t\t/*\n+\t\t\t * A merge commit is a \"diversion\" if it is not\n+\t\t\t * TREESAME to its first parent but is TREESAME\n+\t\t\t * to a later parent. In the simplified history,\n+\t\t\t * we \"divert\" the history walk to the later\n+\t\t\t * parent. These commits are shown when \"diversions\"\n+\t\t\t * is enabled, so do not mark the object as\n+\t\t\t * TREESAME here.\n+\t\t\t */\n+\t\t\tif (!revs->diversions || !nth_parent)\n+\t\t\t\tcommit->object.flags |= TREESAME;\n+\n \t\t\treturn;\n \n \t\tcase REV_TREE_NEW:\n@@ -2265,6 +2277,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t} else if (!strcmp(arg, \"--full-diff\")) {\n \t\trevs->diff = 1;\n \t\trevs->full_diff = 1;\n+\t} else if (!strcmp(arg, \"--include-diversions\")) {\n+\t\trevs->diversions = 1;\n \t} else if (!strcmp(arg, \"--full-history\")) {\n \t\trevs->simplify_history = 0;\n \t} else if (!strcmp(arg, \"--relative-date\")) {\ndiff --git a/revision.h b/revision.h\nindex 475f048fb61..f06a73cbcd8 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -129,6 +129,7 @@ struct rev_info {\n \t\t\tno_walk:2,\n \t\t\tremove_empty_trees:1,\n \t\t\tsimplify_history:1,\n+\t\t\tdiversions:1,\n \t\t\ttopo_order:1,\n \t\t\tsimplify_merges:1,\n \t\t\tsimplify_by_decoration:1,\ndiff --git a/t/t6012-rev-list-simplify.sh b/t/t6012-rev-list-simplify.sh\nindex a10f0df02b0..9c91226f737 100755\n--- a/t/t6012-rev-list-simplify.sh\n+++ b/t/t6012-rev-list-simplify.sh\n@@ -154,4 +154,48 @@ test_expect_success '--full-diff is not affected by --parents' '\n \ttest_cmp expected actual\n '\n \n+#\n+# Modify the test repo to add a merge whose first parent is not TREESAME\n+# but whose second parent is TREESAME\n+#\n+# A--B----------G--H--I--K--L--N\n+#  \\  \\           /     /     /\n+#   \\  \\         /     /     /\n+#    C------E---F     J     /\n+#     \\  \\_/               /\n+#      \\                  /\n+#       M-----------------\n+test_expect_success 'expand graph' '\n+\tgit switch -c branchM C &&\n+\techo \"new data\" >file &&\n+\tgit add file &&\n+\ttest_tick &&\n+\ttest_commit M &&\n+\n+\tgit checkout master &&\n+\tgit merge -Xtheirs branchM -m \"N\" &&\n+\tnote N\n+'\n+\n+check_result 'M C A' -- file\n+check_result 'N M C A' --include-diversions -- file\n+\n+check_result 'N M L K J I H F E D C G B A' --full-history --topo-order\n+check_result 'N M L K I H G F E D C B J A' --full-history\n+check_result 'N M L K I H G F E D C B J A' --full-history --date-order\n+check_result 'N M L K I H G F E D B C J A' --full-history --author-date-order\n+check_result 'N M K I H E C B A' --full-history -- file\n+check_result 'N M K I H E C B A' --full-history --topo-order -- file\n+check_result 'N M K I H E C B A' --full-history --date-order -- file\n+check_result 'N M K I H E B C A' --full-history --author-date-order -- file\n+check_result 'N M I E C B A' --simplify-merges -- file\n+check_result 'N M I E C B A' --simplify-merges --topo-order -- file\n+check_result 'N M I E C B A' --simplify-merges --date-order -- file\n+check_result 'N M I E B C A' --simplify-merges --author-date-order -- file\n+check_result 'M C A' --topo-order -- file\n+check_result 'M C A' --date-order -- file\n+check_result 'M C A' --author-date-order -- file\n+check_result 'H' --first-parent -- another-file\n+check_result 'H' --first-parent --topo-order -- another-file\n+\n test_done\n\nbase-commit: 274b9cc25322d9ee79aa8e6d4e86f0ffe5ced925\n-- \ngitgitgadget\n"},{"id":"395039","messageId":"xmqqv9mapxw9.fsf@gitster.c.googlers.com","threadId":"53185","inReplyTo":"pull.599.git.1586308923544.gitgitgadget@gmail.com","subject":"Re: [PATCH] revision: --include-diversions adds helpful merges","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-08T01:30:14Z","receivedAt":"2020-04-08T01:30:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Derrick Stolee via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n>     This --include-diversions option could use a better name.\n\nTrue, but I do not think of a better (or for that matter a worse)\none.  \n\nAs a new feature, I think this is a reasonable thing to want,\nespecially it is in line with the push in the past few years to\ntreat the first parent history specially.\n\nI wonder how this would interact with the ancestry-path option?\nThat one also, like the simplify-merges option, needs a limited\ntraversal, and if this new mode can do without a limited traversal\n(in other words, the output can be done incrementally from the tip)\nand achieve something similar to what these other options wanted to\nshow, that would be great.\n\nThanks.\n\n"},{"id":"395040","messageId":"5f1c6868-303c-ccc1-553e-e54d0fa8e24f@gmail.com","threadId":"53185","inReplyTo":"xmqqv9mapxw9.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] revision: --include-diversions adds helpful merges","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-04-08T01:39:50Z","receivedAt":"2020-04-08T01:39:54Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 4/7/2020 9:30 PM, Junio C Hamano wrote:\n> \"Derrick Stolee via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n> \n>>     This --include-diversions option could use a better name.\n> \n> True, but I do not think of a better (or for that matter a worse)\n> one.  \n> \n> As a new feature, I think this is a reasonable thing to want,\n> especially it is in line with the push in the past few years to\n> treat the first parent history specially.\n> \n> I wonder how this would interact with the ancestry-path option?\n> That one also, like the simplify-merges option, needs a limited\n> traversal, and if this new mode can do without a limited traversal\n> (in other words, the output can be done incrementally from the tip)\n> and achieve something similar to what these other options wanted to\n> show, that would be great.\n\nYou're right. I briefly considered the --ancestry-path option before\nrealizing that would get a huge set of commits (for example: every\ntopic based on the branch after the pull request was merged).\n\nThe --include-diversions works incrementally like simplified merges.\nBased on the implementation, it would not change the results when\nadded to a --full-history query. This makes sense: a diversion would\nappear in the --full-history results, anyway.\n\nIt is worth adding tests for the combination with --ancestry-path\nand --simplify-merges, as the --include-diversions option would\nadd results to those queries.\n\nThanks,\n-Stolee\n\n"},{"id":"395041","messageId":"20200408021330.GB6549@camp.crustytoothpaste.net","threadId":"53185","inReplyTo":"pull.599.git.1586308923544.gitgitgadget@gmail.com","subject":"Re: [PATCH] revision: --include-diversions adds helpful merges","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-04-08T02:13:30Z","receivedAt":"2020-04-08T02:13:40Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-04-08 at 01:22:03, Derrick Stolee via GitGitGadget wrote:\n> The --simplify-merges option was introduced to remove these extra\n> merge commits. By noticing that the rewritten parents are reachable\n> from their first parents, those edges can be simplified away. Finally,\n> the commits now look like single-parent commits that are TREESAME to\n> their \"only\" parent. Thus, they are removed and this issue does not\n> cause issues anymore. However, this also ends up removing the commit\n> M from the history view! Even worse, the --simplify-merges option\n> requires walking the entire history before returning a single result.\n\nI was not aware --simplify-merges required walking the entire history.\nNow I know why my alias using it performs so poorly on $HUGE_REPOSITORY\nwith thousands of extra backmerges at $DAYJOB.\n\nThanks for teaching me something.\n\n> Many Git users are using Git alongside a Git service that provides\n> code storage alongside a code review tool commonly called \"Pull\n> Requests\" or \"Merge Requests\" against a target branch.  When these\n> requests are accepted and merged, they typically create a merge\n> commit whose first parent is the previous branch tip and the second\n> parent is the tip of the topic branch used for the request. This\n> presents a valuable order to the parents, but also makes that merge\n> commit slightly special. Users may want to see not only which\n> commits changed a file, but which pull requests merged those commits\n> into their branch. In the previous example, this would mean the\n> users want to see the merge commit \"M\" in addition to the single-\n> parent commit \"C\".\n\nI should not hesitate to point out that this history is also true of the\nGit Project's repository, although of course the merges may be of less\ninterest here.\n\n> In some sense, users are asking for the \"first\" merge commit to\n> bring in the change to their branch. As long as the parent order is\n> consistent, this can be handled with the following rule:\n> \n>   Include a merge commit if it is not TREESAME to its first\n>   parent, but is TREESAME to a later parent.\n> \n> I call such merge commits \"diversions\" because they divert the\n> history walk away from the first-parent history. As such, this\n> change adds the \"--include-diversions\" option to rev-list and log.\n> To test these options, extend the standard test example to include\n> a merge commit that is not TREESAME to its first parent. It is\n> surprising that that option was not already in the example, as it\n> is instructive.\n> \n> In particular, this extension demonstrates a common issue with file\n> history simplification. When a user resolves a merge conflict using\n> \"-Xours\" or otherwise ignoring one side of the conflict, they create\n> a TREESAME edge that probably should not be TREESAME. This leads\n> users to become frustrated and complain that \"my change disappeared!\"\n> In my experience, showing them history with --full-history and\n> --simplify-merges quickly reveals the problematic merge. As mentioned,\n> this option is expensive to compute. The --include-diversions option\n> _might_ show the merge commit (usually titled \"resolving conflicts\")\n> more quickly. Of course, this depends on the user having the correct\n> parent order, which is backwards when using \"git pull\".\n\nI can't comment on the contents of the patch, since I'm really not at\nall familiar with the revision machinery, but I do think this change is a\ngood idea.  I see this as a very common use case, and I think this\ncommit message explains the rationale well.\n\n> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>\n> ---\n>     Add a new history mode\n> \n>     This --include-diversions option could use a better name.\n\nAs we all know, I'm terrible at naming things, so I have no suggestions\nhere.  I'm happy with the name as it stands, but am of course open to\nother ideas.\n\n> diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\n> index bfd02ade991..0c878be94a9 100644\n> --- a/Documentation/rev-list-options.txt\n> +++ b/Documentation/rev-list-options.txt\n> @@ -342,6 +342,12 @@ Default mode::\n>  \tbranches if the end result is the same (i.e. merging branches\n>  \twith the same content)\n> \n> +--include-diversions::\n> +\tInclude all commits from the default mode, but also any merge\n> +\tcommits that are not TREESAME to the first parent but are\n> +\tTREESAME to a later parent. This mode is helpful for showing\n> +\tthe merge commits that \"first introduced\" a change to a branch.\n\nI wasn't sure if this use of \"TREESAME\" would be confusing, but it looks\nlike we already use it extensively throughout the documentation, so it's\nprobably fine.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"395068","messageId":"74562b04-b1ce-cad4-da18-4af030a3dc29@gmail.com","threadId":"53185","inReplyTo":"5f1c6868-303c-ccc1-553e-e54d0fa8e24f@gmail.com","subject":"Re: [PATCH] revision: --include-diversions adds helpful merges","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-04-08T15:28:53Z","receivedAt":"2020-04-08T15:29:00Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 4/7/2020 9:39 PM, Derrick Stolee wrote:\n> On 4/7/2020 9:30 PM, Junio C Hamano wrote:\n>> \"Derrick Stolee via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n>>\n>>>     This --include-diversions option could use a better name.\n>>\n>> True, but I do not think of a better (or for that matter a worse)\n>> one.  \n\nHere are some alternative names:\n\n\t--audit-merges\n\t--audit-trunk\n\t--first-merges\n\t--subtle-merges\n\t--more-merges\n\nThe \"audit\" name implies some of the intent: we are trying to\naudit which merge commits introduced these changes. The --audit-trunk\nimplies we are using a trunk-based workflow where parent order is\ncritical. However, \"trunk\" may be confusing when there are multiple\nlong-lived branches.\n\nA \"first merge\" is confusing when we see a sequence of multiple\ndiversion merges (I include a test with this exact situation in\nmy next version.)\n\n\"subtle\" is a bit wishy-washy.\n\n\"--more-merges\" is not very specific. The way we are adding\nmerges to the final result may not be the only way we want to\nadd \"more\" merges in the future.\n\nSo, I think \"--audit-merges\" is the best of these alternatives.\nI'd be happy to be overruled with a different option. Hopefully,\nthese options inspire better ideas from the community.\n\n>> As a new feature, I think this is a reasonable thing to want,\n>> especially it is in line with the push in the past few years to\n>> treat the first parent history specially.\n>>\n>> I wonder how this would interact with the ancestry-path option?\n>> That one also, like the simplify-merges option, needs a limited\n>> traversal, and if this new mode can do without a limited traversal\n>> (in other words, the output can be done incrementally from the tip)\n>> and achieve something similar to what these other options wanted to\n>> show, that would be great.\n> \n> You're right. I briefly considered the --ancestry-path option before\n> realizing that would get a huge set of commits (for example: every\n> topic based on the branch after the pull request was merged).\n> \n> The --include-diversions works incrementally like simplified merges.\n> Based on the implementation, it would not change the results when\n> added to a --full-history query. This makes sense: a diversion would\n> appear in the --full-history results, anyway.\n> \n> It is worth adding tests for the combination with --ancestry-path\n> and --simplify-merges, as the --include-diversions option would\n> add results to those queries.\n\nMy gitgitgadget PR [1] is updated with tests and some new logic to\nhandle the new option along with --simplify-merges. The situation was\na bit subtle, so my next version will include a significant update to\nthe rev-list documentation under the \"History Simplification\" mode.\n\nI'll give things some time to calm on the name of the option before\nsending a v2.\n\nMy v2 also includes adding a new object flag \"DIVERSION\" to track\nthese commits from the TREESAME calculation through the simplify-merges\nlogic. When I was adding a new flag, I realized that I already\nmessed up the 32-bit alignment of \"struct object\" when adding the\nTOPO_ORDER flags. The parsed, type, and flags bitfields add up to\n33 bits!\n\nA solution would include pulling the TOPO_ORDER_* flags to be bits\n22 and 23 instead of 26 and 27, but that would collide with what is\nhappening in builtin/show-branch.c. But then I saw the following\ncomment in builtin/show-branch.c:\n\n/*\n * TODO: convert this use of commit->object.flags to commit-slab\n * instead to store a pointer to ref name directly. Then use the same\n * UNINTERESTING definition from revision.h here.\n */\n\nIs anyone interested in tackling this problem? I don't see any\ntest failures when I swap the TOPO_ORDER_ flag locations, but\nthat might just mean that show-branch isn't tested enough.\n\nThanks,\n-Stolee\n\n[1] https://github.com/gitgitgadget/git/pull/599\n"},{"id":"395081","messageId":"20200408184843.GA1963695@coredump.intra.peff.net","threadId":"53185","inReplyTo":"pull.599.git.1586308923544.gitgitgadget@gmail.com","subject":"Re: [PATCH] revision: --include-diversions adds helpful merges","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-04-08T18:48:43Z","receivedAt":"2020-04-08T18:48:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Apr 08, 2020 at 01:22:03AM +0000, Derrick Stolee via GitGitGadget wrote:\n\n> The default file history simplification of \"git log -- <path>\" or\n> \"git rev-list -- <path>\" focuses on providing the smallest set of\n> commits that first contributed a change. The revision walk greatly\n> restricts the set of walked commits by visiting only the first\n> TREESAME parent of a merge commit, when one exists. This means\n> that portions of the commit-graph are not walked, which can be a\n> performance benefit, but can also \"hide\" commits that added changes\n> but were ignored by a merge resolution.\n> [...]\n\nThanks for a really great description of the problem.\n\nPlaying around with the patch, I found one curiosity. Try this:\n\n  git log --graph --oneline origin -- GIT-VERSION-GEN >old\n  git log --graph --oneline --include-diversions \\\n                            origin -- GIT-VERSION-GEN >new\n  diff -u old new\n\nThe first hunk has:\n\n  @@ -70,6 +70,7 @@\n   * 20769079d2 Git 2.12-rc2\n   * 5588dbffbd Git 2.12-rc1\n   * 6e3a7b3398 Git 2.12-rc0\n  +* 0a45050a14 Merge branch 'rj/git-version-gen-do-not-force-abbrev'\n   * a7659747c2 GIT-VERSION-GEN: do not force abbreviation length used by 'describe'\n   * 8d7a455ed5 Start post 2.11 cycle\n   * 454cb6bd52 Git 2.11\n\nwhich makes sense. That merge brought in a7659747c2, and the other side\nhadn't touched it. But I can't tell from the output how the two are\nrelated. Nor can I just add \"-p\" to the invocation; after we've\nsimplified, it has only a single parent, but it's TREESAME to that\nparent. So it has no diff.\n\nI actually think the most descriptive output here would be something\nlike:\n\n  * 6e3a7b3398 Git 2.12-rc0\n  * 0a45050a14 Merge branch 'rj/git-version-gen-do-not-force-abbrev'\n  |\\\n  | * a7659747c2 GIT-VERSION-GEN: do not force abbreviation length used by 'describe'\n  |/\n  * Start post 2.11 cycle\n\nI.e., leaving both parents intact for a \"diversion\" merge. But maybe\nthat would have secondary effects in other places.\n\n-Peff\n"},{"id":"395093","messageId":"xmqqzhbloj4u.fsf@gitster.c.googlers.com","threadId":"53185","inReplyTo":"74562b04-b1ce-cad4-da18-4af030a3dc29@gmail.com","subject":"Re: [PATCH] revision: --include-diversions adds helpful merges","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-08T19:46:41Z","receivedAt":"2020-04-08T19:46:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee <stolee@gmail.com> writes:\n\n> On 4/7/2020 9:39 PM, Derrick Stolee wrote:\n>> On 4/7/2020 9:30 PM, Junio C Hamano wrote:\n>>> \"Derrick Stolee via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n>>>\n>>>>     This --include-diversions option could use a better name.\n>>>\n>>> True, but I do not think of a better (or for that matter a worse)\n>>> one.  \n>\n> Here are some alternative names:\n>\n> \t--audit-merges\n> \t--audit-trunk\n> \t--first-merges\n> \t--subtle-merges\n> \t--more-merges\n>\n> The \"audit\" name implies some of the intent: we are trying to\n> audit which merge commits introduced these changes. The --audit-trunk\n> implies we are using a trunk-based workflow where parent order is\n> critical. However, \"trunk\" may be confusing when there are multiple\n> long-lived branches.\n\nOur features written with the intent to be useful for one purpose\noften end up being used for purposes other than what the feature was\noriginally written for (the \"--pickaxe\" has always been a bitter\nexample of this for me).\n\nFor that reason, I am a bit hesitant to endorse \"audit\" exactly\nbecause of the implication of \"intent\".\n\nWe usually try to give a single simplest history that explains how\nthe current content in the path came to be.  For that, commit M in\nthe illustration in your original message does not really help when\nwe ignore which parent chain owns the history, but in practice, many\nworkflows treat the first parent chain as something special, so we\nwant to show, not just the individual changes that matter, where the\nchanges are introduced in the first-parent chain.\n\nI wonder if there is a simple-enough phrase to convey what the\nlatter half of above sentence says.  \"include\" and \"keep\" are both\ngood verbs---normally we discard these merges, because they do not\ncontribute at the level of individual changes, but with the option,\nwe \"include\" or \"keep\" these merges in the output.  It's not like\nwe keep _all_ the merges, but selected merges only.  How do we\ndecide which merges to keep?\n\nI guess your \"--first-merges\" came from such a line of thought, and\nis the closest among the five to what I have in mind, but it drops\ntoo many words and loses too much meaning.  \n\n\"--keep-first-parent-merges\", perhaps?\n\n\n\n\n"},{"id":"395094","messageId":"20200408200526.GA2270445@coredump.intra.peff.net","threadId":"53185","inReplyTo":"xmqqzhbloj4u.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] revision: --include-diversions adds helpful merges","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-04-08T20:05:26Z","receivedAt":"2020-04-08T20:05:31Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Apr 08, 2020 at 12:46:41PM -0700, Junio C Hamano wrote:\n\n> Our features written with the intent to be useful for one purpose\n> often end up being used for purposes other than what the feature was\n> originally written for (the \"--pickaxe\" has always been a bitter\n> example of this for me).\n> \n> For that reason, I am a bit hesitant to endorse \"audit\" exactly\n> because of the implication of \"intent\".\n\nYeah, I agree with this.\n\n> I wonder if there is a simple-enough phrase to convey what the\n> latter half of above sentence says.  \"include\" and \"keep\" are both\n> good verbs---normally we discard these merges, because they do not\n> contribute at the level of individual changes, but with the option,\n> we \"include\" or \"keep\" these merges in the output.  It's not like\n> we keep _all_ the merges, but selected merges only.  How do we\n> decide which merges to keep?\n> \n> I guess your \"--first-merges\" came from such a line of thought, and\n> is the closest among the five to what I have in mind, but it drops\n> too many words and loses too much meaning.  \n> \n> \"--keep-first-parent-merges\", perhaps?\n\nFWIW, this name left me more confused, because \"first-parent merges\"\nisn't an already-defined term I knew. And it seems like all merges have\na first parent. Having read the patch description, I guess it's \"a merge\nwhich isn't TREESAME to its first-parent\".\n\nI can't think of a more succinct way to name that, though. And possibly\nif we gave that definition in the documentation, that would be enough.\nThe name doesn't have to be a complete description; it only has to make\nsense once you know what you're trying to do (and be memorable enough).\n\n-Peff\n"},{"id":"395098","messageId":"9eab6c60-3c99-d6ac-9206-02ce37f7a479@gmail.com","threadId":"53185","inReplyTo":"20200408200526.GA2270445@coredump.intra.peff.net","subject":"Re: [PATCH] revision: --include-diversions adds helpful merges","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-04-08T20:22:21Z","receivedAt":"2020-04-08T20:22:27Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 4/8/2020 4:05 PM, Jeff King wrote:\n> On Wed, Apr 08, 2020 at 12:46:41PM -0700, Junio C Hamano wrote:\n> \n>> Our features written with the intent to be useful for one purpose\n>> often end up being used for purposes other than what the feature was\n>> originally written for (the \"--pickaxe\" has always been a bitter\n>> example of this for me).\n>>\n>> For that reason, I am a bit hesitant to endorse \"audit\" exactly\n>> because of the implication of \"intent\".\n> \n> Yeah, I agree with this.\n> \n>> I wonder if there is a simple-enough phrase to convey what the\n>> latter half of above sentence says.  \"include\" and \"keep\" are both\n>> good verbs---normally we discard these merges, because they do not\n>> contribute at the level of individual changes, but with the option,\n>> we \"include\" or \"keep\" these merges in the output.  It's not like\n>> we keep _all_ the merges, but selected merges only.  How do we\n>> decide which merges to keep?\n>>\n>> I guess your \"--first-merges\" came from such a line of thought, and\n>> is the closest among the five to what I have in mind, but it drops\n>> too many words and loses too much meaning.  \n>>\n>> \"--keep-first-parent-merges\", perhaps?\n> \n> FWIW, this name left me more confused, because \"first-parent merges\"\n> isn't an already-defined term I knew. And it seems like all merges have\n> a first parent. Having read the patch description, I guess it's \"a merge\n> which isn't TREESAME to its first-parent\".\n> \n> I can't think of a more succinct way to name that, though. And possibly\n> if we gave that definition in the documentation, that would be enough.\n> The name doesn't have to be a complete description; it only has to make\n> sense once you know what you're trying to do (and be memorable enough).\n\nThen I suppose we should focus on naming merge commits with this property:\n\n  A merge commit that is not TREESAME to its first parent (but is TREESAME\n  to a later parent).\n\nThe part in parentheses may be optional, because a merge commit that is\nnot TREESAME to any parent will be included by every history mode.\n\nIn my latest attempt at documentation, I called these merges \"diverters\"\nyet still used \"--include-diversions\". Here are a few other words that we\ncould use:\n\n * diverters or diversions\n * redirects\n * switches (think railroad switch). Synonym: exchange\n * detours\n\nThe \"switch\" or \"exchange\" words are probably bad because they have\nnoun _and_ verb forms.\n\nOr we could look again at the history results as a whole to find\ninspiration for the command-line option:\n\n * --merge-trail\n\nThanks,\n-Stolee\n"},{"id":"395099","messageId":"xmqqv9m9oe2p.fsf@gitster.c.googlers.com","threadId":"53185","inReplyTo":"9eab6c60-3c99-d6ac-9206-02ce37f7a479@gmail.com","subject":"Re: [PATCH] revision: --include-diversions adds helpful merges","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-08T21:35:58Z","receivedAt":"2020-04-08T21:36:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee <stolee@gmail.com> writes:\n\n> Then I suppose we should focus on naming merge commits with this property:\n>\n>   A merge commit that is not TREESAME to its first parent (but is TREESAME\n>   to a later parent).\n>\n> The part in parentheses may be optional, because a merge commit that is\n> not TREESAME to any parent will be included by every history mode.\n\nA merge that is TREESAME to its first parent does not introduce\nanything new to the mainline (as far as the paths that match the\npathspec are concerned).  We are trying to find names to call merges\nthat are not those no-op merges.  Hmph...\n\n> In my latest attempt at documentation, I called these merges \"diverters\"\n> yet still used \"--include-diversions\". Here are a few other words that we\n> could use:\n>\n>  * diverters or diversions\n>  * redirects\n>  * switches (think railroad switch). Synonym: exchange\n>  * detours\n\n...none of the above tells me that they are not no-op (in other\nwords, they do something meaningful), so I must be coming from\na direction different from you are.  What redirects from what other\nthing, for example?\n\n\n"},{"id":"395106","messageId":"91fafa17-c58f-dcd0-320d-48c4512561f6@gmail.com","threadId":"53185","inReplyTo":"xmqqv9m9oe2p.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] revision: --include-diversions adds helpful merges","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-04-08T23:59:16Z","receivedAt":"2020-04-08T23:59:22Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 4/8/2020 5:35 PM, Junio C Hamano wrote:\n> Derrick Stolee <stolee@gmail.com> writes:\n> \n>> Then I suppose we should focus on naming merge commits with this property:\n>>\n>>   A merge commit that is not TREESAME to its first parent (but is TREESAME\n>>   to a later parent).\n>>\n>> The part in parentheses may be optional, because a merge commit that is\n>> not TREESAME to any parent will be included by every history mode.\n> \n> A merge that is TREESAME to its first parent does not introduce\n> anything new to the mainline (as far as the paths that match the\n> pathspec are concerned).  We are trying to find names to call merges\n> that are not those no-op merges.  Hmph...\n\nThere are three situations for a merge commit:\n\n1. TREESAME to _all_ parents. These are not included.\n2. not TREESAME to _all_ parents. These are already included.\n3. TREESAME to some, but not TREESAME to others.\n\nThe third mode is the one that default mode will drop, but --full-history\nwill include. The new mode will include some of these (the ones that are\nNOT TREESAME to their first parent).\n\n>> In my latest attempt at documentation, I called these merges \"diverters\"\n>> yet still used \"--include-diversions\". Here are a few other words that we\n>> could use:\n>>\n>>  * diverters or diversions\n>>  * redirects\n>>  * switches (think railroad switch). Synonym: exchange\n>>  * detours\n> \n> ...none of the above tells me that they are not no-op (in other\n> words, they do something meaningful), so I must be coming from\n> a direction different from you are.  What redirects from what other\n> thing, for example?\n\nThe merges do something meaningful: they \"merge in\" a \"real\" change.\n\nI'll just submit my v2 as-is, which includes a significant change to\nthe documentation that should make things more clear.\n\nThanks,\n-Stolee\n"},{"id":"395107","messageId":"pull.599.v2.git.1586390501128.gitgitgadget@gmail.com","threadId":"53185","inReplyTo":"pull.599.git.1586308923544.gitgitgadget@gmail.com","subject":"[PATCH v2] revision: --include-diversions adds helpful merges","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-04-09T00:01:40Z","receivedAt":"2020-04-09T00:01:52Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee <dstolee@microsoft.com>\n\nThe default file history simplification of \"git log -- <path>\" or\n\"git rev-list -- <path>\" focuses on providing the smallest set of\ncommits that first contributed a change. The revision walk greatly\nrestricts the set of walked commits by visiting only the first\nTREESAME parent of a merge commit, when one exists. This means\nthat portions of the commit-graph are not walked, which can be a\nperformance benefit, but can also \"hide\" commits that added changes\nbut were ignored by a merge resolution.\n\nThe --full-history option modifies this by walking all commits and\nreporting a merge commit as \"interesting\" if it has _any_ parent\nthat is not TREESAME. This tends to be an over-representation of\nimportant commits, especially in an environment where most merge\ncommits are created by pull request completion.\n\nSuppose we have a commit A and we create a commit B on top that\nchanges our file. When we merge the pull request, we create a merge\ncommit M. If no one else changed the file in the first-parent\nhistory between M and A, then M will not be TREESAME to its first\nparent, but will be TREESAME to B. Thus, the simplified history\nwill be \"B\". However, M will appear in the --full-history mode.\n\nHowever, suppose that a number of topics T1, T2, ..., Tn were\ncreated based on commits C1, C2, ..., Cn between A and M as\nfollows:\n\n  A----C1----C2--- ... ---Cn----M------P1---P2--- ... ---Pn\n   \\     \\     \\            \\  /      /    /            /\n    \\     \\__.. \\            \\/ ..__T1    /           Tn\n     \\           \\__..       /\\     ..__T2           /\n      \\_____________________B  \\____________________/\n\nIf the commits T1, T2, ... Tn did not change the file, then all of\nP1 through Pn will be TREESAME to their first parent, but not\nTREESAME to their second. This means that all of those merge commits\nappear in the --full-history view, with edges that immediately\ncollapse into the lower history without introducing interesting\nsingle-parent commits.\n\nThe --simplify-merges option was introduced to remove these extra\nmerge commits. By noticing that the rewritten parents are reachable\nfrom their first parents, those edges can be simplified away. Finally,\nthe commits now look like single-parent commits that are TREESAME to\ntheir \"only\" parent. Thus, they are removed and this issue does not\ncause issues anymore. However, this also ends up removing the commit\nM from the history view! Even worse, the --simplify-merges option\nrequires walking the entire history before returning a single result.\n\nMany Git users are using Git alongside a Git service that provides\ncode storage alongside a code review tool commonly called \"Pull\nRequests\" or \"Merge Requests\" against a target branch.  When these\nrequests are accepted and merged, they typically create a merge\ncommit whose first parent is the previous branch tip and the second\nparent is the tip of the topic branch used for the request. This\npresents a valuable order to the parents, but also makes that merge\ncommit slightly special. Users may want to see not only which\ncommits changed a file, but which pull requests merged those commits\ninto their branch. In the previous example, this would mean the\nusers want to see the merge commit \"M\" in addition to the single-\nparent commit \"C\".\n\nUsers are even more likely to want these merge commits when they\nuse pull requests to merge into a feature branch before merging that\nfeature branch into their trunk.\n\nIn some sense, users are asking for the \"first\" merge commit to\nbring in the change to their branch. As long as the parent order is\nconsistent, this can be handled with the following rule:\n\n  Include a merge commit if it is not TREESAME to its first\n  parent, but is TREESAME to a later parent.\n\nI call such merge commits \"diversions\" because they divert the\nhistory walk away from the first-parent history. As such, this\nchange adds the \"--include-diversions\" option to rev-list and log.\nTo test these options, extend the standard test example to include\na merge commit that is not TREESAME to its first parent. It is\nsurprising that that option was not already in the example, as it\nis instructive.\n\nIn particular, this extension demonstrates a common issue with file\nhistory simplification. When a user resolves a merge conflict using\n\"-Xours\" or otherwise ignoring one side of the conflict, they create\na TREESAME edge that probably should not be TREESAME. This leads\nusers to become frustrated and complain that \"my change disappeared!\"\nIn my experience, showing them history with --full-history and\n--simplify-merges quickly reveals the problematic merge. As mentioned,\nthis option is expensive to compute. The --include-diversions option\n_might_ show the merge commit (usually titled \"resolving conflicts\")\nmore quickly. Of course, this depends on the user having the correct\nparent order, which is backwards when using \"git pull\".\n\nThere are some special considerations when combining the\n--include-diversions option with --simplify-merges. This requires\nadding a new DIVERSION object flag to store the information from\nthe initial TREESAME comparisons. This helps avoid dropping those\ncommits in later filters. This is covered by a test, including how\nthe parents can be simplified. Since \"struct object\" has already\nruined its 32-bit alignment by using 33 bits across parsed, type,\nand flags member, let's not make it worse. DIVERSION is used in\nrevision.c with the same value (1u<<15) as REACHABLE in\ncommit-graph.c. The REACHABLE flag is only used when writing a\ncommit-graph file, and a revision walk using --include-diversions\ndoes not happen in the same process. Care must be taken in the\nfuture to ensure this remains the case.\n\nUpdate Documentation/rev-list-options.txt with significant details\naround this option. This requires updating the example in the\nHistory Simplification section to demonstrate some of the problems\nwith TREESAME second parents.\n\nSigned-off-by: Derrick Stolee <dstolee@microsoft.com>\n---\n    Add a new history mode\n    \n    This --include-diversions option could use a better name.\n    \n    An experienced developer in the Windows OS Engineering Systems team\n    pointed out how hard it is to find out when a change was \"introduced\" in\n    the Windows OS repo. Due to their multi-leveled, long-lived branch\n    organization, a commit could be part of hundreds of pull requests as the\n    branches are merged across the organization.\n    \n    My default answer was \"this is complicated not because of Git, but\n    because of how you are branching.\" I then tried to explain how finding\n    the \"first merge\" to include a commit is incredibly difficult and\n    requires performing multiple reachability queries. As I was working it\n    out on paper, I realized that was true if we relied only on the\n    commit-graph shape to inform our qurey.\n    \n    If we use the TREESAME information, then suddenly we get a much clearer\n    picture! Let's simply pick out those merge commits that \"introduced a\n    change\" because they are TREESAME to a non-first-parent (and not\n    TREESAME to the first parent).\n    \n    My name of \"diversions\" could probably use some work, but I like the\n    basic concept of this option.\n    \n    I welcome any and all feedback. Thanks!\n    \n    UPDATES in v2:\n    \n     * The functionality is a bit more complicated to work with\n       --simplify-merges.\n       \n       \n     * The documentation is significantly expanded with an example that\n       highlights the shortcomings of the default history simplification.\n       \n       \n     * The documented example is used in the test script instead of simply\n       extending the previous example.\n       \n       \n    \n    -Stolee\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-599%2Fderrickstolee%2Fnew-history-mode-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-599/derrickstolee/new-history-mode-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/599\n\nRange-diff vs v1:\n\n 1:  8bf93688392 ! 1:  146c443e7f1 revision: --include-diversions adds helpful merges\n     @@ Commit message\n          more quickly. Of course, this depends on the user having the correct\n          parent order, which is backwards when using \"git pull\".\n      \n     +    There are some special considerations when combining the\n     +    --include-diversions option with --simplify-merges. This requires\n     +    adding a new DIVERSION object flag to store the information from\n     +    the initial TREESAME comparisons. This helps avoid dropping those\n     +    commits in later filters. This is covered by a test, including how\n     +    the parents can be simplified. Since \"struct object\" has already\n     +    ruined its 32-bit alignment by using 33 bits across parsed, type,\n     +    and flags member, let's not make it worse. DIVERSION is used in\n     +    revision.c with the same value (1u<<15) as REACHABLE in\n     +    commit-graph.c. The REACHABLE flag is only used when writing a\n     +    commit-graph file, and a revision walk using --include-diversions\n     +    does not happen in the same process. Care must be taken in the\n     +    future to ensure this remains the case.\n     +\n     +    Update Documentation/rev-list-options.txt with significant details\n     +    around this option. This requires updating the example in the\n     +    History Simplification section to demonstrate some of the problems\n     +    with TREESAME second parents.\n     +\n          Signed-off-by: Derrick Stolee <dstolee@microsoft.com>\n      \n       ## Documentation/rev-list-options.txt ##\n     @@ Documentation/rev-list-options.txt: Default mode::\n       --full-history::\n       \tSame as the default mode, but does not prune some history.\n       \n     +@@ Documentation/rev-list-options.txt: Note the major differences in `N`, `P`, and `Q` over `--full-history`:\n     +   parent and is TREESAME.\n     + --\n     + \n     +-Finally, there is a fifth simplification mode available:\n     ++There is another simplification mode available:\n     + \n     + --ancestry-path::\n     + \tLimit the displayed commits to those directly on the ancestry\n     +@@ Documentation/rev-list-options.txt: option does. Applied to the 'D..M' range, it results in:\n     + \t\t\t\tL--M\n     + -----------------------------------------------------------------------\n     + \n     ++Before discussing another option, `--include-diversions`, we need to\n     ++create a new example history.\n     +++\n     ++A common problem users face when looking at simplified history is that a\n     ++commit they know changed a file somehow does not appear in the file's\n     ++simplified history. Let's demonstrate a new example and show how options\n     ++such as `--full-history` and `--simplify-merges` works in that case:\n     +++\n     ++-----------------------------------------------------------------------\n     ++\t  .-A---M-----C--N---O---P\n     ++\t /     / \\  \\  \\/   /   /\n     ++\tI     B   \\  R-'`-Z'   /\n     ++\t \\   /     \\/         /\n     ++\t  \\ /      /\\        /\n     ++\t   `---X--'  `---Y--'\n     ++-----------------------------------------------------------------------\n     +++\n     ++For this example, suppose `I` created `file.txt` which was modified by\n     ++`A`, `B`, and `X` in different ways. The single-parent commits `C`, `Z`,\n     ++and `Y` do not change `file.txt`. The merge commit `M` was created by\n     ++resolving the merge conflict to include both changes from `A` and `B`\n     ++and hence is not TREESAME to either. The merge commit `R`, however, was\n     ++created by ignoring the contents of `file.txt` at `M` and taking only\n     ++the contents of `file.txt` at `X`. Hence, `R` is TREESAME to `X` but not\n     ++`M`. Finally, the natural merge resolution to create `N` is to take the\n     ++contents of `file.txt` at `R`, so `N` is TREESAME to `R` but not `C`.\n     ++The merge commits `O` and `P` are TREESAME to their first parents, but\n     ++not to their second parents, `Z` and `Y` respectively.\n     +++\n     ++When using the default mode, `N` and `R` both have a TREESAME parent, so\n     ++those edges are walked and the others are ignored. The resulting history\n     ++graph is:\n     +++\n     ++-----------------------------------------------------------------------\n     ++\tI---X\n     ++-----------------------------------------------------------------------\n     +++\n     ++When using `--full-history`, Git walks every edge. This will discover\n     ++the commits `A` and `B` and the merge `M`, but also will reveal the\n     ++merge commits `O` and `P`. With parent rewriting, the resulting graph is:\n     +++\n     ++-----------------------------------------------------------------------\n     ++\t  .-A---M--------N---O---P\n     ++\t /     / \\  \\  \\/   /   /\n     ++\tI     B   \\  R-'`--'   /\n     ++\t \\   /     \\/         /\n     ++\t  \\ /      /\\        /\n     ++\t   `---X--'  `------'\n     ++-----------------------------------------------------------------------\n     +++\n     ++Here, the merge commits `O` and `P` contribute extra noise, as they did\n     ++not actually contribute a change to `file.txt`. They only merged a topic\n     ++that was based on an older version of `file.txt`. This is a common\n     ++issue in repositories using a workflow where many contributors work in\n     ++parallel and merge their topic branches along a single trunk: manu\n     ++unrelated merges appear in the `--full-history` results.\n     +++\n     ++When using the `--simplify-merges` option, the commits `O` and `P`\n     ++disappear from the results. This is because the rewritten second parents\n     ++of `O` and `P` are reachable from their first parents. Those edges are\n     ++removed and then the commits look like single-parent commits that are\n     ++TREESAME to their parent. This also happens to the commit `N`, resulting\n     ++in a history view as follows:\n     +++\n     ++-----------------------------------------------------------------------\n     ++\t  .-A---M--.\n     ++\t /     /    \\\n     ++\tI     B      R\n     ++\t \\   /      /\n     ++\t  \\ /      /\n     ++\t   `---X--'\n     ++-----------------------------------------------------------------------\n     +++\n     ++In this view, we see all of the important single-parent changes from\n     ++`A`, `B`, and `X`. We also see the carefully-resolved merge `M` and the\n     ++not-so-carefully-resolved merge `R`. This is usually enough information\n     ++to determine why the commits `A` and `B` \"disappeared\" from history in\n     ++the default view. However, there are a few issues with this approach.\n     +++\n     ++The first issue is performance. Unlike any previous option, the\n     ++`--simplify-merges` option requires walking the entire commit history\n     ++before returning a single result. This can make the option difficult to\n     ++use for very large repositories.\n     +++\n     ++The second issue is one of auditing. When many contributors are working\n     ++on the same repository, it is important which merge commits introduced\n     ++a change into an important branch. The problematic merge `R` above is\n     ++not likely to be the merge commit that was used to merge into an\n     ++important branch. Instead, the merge `N` was used to merge `R` and `X`\n     ++into the important branch. This commit may have information about why\n     ++the change `X` came to override the changes from `A` and `B` in its\n     ++commit message.\n     +++\n     ++The `--include-diversions` option helps with both of these issues. A\n     ++merge commit is considered a \"diverter\" if it is not TREESAME to its\n     ++first parent but is TREESAME to a later parent. These merges \"divert\"\n     ++the history walk to a second parent instead of continuing along the\n     ++first-parent history as expected. When using `--include-diversions`\n     ++on this example (and no other options) the resulting graph is:\n     +++\n     ++-----------------------------------------------------------------------\n     ++\tI---X---R---N\n     ++-----------------------------------------------------------------------\n     +++\n     ++Here, the merge commits `R` and `N` are included because they diverted\n     ++the walk away from their first-parent history. They are the reason the\n     ++commits `A` and `B` do not appear in the history.\n     +++\n     ++When `--include-diversions` is paired with `--simplify-merges`, the\n     ++graph includes all of the necessary information:\n     +++\n     ++-----------------------------------------------------------------------\n     ++\t  .-A---M--.   N\n     ++\t /     /    \\ /\n     ++\tI     B      R\n     ++\t \\   /      /\n     ++\t  \\ /      /\n     ++\t   `---X--'\n     ++-----------------------------------------------------------------------\n     +++\n     ++Notice that since `M` is reachable from `R`, the edge from `N` to `M`\n     ++was simplified away. However, `N` still appears in the history as an\n     ++important commit because it would divert a simplified history walk.\n     ++\n     + The `--simplify-by-decoration` option allows you to view only the\n     + big picture of the topology of the history, by omitting commits\n     + that are not referenced by tags.  Commits are marked as !TREESAME\n     +\n     + ## object.h ##\n     +@@ object.h: struct object_array {\n     + \n     + /*\n     +  * object flag allocation:\n     +- * revision.h:               0---------10                              25----28\n     ++ * revision.h:               0---------10         15                   25----28\n     +  * fetch-pack.c:             01\n     +  * negotiator/default.c:       2--5\n     +  * walker.c:                 0-2\n      \n       ## revision.c ##\n      @@ revision.c: static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)\n     @@ revision.c: static void try_to_simplify_commit(struct rev_info *revs, struct com\n       \t\t\treturn;\n       \n       \t\tcase REV_TREE_NEW:\n     +@@ revision.c: static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)\n     + \t\t\t\trelevant_change = 1;\n     + \t\t\telse\n     + \t\t\t\tirrelevant_change = 1;\n     ++\n     ++\t\t\tif (!nth_parent)\n     ++\t\t\t\tcommit->object.flags |= DIVERSION;\n     ++\n     + \t\t\tcontinue;\n     + \t\t}\n     + \t\tdie(\"bad tree compare for commit %s\", oid_to_hex(&commit->object.oid));\n      @@ revision.c: static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n       \t} else if (!strcmp(arg, \"--full-diff\")) {\n       \t\trevs->diff = 1;\n     @@ revision.c: static int handle_revision_opt(struct rev_info *revs, int argc, cons\n       \t} else if (!strcmp(arg, \"--full-history\")) {\n       \t\trevs->simplify_history = 0;\n       \t} else if (!strcmp(arg, \"--relative-date\")) {\n     +@@ revision.c: static struct commit_list **simplify_one(struct rev_info *revs, struct commit *c\n     + \tif (!cnt ||\n     + \t    (commit->object.flags & UNINTERESTING) ||\n     + \t    !(commit->object.flags & TREESAME) ||\n     +-\t    (parent = one_relevant_parent(revs, commit->parents)) == NULL)\n     ++\t    (parent = one_relevant_parent(revs, commit->parents)) == NULL ||\n     ++\t    (revs->diversions && (commit->object.flags & DIVERSION)))\n     + \t\tst->simplified = commit;\n     + \telse {\n     + \t\tpst = locate_simplify_state(revs, parent);\n     +@@ revision.c: enum commit_action get_commit_action(struct rev_info *revs, struct commit *commi\n     + \t\t\t/* drop merges unless we want parenthood */\n     + \t\t\tif (!want_ancestry(revs))\n     + \t\t\t\treturn commit_ignore;\n     ++\n     ++\t\t\tif (revs->diversions && (commit->object.flags & DIVERSION))\n     ++\t\t\t\treturn commit_show;\n     ++\n     + \t\t\t/*\n     + \t\t\t * If we want ancestry, then need to keep any merges\n     + \t\t\t * between relevant commits to tie together topology.\n      \n       ## revision.h ##\n     +@@\n     + #define SYMMETRIC_LEFT\t(1u<<8)\n     + #define PATCHSAME\t(1u<<9)\n     + #define BOTTOM\t\t(1u<<10)\n     ++\n     ++/* WARNING: This is also used as REACHABLE in commit-graph.c. */\n     ++#define DIVERSION\t(1u<<15)\n     + /*\n     +  * Indicates object was reached by traversal. i.e. not given by user on\n     +  * command-line or stdin.\n     +@@\n     +  */\n     + #define NOT_USER_GIVEN\t(1u<<25)\n     + #define TRACK_LINEAR\t(1u<<26)\n     +-#define ALL_REV_FLAGS\t(((1u<<11)-1) | NOT_USER_GIVEN | TRACK_LINEAR)\n     ++#define ALL_REV_FLAGS\t(((1u<<11)-1) | NOT_USER_GIVEN | TRACK_LINEAR | DIVERSION)\n     + \n     + #define TOPO_WALK_EXPLORED\t(1u<<27)\n     + #define TOPO_WALK_INDEGREE\t(1u<<28)\n      @@ revision.h: struct rev_info {\n       \t\t\tno_walk:2,\n       \t\t\tremove_empty_trees:1,\n     @@ t/t6012-rev-list-simplify.sh: test_expect_success '--full-diff is not affected b\n       '\n       \n      +#\n     -+# Modify the test repo to add a merge whose first parent is not TREESAME\n     -+# but whose second parent is TREESAME\n     ++# Create a new history to demonstrate the value of --include-diversions\n     ++# with respect to the subtleties of simplified history, --full-history,\n     ++# and --simplify-merges.\n     ++#\n     ++#   .-A---M-----C--N---O---P\n     ++#  /     / \\  \\  \\/   /   /\n     ++# I     B   \\  R-'`-Z'   /\n     ++#  \\   /     \\/         /\n     ++#   \\ /      /\\        /\n     ++#    `---X--'  `---Y--'\n      +#\n     -+# A--B----------G--H--I--K--L--N\n     -+#  \\  \\           /     /     /\n     -+#   \\  \\         /     /     /\n     -+#    C------E---F     J     /\n     -+#     \\  \\_/               /\n     -+#      \\                  /\n     -+#       M-----------------\n     -+test_expect_success 'expand graph' '\n     -+\tgit switch -c branchM C &&\n     -+\techo \"new data\" >file &&\n     ++# This example is explained in Documentation/rev-list-options.txt\n     ++\n     ++test_expect_success 'rebuild repo' '\n     ++\trm -rf .git * &&\n     ++\tgit init &&\n     ++\tgit switch -c main &&\n     ++\n     ++\techo base >file &&\n      +\tgit add file &&\n     -+\ttest_tick &&\n     -+\ttest_commit M &&\n     ++\ttest_commit I &&\n      +\n     -+\tgit checkout master &&\n     -+\tgit merge -Xtheirs branchM -m \"N\" &&\n     -+\tnote N\n     ++\techo A >file &&\n     ++\tgit add file &&\n     ++\ttest_commit A &&\n     ++\n     ++\tgit switch -c branchB I &&\n     ++\techo B >file &&\n     ++\tgit add file &&\n     ++\ttest_commit B &&\n     ++\n     ++\tgit switch main &&\n     ++\ttest_must_fail git merge -m \"M\" B &&\n     ++\techo A >file &&\n     ++\techo B >>file &&\n     ++\tgit add file &&\n     ++\tgit merge --continue &&\n     ++\tnote M &&\n     ++\n     ++\techo C >other &&\n     ++\tgit add other &&\n     ++\ttest_commit C &&\n     ++\n     ++\tgit switch -c branchX I &&\n     ++\techo X >file &&\n     ++\tgit add file &&\n     ++\ttest_commit X &&\n     ++\n     ++\tgit switch -c branchR M &&\n     ++\tgit merge -m R -Xtheirs X &&\n     ++\tnote R &&\n     ++\n     ++\tgit switch main &&\n     ++\tgit merge -m N R &&\n     ++\tnote N &&\n     ++\n     ++\tgit switch -c branchY M &&\n     ++\techo Y >y &&\n     ++\tgit add y &&\n     ++\ttest_commit Y &&\n     ++\n     ++\tgit switch -c branchZ C &&\n     ++\techo Z >z &&\n     ++\tgit add z &&\n     ++\ttest_commit Z &&\n     ++\n     ++\tgit switch main &&\n     ++\tgit merge -m O Z &&\n     ++\tnote O &&\n     ++\n     ++\tgit merge -m P Y &&\n     ++\tnote P\n      +'\n      +\n     -+check_result 'M C A' -- file\n     -+check_result 'N M C A' --include-diversions -- file\n     -+\n     -+check_result 'N M L K J I H F E D C G B A' --full-history --topo-order\n     -+check_result 'N M L K I H G F E D C B J A' --full-history\n     -+check_result 'N M L K I H G F E D C B J A' --full-history --date-order\n     -+check_result 'N M L K I H G F E D B C J A' --full-history --author-date-order\n     -+check_result 'N M K I H E C B A' --full-history -- file\n     -+check_result 'N M K I H E C B A' --full-history --topo-order -- file\n     -+check_result 'N M K I H E C B A' --full-history --date-order -- file\n     -+check_result 'N M K I H E B C A' --full-history --author-date-order -- file\n     -+check_result 'N M I E C B A' --simplify-merges -- file\n     -+check_result 'N M I E C B A' --simplify-merges --topo-order -- file\n     -+check_result 'N M I E C B A' --simplify-merges --date-order -- file\n     -+check_result 'N M I E B C A' --simplify-merges --author-date-order -- file\n     -+check_result 'M C A' --topo-order -- file\n     -+check_result 'M C A' --date-order -- file\n     -+check_result 'M C A' --author-date-order -- file\n     -+check_result 'H' --first-parent -- another-file\n     -+check_result 'H' --first-parent --topo-order -- another-file\n     ++check_result 'X I' -- file\n     ++check_result 'N R X I' --include-diversions -- file\n     ++\n     ++check_result 'P O N R X M B A I' --full-history --topo-order -- file\n     ++check_result 'N R X M B A I' --simplify-merges --topo-order --include-diversions -- file\n     ++check_result 'R X M B A I' --simplify-merges --topo-order -- file\n     ++check_result 'N M A I' --first-parent -- file\n     ++check_result 'N M A I' --first-parent --include-diversions -- file\n     ++\n     ++# --ancestry-path implies --full-history\n     ++check_result 'P O N R M' --topo-order \\\n     ++\t--ancestry-path A..HEAD -- file\n     ++check_result 'P O N R M' --topo-order \\\n     ++\t--include-diversions \\\n     ++\t--ancestry-path A..HEAD -- file\n     ++check_result 'P O N R M' --topo-order \\\n     ++\t--full-history \\\n     ++\t--ancestry-path A..HEAD -- file\n     ++check_result 'R M' --topo-order \\\n     ++\t--simplify-merges \\\n     ++\t--ancestry-path A..HEAD -- file\n     ++check_result 'N R M' --topo-order \\\n     ++\t--simplify-merges --include-diversions \\\n     ++\t--ancestry-path A..HEAD -- file\n     ++\n     ++test_expect_success 'log --graph --simplify-merges --include-diversions' '\n     ++\tcat >expect <<-\\EOF &&\n     ++\t* N\n     ++\t*   R\n     ++\t|\\  \n     ++\t| * X\n     ++\t* |   M\n     ++\t|\\ \\  \n     ++\t| * | B\n     ++\t| |/  \n     ++\t* / A\n     ++\t|/  \n     ++\t* I\n     ++\tEOF\n     ++\tgit log --graph --pretty=\"%s\" \\\n     ++\t\t--simplify-merges --include-diversions \\\n     ++\t\t-- file >actual &&\n     ++\ttest_cmp expect actual\n     ++'\n      +\n       test_done\n\n\n Documentation/rev-list-options.txt | 132 ++++++++++++++++++++++++++++-\n object.h                           |   2 +-\n revision.c                         |  27 +++++-\n revision.h                         |   6 +-\n t/t6012-rev-list-simplify.sh       | 120 ++++++++++++++++++++++++++\n 5 files changed, 282 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex bfd02ade991..858352bf351 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -342,6 +342,12 @@ Default mode::\n \tbranches if the end result is the same (i.e. merging branches\n \twith the same content)\n \n+--include-diversions::\n+\tInclude all commits from the default mode, but also any merge\n+\tcommits that are not TREESAME to the first parent but are\n+\tTREESAME to a later parent. This mode is helpful for showing\n+\tthe merge commits that \"first introduced\" a change to a branch.\n+\n --full-history::\n \tSame as the default mode, but does not prune some history.\n \n@@ -534,7 +540,7 @@ Note the major differences in `N`, `P`, and `Q` over `--full-history`:\n   parent and is TREESAME.\n --\n \n-Finally, there is a fifth simplification mode available:\n+There is another simplification mode available:\n \n --ancestry-path::\n \tLimit the displayed commits to those directly on the ancestry\n@@ -573,6 +579,130 @@ option does. Applied to the 'D..M' range, it results in:\n \t\t\t\tL--M\n -----------------------------------------------------------------------\n \n+Before discussing another option, `--include-diversions`, we need to\n+create a new example history.\n++\n+A common problem users face when looking at simplified history is that a\n+commit they know changed a file somehow does not appear in the file's\n+simplified history. Let's demonstrate a new example and show how options\n+such as `--full-history` and `--simplify-merges` works in that case:\n++\n+-----------------------------------------------------------------------\n+\t  .-A---M-----C--N---O---P\n+\t /     / \\  \\  \\/   /   /\n+\tI     B   \\  R-'`-Z'   /\n+\t \\   /     \\/         /\n+\t  \\ /      /\\        /\n+\t   `---X--'  `---Y--'\n+-----------------------------------------------------------------------\n++\n+For this example, suppose `I` created `file.txt` which was modified by\n+`A`, `B`, and `X` in different ways. The single-parent commits `C`, `Z`,\n+and `Y` do not change `file.txt`. The merge commit `M` was created by\n+resolving the merge conflict to include both changes from `A` and `B`\n+and hence is not TREESAME to either. The merge commit `R`, however, was\n+created by ignoring the contents of `file.txt` at `M` and taking only\n+the contents of `file.txt` at `X`. Hence, `R` is TREESAME to `X` but not\n+`M`. Finally, the natural merge resolution to create `N` is to take the\n+contents of `file.txt` at `R`, so `N` is TREESAME to `R` but not `C`.\n+The merge commits `O` and `P` are TREESAME to their first parents, but\n+not to their second parents, `Z` and `Y` respectively.\n++\n+When using the default mode, `N` and `R` both have a TREESAME parent, so\n+those edges are walked and the others are ignored. The resulting history\n+graph is:\n++\n+-----------------------------------------------------------------------\n+\tI---X\n+-----------------------------------------------------------------------\n++\n+When using `--full-history`, Git walks every edge. This will discover\n+the commits `A` and `B` and the merge `M`, but also will reveal the\n+merge commits `O` and `P`. With parent rewriting, the resulting graph is:\n++\n+-----------------------------------------------------------------------\n+\t  .-A---M--------N---O---P\n+\t /     / \\  \\  \\/   /   /\n+\tI     B   \\  R-'`--'   /\n+\t \\   /     \\/         /\n+\t  \\ /      /\\        /\n+\t   `---X--'  `------'\n+-----------------------------------------------------------------------\n++\n+Here, the merge commits `O` and `P` contribute extra noise, as they did\n+not actually contribute a change to `file.txt`. They only merged a topic\n+that was based on an older version of `file.txt`. This is a common\n+issue in repositories using a workflow where many contributors work in\n+parallel and merge their topic branches along a single trunk: manu\n+unrelated merges appear in the `--full-history` results.\n++\n+When using the `--simplify-merges` option, the commits `O` and `P`\n+disappear from the results. This is because the rewritten second parents\n+of `O` and `P` are reachable from their first parents. Those edges are\n+removed and then the commits look like single-parent commits that are\n+TREESAME to their parent. This also happens to the commit `N`, resulting\n+in a history view as follows:\n++\n+-----------------------------------------------------------------------\n+\t  .-A---M--.\n+\t /     /    \\\n+\tI     B      R\n+\t \\   /      /\n+\t  \\ /      /\n+\t   `---X--'\n+-----------------------------------------------------------------------\n++\n+In this view, we see all of the important single-parent changes from\n+`A`, `B`, and `X`. We also see the carefully-resolved merge `M` and the\n+not-so-carefully-resolved merge `R`. This is usually enough information\n+to determine why the commits `A` and `B` \"disappeared\" from history in\n+the default view. However, there are a few issues with this approach.\n++\n+The first issue is performance. Unlike any previous option, the\n+`--simplify-merges` option requires walking the entire commit history\n+before returning a single result. This can make the option difficult to\n+use for very large repositories.\n++\n+The second issue is one of auditing. When many contributors are working\n+on the same repository, it is important which merge commits introduced\n+a change into an important branch. The problematic merge `R` above is\n+not likely to be the merge commit that was used to merge into an\n+important branch. Instead, the merge `N` was used to merge `R` and `X`\n+into the important branch. This commit may have information about why\n+the change `X` came to override the changes from `A` and `B` in its\n+commit message.\n++\n+The `--include-diversions` option helps with both of these issues. A\n+merge commit is considered a \"diverter\" if it is not TREESAME to its\n+first parent but is TREESAME to a later parent. These merges \"divert\"\n+the history walk to a second parent instead of continuing along the\n+first-parent history as expected. When using `--include-diversions`\n+on this example (and no other options) the resulting graph is:\n++\n+-----------------------------------------------------------------------\n+\tI---X---R---N\n+-----------------------------------------------------------------------\n++\n+Here, the merge commits `R` and `N` are included because they diverted\n+the walk away from their first-parent history. They are the reason the\n+commits `A` and `B` do not appear in the history.\n++\n+When `--include-diversions` is paired with `--simplify-merges`, the\n+graph includes all of the necessary information:\n++\n+-----------------------------------------------------------------------\n+\t  .-A---M--.   N\n+\t /     /    \\ /\n+\tI     B      R\n+\t \\   /      /\n+\t  \\ /      /\n+\t   `---X--'\n+-----------------------------------------------------------------------\n++\n+Notice that since `M` is reachable from `R`, the edge from `N` to `M`\n+was simplified away. However, `N` still appears in the history as an\n+important commit because it would divert a simplified history walk.\n+\n The `--simplify-by-decoration` option allows you to view only the\n big picture of the topology of the history, by omitting commits\n that are not referenced by tags.  Commits are marked as !TREESAME\ndiff --git a/object.h b/object.h\nindex 2dbabfca0ab..b22328b8383 100644\n--- a/object.h\n+++ b/object.h\n@@ -59,7 +59,7 @@ struct object_array {\n \n /*\n  * object flag allocation:\n- * revision.h:               0---------10                              25----28\n+ * revision.h:               0---------10         15                   25----28\n  * fetch-pack.c:             01\n  * negotiator/default.c:       2--5\n  * walker.c:                 0-2\ndiff --git a/revision.c b/revision.c\nindex 8136929e236..96f66400659 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -870,7 +870,19 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)\n \t\t\t}\n \t\t\tparent->next = NULL;\n \t\t\tcommit->parents = parent;\n-\t\t\tcommit->object.flags |= TREESAME;\n+\n+\t\t\t/*\n+\t\t\t * A merge commit is a \"diversion\" if it is not\n+\t\t\t * TREESAME to its first parent but is TREESAME\n+\t\t\t * to a later parent. In the simplified history,\n+\t\t\t * we \"divert\" the history walk to the later\n+\t\t\t * parent. These commits are shown when \"diversions\"\n+\t\t\t * is enabled, so do not mark the object as\n+\t\t\t * TREESAME here.\n+\t\t\t */\n+\t\t\tif (!revs->diversions || !nth_parent)\n+\t\t\t\tcommit->object.flags |= TREESAME;\n+\n \t\t\treturn;\n \n \t\tcase REV_TREE_NEW:\n@@ -897,6 +909,10 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)\n \t\t\t\trelevant_change = 1;\n \t\t\telse\n \t\t\t\tirrelevant_change = 1;\n+\n+\t\t\tif (!nth_parent)\n+\t\t\t\tcommit->object.flags |= DIVERSION;\n+\n \t\t\tcontinue;\n \t\t}\n \t\tdie(\"bad tree compare for commit %s\", oid_to_hex(&commit->object.oid));\n@@ -2265,6 +2281,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t} else if (!strcmp(arg, \"--full-diff\")) {\n \t\trevs->diff = 1;\n \t\trevs->full_diff = 1;\n+\t} else if (!strcmp(arg, \"--include-diversions\")) {\n+\t\trevs->diversions = 1;\n \t} else if (!strcmp(arg, \"--full-history\")) {\n \t\trevs->simplify_history = 0;\n \t} else if (!strcmp(arg, \"--relative-date\")) {\n@@ -3019,7 +3037,8 @@ static struct commit_list **simplify_one(struct rev_info *revs, struct commit *c\n \tif (!cnt ||\n \t    (commit->object.flags & UNINTERESTING) ||\n \t    !(commit->object.flags & TREESAME) ||\n-\t    (parent = one_relevant_parent(revs, commit->parents)) == NULL)\n+\t    (parent = one_relevant_parent(revs, commit->parents)) == NULL ||\n+\t    (revs->diversions && (commit->object.flags & DIVERSION)))\n \t\tst->simplified = commit;\n \telse {\n \t\tpst = locate_simplify_state(revs, parent);\n@@ -3602,6 +3621,10 @@ enum commit_action get_commit_action(struct rev_info *revs, struct commit *commi\n \t\t\t/* drop merges unless we want parenthood */\n \t\t\tif (!want_ancestry(revs))\n \t\t\t\treturn commit_ignore;\n+\n+\t\t\tif (revs->diversions && (commit->object.flags & DIVERSION))\n+\t\t\t\treturn commit_show;\n+\n \t\t\t/*\n \t\t\t * If we want ancestry, then need to keep any merges\n \t\t\t * between relevant commits to tie together topology.\ndiff --git a/revision.h b/revision.h\nindex 475f048fb61..f3c28e5f1c1 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -34,6 +34,9 @@\n #define SYMMETRIC_LEFT\t(1u<<8)\n #define PATCHSAME\t(1u<<9)\n #define BOTTOM\t\t(1u<<10)\n+\n+/* WARNING: This is also used as REACHABLE in commit-graph.c. */\n+#define DIVERSION\t(1u<<15)\n /*\n  * Indicates object was reached by traversal. i.e. not given by user on\n  * command-line or stdin.\n@@ -43,7 +46,7 @@\n  */\n #define NOT_USER_GIVEN\t(1u<<25)\n #define TRACK_LINEAR\t(1u<<26)\n-#define ALL_REV_FLAGS\t(((1u<<11)-1) | NOT_USER_GIVEN | TRACK_LINEAR)\n+#define ALL_REV_FLAGS\t(((1u<<11)-1) | NOT_USER_GIVEN | TRACK_LINEAR | DIVERSION)\n \n #define TOPO_WALK_EXPLORED\t(1u<<27)\n #define TOPO_WALK_INDEGREE\t(1u<<28)\n@@ -129,6 +132,7 @@ struct rev_info {\n \t\t\tno_walk:2,\n \t\t\tremove_empty_trees:1,\n \t\t\tsimplify_history:1,\n+\t\t\tdiversions:1,\n \t\t\ttopo_order:1,\n \t\t\tsimplify_merges:1,\n \t\t\tsimplify_by_decoration:1,\ndiff --git a/t/t6012-rev-list-simplify.sh b/t/t6012-rev-list-simplify.sh\nindex a10f0df02b0..23226f2144c 100755\n--- a/t/t6012-rev-list-simplify.sh\n+++ b/t/t6012-rev-list-simplify.sh\n@@ -154,4 +154,124 @@ test_expect_success '--full-diff is not affected by --parents' '\n \ttest_cmp expected actual\n '\n \n+#\n+# Create a new history to demonstrate the value of --include-diversions\n+# with respect to the subtleties of simplified history, --full-history,\n+# and --simplify-merges.\n+#\n+#   .-A---M-----C--N---O---P\n+#  /     / \\  \\  \\/   /   /\n+# I     B   \\  R-'`-Z'   /\n+#  \\   /     \\/         /\n+#   \\ /      /\\        /\n+#    `---X--'  `---Y--'\n+#\n+# This example is explained in Documentation/rev-list-options.txt\n+\n+test_expect_success 'rebuild repo' '\n+\trm -rf .git * &&\n+\tgit init &&\n+\tgit switch -c main &&\n+\n+\techo base >file &&\n+\tgit add file &&\n+\ttest_commit I &&\n+\n+\techo A >file &&\n+\tgit add file &&\n+\ttest_commit A &&\n+\n+\tgit switch -c branchB I &&\n+\techo B >file &&\n+\tgit add file &&\n+\ttest_commit B &&\n+\n+\tgit switch main &&\n+\ttest_must_fail git merge -m \"M\" B &&\n+\techo A >file &&\n+\techo B >>file &&\n+\tgit add file &&\n+\tgit merge --continue &&\n+\tnote M &&\n+\n+\techo C >other &&\n+\tgit add other &&\n+\ttest_commit C &&\n+\n+\tgit switch -c branchX I &&\n+\techo X >file &&\n+\tgit add file &&\n+\ttest_commit X &&\n+\n+\tgit switch -c branchR M &&\n+\tgit merge -m R -Xtheirs X &&\n+\tnote R &&\n+\n+\tgit switch main &&\n+\tgit merge -m N R &&\n+\tnote N &&\n+\n+\tgit switch -c branchY M &&\n+\techo Y >y &&\n+\tgit add y &&\n+\ttest_commit Y &&\n+\n+\tgit switch -c branchZ C &&\n+\techo Z >z &&\n+\tgit add z &&\n+\ttest_commit Z &&\n+\n+\tgit switch main &&\n+\tgit merge -m O Z &&\n+\tnote O &&\n+\n+\tgit merge -m P Y &&\n+\tnote P\n+'\n+\n+check_result 'X I' -- file\n+check_result 'N R X I' --include-diversions -- file\n+\n+check_result 'P O N R X M B A I' --full-history --topo-order -- file\n+check_result 'N R X M B A I' --simplify-merges --topo-order --include-diversions -- file\n+check_result 'R X M B A I' --simplify-merges --topo-order -- file\n+check_result 'N M A I' --first-parent -- file\n+check_result 'N M A I' --first-parent --include-diversions -- file\n+\n+# --ancestry-path implies --full-history\n+check_result 'P O N R M' --topo-order \\\n+\t--ancestry-path A..HEAD -- file\n+check_result 'P O N R M' --topo-order \\\n+\t--include-diversions \\\n+\t--ancestry-path A..HEAD -- file\n+check_result 'P O N R M' --topo-order \\\n+\t--full-history \\\n+\t--ancestry-path A..HEAD -- file\n+check_result 'R M' --topo-order \\\n+\t--simplify-merges \\\n+\t--ancestry-path A..HEAD -- file\n+check_result 'N R M' --topo-order \\\n+\t--simplify-merges --include-diversions \\\n+\t--ancestry-path A..HEAD -- file\n+\n+test_expect_success 'log --graph --simplify-merges --include-diversions' '\n+\tcat >expect <<-\\EOF &&\n+\t* N\n+\t*   R\n+\t|\\  \n+\t| * X\n+\t* |   M\n+\t|\\ \\  \n+\t| * | B\n+\t| |/  \n+\t* / A\n+\t|/  \n+\t* I\n+\tEOF\n+\tgit log --graph --pretty=\"%s\" \\\n+\t\t--simplify-merges --include-diversions \\\n+\t\t-- file >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n\nbase-commit: 274b9cc25322d9ee79aa8e6d4e86f0ffe5ced925\n-- \ngitgitgadget\n"},{"id":"395108","messageId":"xmqqimi9o705.fsf@gitster.c.googlers.com","threadId":"53185","inReplyTo":"91fafa17-c58f-dcd0-320d-48c4512561f6@gmail.com","subject":"Re: [PATCH] revision: --include-diversions adds helpful merges","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-09T00:08:42Z","receivedAt":"2020-04-09T00:08:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee <stolee@gmail.com> writes:\n\n>>> In my latest attempt at documentation, I called these merges \"diverters\"\n>>> yet still used \"--include-diversions\". Here are a few other words that we\n>>> could use:\n>>>\n>>>  * diverters or diversions\n>>>  * redirects\n>>>  * switches (think railroad switch). Synonym: exchange\n>>>  * detours\n>> \n>> ...none of the above tells me that they are not no-op (in other\n>> words, they do something meaningful), so I must be coming from\n>> a direction different from you are.  What redirects from what other\n>> thing, for example?\n>\n> The merges do something meaningful: they \"merge in\" a \"real\" change.\n\nYes, but \"redirect\", \"switch\", \"detour\", or \"divert\" do not quite\nmean \"merging in a real change\", at least to me.\n\n> I'll just submit my v2 as-is, which includes a significant change to\n> the documentation that should make things more clear.\n\nThanks.\n"},{"id":"395115","messageId":"076b4bf6-2bc5-97d2-2a50-d6d59ea85221@gmail.com","threadId":"53185","inReplyTo":"xmqqimi9o705.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] revision: --include-diversions adds helpful merges","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-04-09T11:52:34Z","receivedAt":"2020-04-09T11:52:40Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 4/8/2020 8:08 PM, Junio C Hamano wrote:\n> Derrick Stolee <stolee@gmail.com> writes:\n> \n>>>> In my latest attempt at documentation, I called these merges \"diverters\"\n>>>> yet still used \"--include-diversions\". Here are a few other words that we\n>>>> could use:\n>>>>\n>>>>  * diverters or diversions\n>>>>  * redirects\n>>>>  * switches (think railroad switch). Synonym: exchange\n>>>>  * detours\n>>>\n>>> ...none of the above tells me that they are not no-op (in other\n>>> words, they do something meaningful), so I must be coming from\n>>> a direction different from you are.  What redirects from what other\n>>> thing, for example?\n>>\n>> The merges do something meaningful: they \"merge in\" a \"real\" change.\n> \n> Yes, but \"redirect\", \"switch\", \"detour\", or \"divert\" do not quite\n> mean \"merging in a real change\", at least to me.\n\nMakes sense to me. The way you explain why certain words don't work\nfor you helps me think of new words to describe these merges:\n\n * signposts\n * guides\n * signals\n\nFor the argument, we cwould add \"-merge\" to each of these, such as\n\"--signpost-merges\" or \"--signal-merges\".\n\nI'm going to keep replying to this thread with ideas until someone\nsays \"This one makes sense to me\" or an equivalent. Alternatively,\nsomeone else could present an idea and then I get to say \"Aha!\nThat captures this concept clearly with an obvious metaphor!\"\n\nThanks,\n-Stolee\n"},{"id":"395126","messageId":"7d84fc45-786f-7a2e-5889-ef26916627ef@iee.email","threadId":"53185","inReplyTo":"xmqqimi9o705.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] revision: --include-diversions adds helpful merges","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2020-04-09T14:28:15Z","receivedAt":"2020-04-09T14:28:20Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Hi\n\nOn 09/04/2020 01:08, Junio C Hamano wrote:\n> Derrick Stolee <stolee@gmail.com> writes:\n>\n>>>> In my latest attempt at documentation, I called these merges \"diverters\"\n>>>> yet still used \"--include-diversions\". Here are a few other words that we\n>>>> could use:\n>>>>\n>>>>  * diverters or diversions\n>>>>  * redirects\n>>>>  * switches (think railroad switch). Synonym: exchange\n>>>>  * detours\n>>> ...none of the above tells me that they are not no-op (in other\n>>> words, they do something meaningful), so I must be coming from\n>>> a direction different from you are.  What redirects from what other\n>>> thing, for example?\n>> The merges do something meaningful: they \"merge in\" a \"real\" change.\n> Yes, but \"redirect\", \"switch\", \"detour\", or \"divert\" do not quite\n> mean \"merging in a real change\", at least to me.\n>\n>> I'll just submit my v2 as-is, which includes a significant change to\n>> the documentation that should make things more clear.\n> Thanks.\nCan I suggest \"--side-merges\" as a possible descriptor for these\nnon-mainline diversions?\n\nMy thesaurus had suggested detour and sidetracked, which led to the\nside-merge view.\n\nPhilip\n"},{"id":"395131","messageId":"xmqqeeswodol.fsf@gitster.c.googlers.com","threadId":"53185","inReplyTo":"7d84fc45-786f-7a2e-5889-ef26916627ef@iee.email","subject":"Re: [PATCH] revision: --include-diversions adds helpful merges","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-09T15:56:42Z","receivedAt":"2020-04-09T15:56:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Philip Oakley <philipoakley@iee.email> writes:\n\n>> Yes, but \"redirect\", \"switch\", \"detour\", or \"divert\" do not quite\n>> mean \"merging in a real change\", at least to me.\n>>\n>>> I'll just submit my v2 as-is, which includes a significant change to\n>>> the documentation that should make things more clear.\n>> Thanks.\n> Can I suggest \"--side-merges\" as a possible descriptor for these\n> non-mainline diversions?\n>\n> My thesaurus had suggested detour and sidetracked, which led to the\n> side-merge view.\n\nAhh, sorry Derrick for being slow and thanks Philip for repeating\n\"diversion\", as the word did not click for me at all when I saw the\npatch and wrote my response.\n\nBut I think it started slowly to dawn on me.  \n\nDoes it come from the worldview where we want to follow the \"trunk\"\nbut because when we notice at a merge that we got everything that\nmatters to us from a side branch, we switch the track out of the\nmainline and from then on follow that side branch?  Switching the\ntrack and following the side branch happens silently with the\ndefault \"history simplification\", but the new feature shows where\nthat side-tracking happens more prominently---is that where the\nwords \"divert\" etc. come from?\n\nThen I can understand how these candidate words may have place in\ndescribing the situation we want to use the feature; I am not yet\nconvinced any of the concrete option names floated on the thread (or\nwhat I can come up with right now) would be clear to our target\naudiences, but at least I am not as confused as I was before.\n\nThanks.\n"},{"id":"395132","messageId":"63667043-dbca-0092-209d-865c2f60efc8@gmail.com","threadId":"53185","inReplyTo":"xmqqeeswodol.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] revision: --include-diversions adds helpful merges","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-04-09T17:20:57Z","receivedAt":"2020-04-09T17:21:02Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 4/9/2020 11:56 AM, Junio C Hamano wrote:\n> Philip Oakley <philipoakley@iee.email> writes:\n> \n>>> Yes, but \"redirect\", \"switch\", \"detour\", or \"divert\" do not quite\n>>> mean \"merging in a real change\", at least to me.\n>>>\n>>>> I'll just submit my v2 as-is, which includes a significant change to\n>>>> the documentation that should make things more clear.\n>>> Thanks.\n>> Can I suggest \"--side-merges\" as a possible descriptor for these\n>> non-mainline diversions?\n>>\n>> My thesaurus had suggested detour and sidetracked, which led to the\n>> side-merge view.\n> \n> Ahh, sorry Derrick for being slow and thanks Philip for repeating\n> \"diversion\", as the word did not click for me at all when I saw the\n> patch and wrote my response.\n> \n> But I think it started slowly to dawn on me.  \n> \n> Does it come from the worldview where we want to follow the \"trunk\"\n> but because when we notice at a merge that we got everything that\n> matters to us from a side branch, we switch the track out of the\n> mainline and from then on follow that side branch?  Switching the\n> track and following the side branch happens silently with the\n> default \"history simplification\", but the new feature shows where\n> that side-tracking happens more prominently---is that where the\n> words \"divert\" etc. come from?\n> \n> Then I can understand how these candidate words may have place in\n> describing the situation we want to use the feature; I am not yet\n> convinced any of the concrete option names floated on the thread (or\n> what I can come up with right now) would be clear to our target\n> audiences, but at least I am not as confused as I was before.\n\nAfter thinking about all the great responses here, and having a\nchat with Dscho about this, then taking a break, I had an \"Aha!\"\nmoment. We should call this option:\n\n\t--show-pulls\n\nThe direction here is important. Let's look at a potential\n\"git log --graph --oneline\" output to explore this idea:\n\n\t* (master) A\n\t|\\\n\t| * (feature) B\n\t| |\\\n\t| | *   (topic) C\n\t| | |\\\n\t| | | |\n\t| | * | D\n\t| | | |\n\t| * | | E\n\t| | | |\n\t* | | | F\n\t| |_|/\n\t|/| |\n\t* | |   G\n\t|/ /\n\t* /     H\n\t|/\n\t*       I\n\nI use (master), (feature), and (topic) to decorate branches\nthat are updated only by \"git commit\" or \"git pull\". The\nfile 'foo' was created by commit I.\n\nIn this graph, the single-parent commits G and D change 'foo'.\nThe commit G enters master using \"git commit\".\n\nThe commit G enters topic using \"git pull\" starting from D. The\ndeveloper on that branch resolves conflicts by taking the version\nof 'foo' from D. Thus C is TREESAME to D but not G.\n\nThe commit B is created by running \"git pull topic\" from the\nfeature branch.\n\nThe commit A is created by running \"git pull feature\" from the\nmaster branch.\n\nThus, A and B \"pulled\" the change into their branches. The\ncommit C \"pulled\" G into the branch, but did not keep the change\nto 'foo'.\n\nThus 'git log --graph --oneline master -- foo' would output:\n\n\n\t* D\n\t* I\n\n'git log --graph --oneline --show-pulls master -- foo' shows:\n\n\t* A\n\t* B\n\t* D\n\t* I\n\n'git log --graph --oneline --full-history -- foo' shows:\n\n\t* (master) A\n\t|\\\n\t| * (feature) B\n\t| |\\\n\t| | *   (topic) C\n\t| | |\\\n\t| | | |\n\t| | * | D\n\t| |_|/\n\t|/| |\n\t* | |   G\n\t|/ /\n\t| /\n\t|/\n\t*       I\n\n'git log --graph --oneline --full-history --simplify-merges -- foo'\nwould show:\n\n\t*   C\n\t|\\\n\t* | D\n\t| |\n\t| * G\n\t|/\n\t*   I\n\n'git log --graph --oneline --full-history --simplify-merges --show-pulls -- foo'\nwould show:\n\n\t* A\n\t* B\n\t*   C\n\t|\\\n\t* | D\n\t| |\n\t| * G\n\t|/\n\t*   I\n\nIn conclusion, I think \"--show-pulls\" provides the right context for these\nextra merges to show in the history view. It also roots these merges in a\nGit-native name (that also happens to evoke the \"pull request\" concept that\nis _not_ native to Git).\n\nWhat do you think?\n\nThanks,\n-Stolee\n"},{"id":"395136","messageId":"20200409182455.GA4166316@coredump.intra.peff.net","threadId":"53185","inReplyTo":"63667043-dbca-0092-209d-865c2f60efc8@gmail.com","subject":"Re: [PATCH] revision: --include-diversions adds helpful merges","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-04-09T18:24:55Z","receivedAt":"2020-04-09T18:24:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 09, 2020 at 01:20:57PM -0400, Derrick Stolee wrote:\n\n> In conclusion, I think \"--show-pulls\" provides the right context for these\n> extra merges to show in the history view. It also roots these merges in a\n> Git-native name (that also happens to evoke the \"pull request\" concept that\n> is _not_ native to Git).\n> \n> What do you think?\n\nYeah, after reading more of the thread, I think the simplest way to\nthink about is \"keep merges that pulled in something\" with the\nimplication of \"(even if the other side didn't touch anything)\".\n\nAnd \"something you pulled\" is a sensible way to think of that. So\n--show-pulls makes sense to me. Or if we really want to tie it in to\nsimplification, --no-simplify-pulls. But that's more awkward to type,\nand none of the existing simplification options use the word simplify. ;)\n\n-Peff\n"},{"id":"395137","messageId":"xmqq369co5e0.fsf@gitster.c.googlers.com","threadId":"53185","inReplyTo":"20200409182455.GA4166316@coredump.intra.peff.net","subject":"Re: [PATCH] revision: --include-diversions adds helpful merges","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-09T18:55:51Z","receivedAt":"2020-04-09T18:56:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Thu, Apr 09, 2020 at 01:20:57PM -0400, Derrick Stolee wrote:\n>\n>> In conclusion, I think \"--show-pulls\" provides the right context for these\n>> extra merges to show in the history view. It also roots these merges in a\n>> Git-native name (that also happens to evoke the \"pull request\" concept that\n>> is _not_ native to Git).\n>> \n>> What do you think?\n>\n> Yeah, after reading more of the thread, I think the simplest way to\n> think about is \"keep merges that pulled in something\" with the\n> implication of \"(even if the other side didn't touch anything)\".\n\nIsn't it more like \"even if our side didn't touch anything\", though?\n\nIf a merge pulled in something, the other side by definition did\nsomething (i.e. what was pulled in); if we did something since they\nforked, we would have shown the merge without this patch---the only\nnew behaviour we are adding is to show the merge even when our side\ndidn't touch since they forked---so far we never showed that merge,\nbut now with this option we would when we are asked to.\n\nI agree that \"this is showing pulls\" is an easy way to explain.\n\n> And \"something you pulled\" is a sensible way to think of that. So\n> --show-pulls makes sense to me. Or if we really want to tie it in to\n> simplification, --no-simplify-pulls. But that's more awkward to type,\n> and none of the existing simplification options use the word simplify. ;)\n\n;-)\n \n"},{"id":"395139","messageId":"20200409192149.GA4168991@coredump.intra.peff.net","threadId":"53185","inReplyTo":"xmqq369co5e0.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] revision: --include-diversions adds helpful merges","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-04-09T19:21:49Z","receivedAt":"2020-04-09T19:21:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 09, 2020 at 11:55:51AM -0700, Junio C Hamano wrote:\n\n> > On Thu, Apr 09, 2020 at 01:20:57PM -0400, Derrick Stolee wrote:\n> >\n> >> In conclusion, I think \"--show-pulls\" provides the right context for these\n> >> extra merges to show in the history view. It also roots these merges in a\n> >> Git-native name (that also happens to evoke the \"pull request\" concept that\n> >> is _not_ native to Git).\n> >> \n> >> What do you think?\n> >\n> > Yeah, after reading more of the thread, I think the simplest way to\n> > think about is \"keep merges that pulled in something\" with the\n> > implication of \"(even if the other side didn't touch anything)\".\n> \n> Isn't it more like \"even if our side didn't touch anything\", though?\n\nI meant the _other_ other. :) I.e., the other one that is not what just\ngot pulled in. Which is the first parent. ;)\n\nSo yes, I think we are on the same page, and I just said it badly. Using\n\"our side\" is better than trying to double-negate \"other\".\n\n-Peff\n"},{"id":"395180","messageId":"pull.599.v3.git.1586521183873.gitgitgadget@gmail.com","threadId":"53185","inReplyTo":"pull.599.v2.git.1586390501128.gitgitgadget@gmail.com","subject":"[PATCH v3] revision: --show-pulls adds helpful merges","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-04-10T12:19:43Z","receivedAt":"2020-04-10T12:19:53Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee <dstolee@microsoft.com>\n\nThe default file history simplification of \"git log -- <path>\" or\n\"git rev-list -- <path>\" focuses on providing the smallest set of\ncommits that first contributed a change. The revision walk greatly\nrestricts the set of walked commits by visiting only the first\nTREESAME parent of a merge commit, when one exists. This means\nthat portions of the commit-graph are not walked, which can be a\nperformance benefit, but can also \"hide\" commits that added changes\nbut were ignored by a merge resolution.\n\nThe --full-history option modifies this by walking all commits and\nreporting a merge commit as \"interesting\" if it has _any_ parent\nthat is not TREESAME. This tends to be an over-representation of\nimportant commits, especially in an environment where most merge\ncommits are created by pull request completion.\n\nSuppose we have a commit A and we create a commit B on top that\nchanges our file. When we merge the pull request, we create a merge\ncommit M. If no one else changed the file in the first-parent\nhistory between M and A, then M will not be TREESAME to its first\nparent, but will be TREESAME to B. Thus, the simplified history\nwill be \"B\". However, M will appear in the --full-history mode.\n\nHowever, suppose that a number of topics T1, T2, ..., Tn were\ncreated based on commits C1, C2, ..., Cn between A and M as\nfollows:\n\n  A----C1----C2--- ... ---Cn----M------P1---P2--- ... ---Pn\n   \\     \\     \\            \\  /      /    /            /\n    \\     \\__.. \\            \\/ ..__T1    /           Tn\n     \\           \\__..       /\\     ..__T2           /\n      \\_____________________B  \\____________________/\n\nIf the commits T1, T2, ... Tn did not change the file, then all of\nP1 through Pn will be TREESAME to their first parent, but not\nTREESAME to their second. This means that all of those merge commits\nappear in the --full-history view, with edges that immediately\ncollapse into the lower history without introducing interesting\nsingle-parent commits.\n\nThe --simplify-merges option was introduced to remove these extra\nmerge commits. By noticing that the rewritten parents are reachable\nfrom their first parents, those edges can be simplified away. Finally,\nthe commits now look like single-parent commits that are TREESAME to\ntheir \"only\" parent. Thus, they are removed and this issue does not\ncause issues anymore. However, this also ends up removing the commit\nM from the history view! Even worse, the --simplify-merges option\nrequires walking the entire history before returning a single result.\n\nMany Git users are using Git alongside a Git service that provides\ncode storage alongside a code review tool commonly called \"Pull\nRequests\" or \"Merge Requests\" against a target branch.  When these\nrequests are accepted and merged, they typically create a merge\ncommit whose first parent is the previous branch tip and the second\nparent is the tip of the topic branch used for the request. This\npresents a valuable order to the parents, but also makes that merge\ncommit slightly special. Users may want to see not only which\ncommits changed a file, but which pull requests merged those commits\ninto their branch. In the previous example, this would mean the\nusers want to see the merge commit \"M\" in addition to the single-\nparent commit \"C\".\n\nUsers are even more likely to want these merge commits when they\nuse pull requests to merge into a feature branch before merging that\nfeature branch into their trunk.\n\nIn some sense, users are asking for the \"first\" merge commit to\nbring in the change to their branch. As long as the parent order is\nconsistent, this can be handled with the following rule:\n\n  Include a merge commit if it is not TREESAME to its first\n  parent, but is TREESAME to a later parent.\n\nThese merges look like the merge commits that would result from\nrunning \"git pull <topic>\" on a main branch. Thus, the option to\nshow these commits is called \"--show-pulls\". This has the added\nbenefit of showing the commits created by closing a pull request or\nmerge request on any of the Git hosting and code review platforms.\n\nTo test these options, extend the standard test example to include\na merge commit that is not TREESAME to its first parent. It is\nsurprising that that option was not already in the example, as it\nis instructive.\n\nIn particular, this extension demonstrates a common issue with file\nhistory simplification. When a user resolves a merge conflict using\n\"-Xours\" or otherwise ignoring one side of the conflict, they create\na TREESAME edge that probably should not be TREESAME. This leads\nusers to become frustrated and complain that \"my change disappeared!\"\nIn my experience, showing them history with --full-history and\n--simplify-merges quickly reveals the problematic merge. As mentioned,\nthis option is expensive to compute. The --show-pulls option\n_might_ show the merge commit (usually titled \"resolving conflicts\")\nmore quickly. Of course, this depends on the user having the correct\nparent order, which is backwards when using \"git pull master\" from a\ntopic branch.\n\nThere are some special considerations when combining the --show-pulls\noption with --simplify-merges. This requires adding a new PULL_MERGE\nobject flag to store the information from the initial TREESAME\ncomparisons. This helps avoid dropping those commits in later filters.\nThis is covered by a test, including how the parents can be simplified.\nSince \"struct object\" has already ruined its 32-bit alignment by using\n33 bits across parsed, type, and flags member, let's not make it worse.\nPULL_MERGE is used in revision.c with the same value (1u<<15) as\nREACHABLE in commit-graph.c. The REACHABLE flag is only used when\nwriting a commit-graph file, and a revision walk using --show-pulls\ndoes not happen in the same process. Care must be taken in the future\nto ensure this remains the case.\n\nUpdate Documentation/rev-list-options.txt with significant details\naround this option. This requires updating the example in the\nHistory Simplification section to demonstrate some of the problems\nwith TREESAME second parents.\n\nSigned-off-by: Derrick Stolee <dstolee@microsoft.com>\n---\n    Add a new history mode\n    \n    This --include-diversions option could use a better name.\n    \n    An experienced developer in the Windows OS Engineering Systems team\n    pointed out how hard it is to find out when a change was \"introduced\" in\n    the Windows OS repo. Due to their multi-leveled, long-lived branch\n    organization, a commit could be part of hundreds of pull requests as the\n    branches are merged across the organization.\n    \n    My default answer was \"this is complicated not because of Git, but\n    because of how you are branching.\" I then tried to explain how finding\n    the \"first merge\" to include a commit is incredibly difficult and\n    requires performing multiple reachability queries. As I was working it\n    out on paper, I realized that was true if we relied only on the\n    commit-graph shape to inform our qurey.\n    \n    If we use the TREESAME information, then suddenly we get a much clearer\n    picture! Let's simply pick out those merge commits that \"introduced a\n    change\" because they are TREESAME to a non-first-parent (and not\n    TREESAME to the first parent).\n    \n    My name of \"diversions\" could probably use some work, but I like the\n    basic concept of this option.\n    \n    I welcome any and all feedback. Thanks!\n    \n    UPDATES in v2:\n    \n     * The functionality is a bit more complicated to work with\n       --simplify-merges.\n       \n       \n     * The documentation is significantly expanded with an example that\n       highlights the shortcomings of the default history simplification.\n       \n       \n     * The documented example is used in the test script instead of simply\n       extending the previous example.\n       \n       \n    \n    UPDATES in v3:\n    \n     * Renamed things appropriately for the shift from\n       \"--include-diversions\" to \"--show-pulls\".\n       \n       \n     * Slight rewording in the documentation to remove references to\n       \"diverters\".\n       \n       \n    \n    I don't recommend looking at the range-diff for this version.\n    \n    -Stolee\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-599%2Fderrickstolee%2Fnew-history-mode-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-599/derrickstolee/new-history-mode-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/599\n\nRange-diff vs v2:\n\n 1:  146c443e7f1 ! 1:  19026f4d4c0 revision: --include-diversions adds helpful merges\n     @@ Metadata\n      Author: Derrick Stolee <dstolee@microsoft.com>\n      \n       ## Commit message ##\n     -    revision: --include-diversions adds helpful merges\n     +    revision: --show-pulls adds helpful merges\n      \n          The default file history simplification of \"git log -- <path>\" or\n          \"git rev-list -- <path>\" focuses on providing the smallest set of\n     @@ Commit message\n            Include a merge commit if it is not TREESAME to its first\n            parent, but is TREESAME to a later parent.\n      \n     -    I call such merge commits \"diversions\" because they divert the\n     -    history walk away from the first-parent history. As such, this\n     -    change adds the \"--include-diversions\" option to rev-list and log.\n     +    These merges look like the merge commits that would result from\n     +    running \"git pull <topic>\" on a main branch. Thus, the option to\n     +    show these commits is called \"--show-pulls\". This has the added\n     +    benefit of showing the commits created by closing a pull request or\n     +    merge request on any of the Git hosting and code review platforms.\n     +\n          To test these options, extend the standard test example to include\n          a merge commit that is not TREESAME to its first parent. It is\n          surprising that that option was not already in the example, as it\n     @@ Commit message\n          users to become frustrated and complain that \"my change disappeared!\"\n          In my experience, showing them history with --full-history and\n          --simplify-merges quickly reveals the problematic merge. As mentioned,\n     -    this option is expensive to compute. The --include-diversions option\n     +    this option is expensive to compute. The --show-pulls option\n          _might_ show the merge commit (usually titled \"resolving conflicts\")\n          more quickly. Of course, this depends on the user having the correct\n     -    parent order, which is backwards when using \"git pull\".\n     +    parent order, which is backwards when using \"git pull master\" from a\n     +    topic branch.\n      \n     -    There are some special considerations when combining the\n     -    --include-diversions option with --simplify-merges. This requires\n     -    adding a new DIVERSION object flag to store the information from\n     -    the initial TREESAME comparisons. This helps avoid dropping those\n     -    commits in later filters. This is covered by a test, including how\n     -    the parents can be simplified. Since \"struct object\" has already\n     -    ruined its 32-bit alignment by using 33 bits across parsed, type,\n     -    and flags member, let's not make it worse. DIVERSION is used in\n     -    revision.c with the same value (1u<<15) as REACHABLE in\n     -    commit-graph.c. The REACHABLE flag is only used when writing a\n     -    commit-graph file, and a revision walk using --include-diversions\n     -    does not happen in the same process. Care must be taken in the\n     -    future to ensure this remains the case.\n     +    There are some special considerations when combining the --show-pulls\n     +    option with --simplify-merges. This requires adding a new PULL_MERGE\n     +    object flag to store the information from the initial TREESAME\n     +    comparisons. This helps avoid dropping those commits in later filters.\n     +    This is covered by a test, including how the parents can be simplified.\n     +    Since \"struct object\" has already ruined its 32-bit alignment by using\n     +    33 bits across parsed, type, and flags member, let's not make it worse.\n     +    PULL_MERGE is used in revision.c with the same value (1u<<15) as\n     +    REACHABLE in commit-graph.c. The REACHABLE flag is only used when\n     +    writing a commit-graph file, and a revision walk using --show-pulls\n     +    does not happen in the same process. Care must be taken in the future\n     +    to ensure this remains the case.\n      \n          Update Documentation/rev-list-options.txt with significant details\n          around this option. This requires updating the example in the\n     @@ Documentation/rev-list-options.txt: Default mode::\n       \tbranches if the end result is the same (i.e. merging branches\n       \twith the same content)\n       \n     -+--include-diversions::\n     ++--show-pulls::\n      +\tInclude all commits from the default mode, but also any merge\n      +\tcommits that are not TREESAME to the first parent but are\n      +\tTREESAME to a later parent. This mode is helpful for showing\n     @@ Documentation/rev-list-options.txt: option does. Applied to the 'D..M' range, it\n       \t\t\t\tL--M\n       -----------------------------------------------------------------------\n       \n     -+Before discussing another option, `--include-diversions`, we need to\n     ++Before discussing another option, `--show-pulls`, we need to\n      +create a new example history.\n      ++\n      +A common problem users face when looking at simplified history is that a\n     @@ Documentation/rev-list-options.txt: option does. Applied to the 'D..M' range, it\n      +the change `X` came to override the changes from `A` and `B` in its\n      +commit message.\n      ++\n     -+The `--include-diversions` option helps with both of these issues. A\n     -+merge commit is considered a \"diverter\" if it is not TREESAME to its\n     -+first parent but is TREESAME to a later parent. These merges \"divert\"\n     -+the history walk to a second parent instead of continuing along the\n     -+first-parent history as expected. When using `--include-diversions`\n     -+on this example (and no other options) the resulting graph is:\n     ++The `--show-pulls` option helps with both of these issues by adding more\n     ++merge commits to the history results. If a merge is not TREESAME to its\n     ++first parent but is TREESAME to a later parent, then that merge is\n     ++treated as if it \"pulled\" the change from another branch. When using\n     ++`--show-pulls` on this example (and no other options) the resulting\n     ++graph is:\n      ++\n      +-----------------------------------------------------------------------\n      +\tI---X---R---N\n      +-----------------------------------------------------------------------\n      ++\n     -+Here, the merge commits `R` and `N` are included because they diverted\n     -+the walk away from their first-parent history. They are the reason the\n     -+commits `A` and `B` do not appear in the history.\n     ++Here, the merge commits `R` and `N` are included because they pulled\n     ++the commits `X` and `R` into the base branch, respectively. These\n     ++merges are the reason the commits `A` and `B` do not appear in the\n     ++default history.\n      ++\n     -+When `--include-diversions` is paired with `--simplify-merges`, the\n     ++When `--show-pulls` is paired with `--simplify-merges`, the\n      +graph includes all of the necessary information:\n      ++\n      +-----------------------------------------------------------------------\n     @@ Documentation/rev-list-options.txt: option does. Applied to the 'D..M' range, it\n      ++\n      +Notice that since `M` is reachable from `R`, the edge from `N` to `M`\n      +was simplified away. However, `N` still appears in the history as an\n     -+important commit because it would divert a simplified history walk.\n     ++important commit because it \"pulled\" the change `R` into the main\n     ++branch.\n      +\n       The `--simplify-by-decoration` option allows you to view only the\n       big picture of the topology of the history, by omitting commits\n     @@ revision.c: static void try_to_simplify_commit(struct rev_info *revs, struct com\n      +\t\t\t * TREESAME to its first parent but is TREESAME\n      +\t\t\t * to a later parent. In the simplified history,\n      +\t\t\t * we \"divert\" the history walk to the later\n     -+\t\t\t * parent. These commits are shown when \"diversions\"\n     ++\t\t\t * parent. These commits are shown when \"show_pulls\"\n      +\t\t\t * is enabled, so do not mark the object as\n      +\t\t\t * TREESAME here.\n      +\t\t\t */\n     -+\t\t\tif (!revs->diversions || !nth_parent)\n     ++\t\t\tif (!revs->show_pulls || !nth_parent)\n      +\t\t\t\tcommit->object.flags |= TREESAME;\n      +\n       \t\t\treturn;\n     @@ revision.c: static void try_to_simplify_commit(struct rev_info *revs, struct com\n       \t\t\t\tirrelevant_change = 1;\n      +\n      +\t\t\tif (!nth_parent)\n     -+\t\t\t\tcommit->object.flags |= DIVERSION;\n     ++\t\t\t\tcommit->object.flags |= PULL_MERGE;\n      +\n       \t\t\tcontinue;\n       \t\t}\n     @@ revision.c: static int handle_revision_opt(struct rev_info *revs, int argc, cons\n       \t} else if (!strcmp(arg, \"--full-diff\")) {\n       \t\trevs->diff = 1;\n       \t\trevs->full_diff = 1;\n     -+\t} else if (!strcmp(arg, \"--include-diversions\")) {\n     -+\t\trevs->diversions = 1;\n     ++\t} else if (!strcmp(arg, \"--show-pulls\")) {\n     ++\t\trevs->show_pulls = 1;\n       \t} else if (!strcmp(arg, \"--full-history\")) {\n       \t\trevs->simplify_history = 0;\n       \t} else if (!strcmp(arg, \"--relative-date\")) {\n     @@ revision.c: static struct commit_list **simplify_one(struct rev_info *revs, stru\n       \t    !(commit->object.flags & TREESAME) ||\n      -\t    (parent = one_relevant_parent(revs, commit->parents)) == NULL)\n      +\t    (parent = one_relevant_parent(revs, commit->parents)) == NULL ||\n     -+\t    (revs->diversions && (commit->object.flags & DIVERSION)))\n     ++\t    (revs->show_pulls && (commit->object.flags & PULL_MERGE)))\n       \t\tst->simplified = commit;\n       \telse {\n       \t\tpst = locate_simplify_state(revs, parent);\n     @@ revision.c: enum commit_action get_commit_action(struct rev_info *revs, struct c\n       \t\t\tif (!want_ancestry(revs))\n       \t\t\t\treturn commit_ignore;\n      +\n     -+\t\t\tif (revs->diversions && (commit->object.flags & DIVERSION))\n     ++\t\t\tif (revs->show_pulls && (commit->object.flags & PULL_MERGE))\n      +\t\t\t\treturn commit_show;\n      +\n       \t\t\t/*\n     @@ revision.h\n       #define BOTTOM\t\t(1u<<10)\n      +\n      +/* WARNING: This is also used as REACHABLE in commit-graph.c. */\n     -+#define DIVERSION\t(1u<<15)\n     ++#define PULL_MERGE\t(1u<<15)\n       /*\n        * Indicates object was reached by traversal. i.e. not given by user on\n        * command-line or stdin.\n     @@ revision.h\n       #define NOT_USER_GIVEN\t(1u<<25)\n       #define TRACK_LINEAR\t(1u<<26)\n      -#define ALL_REV_FLAGS\t(((1u<<11)-1) | NOT_USER_GIVEN | TRACK_LINEAR)\n     -+#define ALL_REV_FLAGS\t(((1u<<11)-1) | NOT_USER_GIVEN | TRACK_LINEAR | DIVERSION)\n     ++#define ALL_REV_FLAGS\t(((1u<<11)-1) | NOT_USER_GIVEN | TRACK_LINEAR | PULL_MERGE)\n       \n       #define TOPO_WALK_EXPLORED\t(1u<<27)\n       #define TOPO_WALK_INDEGREE\t(1u<<28)\n     @@ revision.h: struct rev_info {\n       \t\t\tno_walk:2,\n       \t\t\tremove_empty_trees:1,\n       \t\t\tsimplify_history:1,\n     -+\t\t\tdiversions:1,\n     ++\t\t\tshow_pulls:1,\n       \t\t\ttopo_order:1,\n       \t\t\tsimplify_merges:1,\n       \t\t\tsimplify_by_decoration:1,\n     @@ t/t6012-rev-list-simplify.sh: test_expect_success '--full-diff is not affected b\n       '\n       \n      +#\n     -+# Create a new history to demonstrate the value of --include-diversions\n     ++# Create a new history to demonstrate the value of --show-pulls\n      +# with respect to the subtleties of simplified history, --full-history,\n      +# and --simplify-merges.\n      +#\n     @@ t/t6012-rev-list-simplify.sh: test_expect_success '--full-diff is not affected b\n      +'\n      +\n      +check_result 'X I' -- file\n     -+check_result 'N R X I' --include-diversions -- file\n     ++check_result 'N R X I' --show-pulls -- file\n      +\n      +check_result 'P O N R X M B A I' --full-history --topo-order -- file\n     -+check_result 'N R X M B A I' --simplify-merges --topo-order --include-diversions -- file\n     ++check_result 'N R X M B A I' --simplify-merges --topo-order --show-pulls -- file\n      +check_result 'R X M B A I' --simplify-merges --topo-order -- file\n      +check_result 'N M A I' --first-parent -- file\n     -+check_result 'N M A I' --first-parent --include-diversions -- file\n     ++check_result 'N M A I' --first-parent --show-pulls -- file\n      +\n      +# --ancestry-path implies --full-history\n      +check_result 'P O N R M' --topo-order \\\n      +\t--ancestry-path A..HEAD -- file\n      +check_result 'P O N R M' --topo-order \\\n     -+\t--include-diversions \\\n     ++\t--show-pulls \\\n      +\t--ancestry-path A..HEAD -- file\n      +check_result 'P O N R M' --topo-order \\\n      +\t--full-history \\\n     @@ t/t6012-rev-list-simplify.sh: test_expect_success '--full-diff is not affected b\n      +\t--simplify-merges \\\n      +\t--ancestry-path A..HEAD -- file\n      +check_result 'N R M' --topo-order \\\n     -+\t--simplify-merges --include-diversions \\\n     ++\t--simplify-merges --show-pulls \\\n      +\t--ancestry-path A..HEAD -- file\n      +\n     -+test_expect_success 'log --graph --simplify-merges --include-diversions' '\n     ++test_expect_success 'log --graph --simplify-merges --show-pulls' '\n      +\tcat >expect <<-\\EOF &&\n      +\t* N\n      +\t*   R\n     @@ t/t6012-rev-list-simplify.sh: test_expect_success '--full-diff is not affected b\n      +\t* I\n      +\tEOF\n      +\tgit log --graph --pretty=\"%s\" \\\n     -+\t\t--simplify-merges --include-diversions \\\n     ++\t\t--simplify-merges --show-pulls \\\n      +\t\t-- file >actual &&\n      +\ttest_cmp expect actual\n      +'\n\n\n Documentation/rev-list-options.txt | 134 ++++++++++++++++++++++++++++-\n object.h                           |   2 +-\n revision.c                         |  27 +++++-\n revision.h                         |   6 +-\n t/t6012-rev-list-simplify.sh       | 120 ++++++++++++++++++++++++++\n 5 files changed, 284 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex bfd02ade991..04ad7dd36eb 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -342,6 +342,12 @@ Default mode::\n \tbranches if the end result is the same (i.e. merging branches\n \twith the same content)\n \n+--show-pulls::\n+\tInclude all commits from the default mode, but also any merge\n+\tcommits that are not TREESAME to the first parent but are\n+\tTREESAME to a later parent. This mode is helpful for showing\n+\tthe merge commits that \"first introduced\" a change to a branch.\n+\n --full-history::\n \tSame as the default mode, but does not prune some history.\n \n@@ -534,7 +540,7 @@ Note the major differences in `N`, `P`, and `Q` over `--full-history`:\n   parent and is TREESAME.\n --\n \n-Finally, there is a fifth simplification mode available:\n+There is another simplification mode available:\n \n --ancestry-path::\n \tLimit the displayed commits to those directly on the ancestry\n@@ -573,6 +579,132 @@ option does. Applied to the 'D..M' range, it results in:\n \t\t\t\tL--M\n -----------------------------------------------------------------------\n \n+Before discussing another option, `--show-pulls`, we need to\n+create a new example history.\n++\n+A common problem users face when looking at simplified history is that a\n+commit they know changed a file somehow does not appear in the file's\n+simplified history. Let's demonstrate a new example and show how options\n+such as `--full-history` and `--simplify-merges` works in that case:\n++\n+-----------------------------------------------------------------------\n+\t  .-A---M-----C--N---O---P\n+\t /     / \\  \\  \\/   /   /\n+\tI     B   \\  R-'`-Z'   /\n+\t \\   /     \\/         /\n+\t  \\ /      /\\        /\n+\t   `---X--'  `---Y--'\n+-----------------------------------------------------------------------\n++\n+For this example, suppose `I` created `file.txt` which was modified by\n+`A`, `B`, and `X` in different ways. The single-parent commits `C`, `Z`,\n+and `Y` do not change `file.txt`. The merge commit `M` was created by\n+resolving the merge conflict to include both changes from `A` and `B`\n+and hence is not TREESAME to either. The merge commit `R`, however, was\n+created by ignoring the contents of `file.txt` at `M` and taking only\n+the contents of `file.txt` at `X`. Hence, `R` is TREESAME to `X` but not\n+`M`. Finally, the natural merge resolution to create `N` is to take the\n+contents of `file.txt` at `R`, so `N` is TREESAME to `R` but not `C`.\n+The merge commits `O` and `P` are TREESAME to their first parents, but\n+not to their second parents, `Z` and `Y` respectively.\n++\n+When using the default mode, `N` and `R` both have a TREESAME parent, so\n+those edges are walked and the others are ignored. The resulting history\n+graph is:\n++\n+-----------------------------------------------------------------------\n+\tI---X\n+-----------------------------------------------------------------------\n++\n+When using `--full-history`, Git walks every edge. This will discover\n+the commits `A` and `B` and the merge `M`, but also will reveal the\n+merge commits `O` and `P`. With parent rewriting, the resulting graph is:\n++\n+-----------------------------------------------------------------------\n+\t  .-A---M--------N---O---P\n+\t /     / \\  \\  \\/   /   /\n+\tI     B   \\  R-'`--'   /\n+\t \\   /     \\/         /\n+\t  \\ /      /\\        /\n+\t   `---X--'  `------'\n+-----------------------------------------------------------------------\n++\n+Here, the merge commits `O` and `P` contribute extra noise, as they did\n+not actually contribute a change to `file.txt`. They only merged a topic\n+that was based on an older version of `file.txt`. This is a common\n+issue in repositories using a workflow where many contributors work in\n+parallel and merge their topic branches along a single trunk: manu\n+unrelated merges appear in the `--full-history` results.\n++\n+When using the `--simplify-merges` option, the commits `O` and `P`\n+disappear from the results. This is because the rewritten second parents\n+of `O` and `P` are reachable from their first parents. Those edges are\n+removed and then the commits look like single-parent commits that are\n+TREESAME to their parent. This also happens to the commit `N`, resulting\n+in a history view as follows:\n++\n+-----------------------------------------------------------------------\n+\t  .-A---M--.\n+\t /     /    \\\n+\tI     B      R\n+\t \\   /      /\n+\t  \\ /      /\n+\t   `---X--'\n+-----------------------------------------------------------------------\n++\n+In this view, we see all of the important single-parent changes from\n+`A`, `B`, and `X`. We also see the carefully-resolved merge `M` and the\n+not-so-carefully-resolved merge `R`. This is usually enough information\n+to determine why the commits `A` and `B` \"disappeared\" from history in\n+the default view. However, there are a few issues with this approach.\n++\n+The first issue is performance. Unlike any previous option, the\n+`--simplify-merges` option requires walking the entire commit history\n+before returning a single result. This can make the option difficult to\n+use for very large repositories.\n++\n+The second issue is one of auditing. When many contributors are working\n+on the same repository, it is important which merge commits introduced\n+a change into an important branch. The problematic merge `R` above is\n+not likely to be the merge commit that was used to merge into an\n+important branch. Instead, the merge `N` was used to merge `R` and `X`\n+into the important branch. This commit may have information about why\n+the change `X` came to override the changes from `A` and `B` in its\n+commit message.\n++\n+The `--show-pulls` option helps with both of these issues by adding more\n+merge commits to the history results. If a merge is not TREESAME to its\n+first parent but is TREESAME to a later parent, then that merge is\n+treated as if it \"pulled\" the change from another branch. When using\n+`--show-pulls` on this example (and no other options) the resulting\n+graph is:\n++\n+-----------------------------------------------------------------------\n+\tI---X---R---N\n+-----------------------------------------------------------------------\n++\n+Here, the merge commits `R` and `N` are included because they pulled\n+the commits `X` and `R` into the base branch, respectively. These\n+merges are the reason the commits `A` and `B` do not appear in the\n+default history.\n++\n+When `--show-pulls` is paired with `--simplify-merges`, the\n+graph includes all of the necessary information:\n++\n+-----------------------------------------------------------------------\n+\t  .-A---M--.   N\n+\t /     /    \\ /\n+\tI     B      R\n+\t \\   /      /\n+\t  \\ /      /\n+\t   `---X--'\n+-----------------------------------------------------------------------\n++\n+Notice that since `M` is reachable from `R`, the edge from `N` to `M`\n+was simplified away. However, `N` still appears in the history as an\n+important commit because it \"pulled\" the change `R` into the main\n+branch.\n+\n The `--simplify-by-decoration` option allows you to view only the\n big picture of the topology of the history, by omitting commits\n that are not referenced by tags.  Commits are marked as !TREESAME\ndiff --git a/object.h b/object.h\nindex 2dbabfca0ab..b22328b8383 100644\n--- a/object.h\n+++ b/object.h\n@@ -59,7 +59,7 @@ struct object_array {\n \n /*\n  * object flag allocation:\n- * revision.h:               0---------10                              25----28\n+ * revision.h:               0---------10         15                   25----28\n  * fetch-pack.c:             01\n  * negotiator/default.c:       2--5\n  * walker.c:                 0-2\ndiff --git a/revision.c b/revision.c\nindex 8136929e236..f89dd6caa55 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -870,7 +870,19 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)\n \t\t\t}\n \t\t\tparent->next = NULL;\n \t\t\tcommit->parents = parent;\n-\t\t\tcommit->object.flags |= TREESAME;\n+\n+\t\t\t/*\n+\t\t\t * A merge commit is a \"diversion\" if it is not\n+\t\t\t * TREESAME to its first parent but is TREESAME\n+\t\t\t * to a later parent. In the simplified history,\n+\t\t\t * we \"divert\" the history walk to the later\n+\t\t\t * parent. These commits are shown when \"show_pulls\"\n+\t\t\t * is enabled, so do not mark the object as\n+\t\t\t * TREESAME here.\n+\t\t\t */\n+\t\t\tif (!revs->show_pulls || !nth_parent)\n+\t\t\t\tcommit->object.flags |= TREESAME;\n+\n \t\t\treturn;\n \n \t\tcase REV_TREE_NEW:\n@@ -897,6 +909,10 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)\n \t\t\t\trelevant_change = 1;\n \t\t\telse\n \t\t\t\tirrelevant_change = 1;\n+\n+\t\t\tif (!nth_parent)\n+\t\t\t\tcommit->object.flags |= PULL_MERGE;\n+\n \t\t\tcontinue;\n \t\t}\n \t\tdie(\"bad tree compare for commit %s\", oid_to_hex(&commit->object.oid));\n@@ -2265,6 +2281,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t} else if (!strcmp(arg, \"--full-diff\")) {\n \t\trevs->diff = 1;\n \t\trevs->full_diff = 1;\n+\t} else if (!strcmp(arg, \"--show-pulls\")) {\n+\t\trevs->show_pulls = 1;\n \t} else if (!strcmp(arg, \"--full-history\")) {\n \t\trevs->simplify_history = 0;\n \t} else if (!strcmp(arg, \"--relative-date\")) {\n@@ -3019,7 +3037,8 @@ static struct commit_list **simplify_one(struct rev_info *revs, struct commit *c\n \tif (!cnt ||\n \t    (commit->object.flags & UNINTERESTING) ||\n \t    !(commit->object.flags & TREESAME) ||\n-\t    (parent = one_relevant_parent(revs, commit->parents)) == NULL)\n+\t    (parent = one_relevant_parent(revs, commit->parents)) == NULL ||\n+\t    (revs->show_pulls && (commit->object.flags & PULL_MERGE)))\n \t\tst->simplified = commit;\n \telse {\n \t\tpst = locate_simplify_state(revs, parent);\n@@ -3602,6 +3621,10 @@ enum commit_action get_commit_action(struct rev_info *revs, struct commit *commi\n \t\t\t/* drop merges unless we want parenthood */\n \t\t\tif (!want_ancestry(revs))\n \t\t\t\treturn commit_ignore;\n+\n+\t\t\tif (revs->show_pulls && (commit->object.flags & PULL_MERGE))\n+\t\t\t\treturn commit_show;\n+\n \t\t\t/*\n \t\t\t * If we want ancestry, then need to keep any merges\n \t\t\t * between relevant commits to tie together topology.\ndiff --git a/revision.h b/revision.h\nindex 475f048fb61..70899eb246c 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -34,6 +34,9 @@\n #define SYMMETRIC_LEFT\t(1u<<8)\n #define PATCHSAME\t(1u<<9)\n #define BOTTOM\t\t(1u<<10)\n+\n+/* WARNING: This is also used as REACHABLE in commit-graph.c. */\n+#define PULL_MERGE\t(1u<<15)\n /*\n  * Indicates object was reached by traversal. i.e. not given by user on\n  * command-line or stdin.\n@@ -43,7 +46,7 @@\n  */\n #define NOT_USER_GIVEN\t(1u<<25)\n #define TRACK_LINEAR\t(1u<<26)\n-#define ALL_REV_FLAGS\t(((1u<<11)-1) | NOT_USER_GIVEN | TRACK_LINEAR)\n+#define ALL_REV_FLAGS\t(((1u<<11)-1) | NOT_USER_GIVEN | TRACK_LINEAR | PULL_MERGE)\n \n #define TOPO_WALK_EXPLORED\t(1u<<27)\n #define TOPO_WALK_INDEGREE\t(1u<<28)\n@@ -129,6 +132,7 @@ struct rev_info {\n \t\t\tno_walk:2,\n \t\t\tremove_empty_trees:1,\n \t\t\tsimplify_history:1,\n+\t\t\tshow_pulls:1,\n \t\t\ttopo_order:1,\n \t\t\tsimplify_merges:1,\n \t\t\tsimplify_by_decoration:1,\ndiff --git a/t/t6012-rev-list-simplify.sh b/t/t6012-rev-list-simplify.sh\nindex a10f0df02b0..b6fa43ace01 100755\n--- a/t/t6012-rev-list-simplify.sh\n+++ b/t/t6012-rev-list-simplify.sh\n@@ -154,4 +154,124 @@ test_expect_success '--full-diff is not affected by --parents' '\n \ttest_cmp expected actual\n '\n \n+#\n+# Create a new history to demonstrate the value of --show-pulls\n+# with respect to the subtleties of simplified history, --full-history,\n+# and --simplify-merges.\n+#\n+#   .-A---M-----C--N---O---P\n+#  /     / \\  \\  \\/   /   /\n+# I     B   \\  R-'`-Z'   /\n+#  \\   /     \\/         /\n+#   \\ /      /\\        /\n+#    `---X--'  `---Y--'\n+#\n+# This example is explained in Documentation/rev-list-options.txt\n+\n+test_expect_success 'rebuild repo' '\n+\trm -rf .git * &&\n+\tgit init &&\n+\tgit switch -c main &&\n+\n+\techo base >file &&\n+\tgit add file &&\n+\ttest_commit I &&\n+\n+\techo A >file &&\n+\tgit add file &&\n+\ttest_commit A &&\n+\n+\tgit switch -c branchB I &&\n+\techo B >file &&\n+\tgit add file &&\n+\ttest_commit B &&\n+\n+\tgit switch main &&\n+\ttest_must_fail git merge -m \"M\" B &&\n+\techo A >file &&\n+\techo B >>file &&\n+\tgit add file &&\n+\tgit merge --continue &&\n+\tnote M &&\n+\n+\techo C >other &&\n+\tgit add other &&\n+\ttest_commit C &&\n+\n+\tgit switch -c branchX I &&\n+\techo X >file &&\n+\tgit add file &&\n+\ttest_commit X &&\n+\n+\tgit switch -c branchR M &&\n+\tgit merge -m R -Xtheirs X &&\n+\tnote R &&\n+\n+\tgit switch main &&\n+\tgit merge -m N R &&\n+\tnote N &&\n+\n+\tgit switch -c branchY M &&\n+\techo Y >y &&\n+\tgit add y &&\n+\ttest_commit Y &&\n+\n+\tgit switch -c branchZ C &&\n+\techo Z >z &&\n+\tgit add z &&\n+\ttest_commit Z &&\n+\n+\tgit switch main &&\n+\tgit merge -m O Z &&\n+\tnote O &&\n+\n+\tgit merge -m P Y &&\n+\tnote P\n+'\n+\n+check_result 'X I' -- file\n+check_result 'N R X I' --show-pulls -- file\n+\n+check_result 'P O N R X M B A I' --full-history --topo-order -- file\n+check_result 'N R X M B A I' --simplify-merges --topo-order --show-pulls -- file\n+check_result 'R X M B A I' --simplify-merges --topo-order -- file\n+check_result 'N M A I' --first-parent -- file\n+check_result 'N M A I' --first-parent --show-pulls -- file\n+\n+# --ancestry-path implies --full-history\n+check_result 'P O N R M' --topo-order \\\n+\t--ancestry-path A..HEAD -- file\n+check_result 'P O N R M' --topo-order \\\n+\t--show-pulls \\\n+\t--ancestry-path A..HEAD -- file\n+check_result 'P O N R M' --topo-order \\\n+\t--full-history \\\n+\t--ancestry-path A..HEAD -- file\n+check_result 'R M' --topo-order \\\n+\t--simplify-merges \\\n+\t--ancestry-path A..HEAD -- file\n+check_result 'N R M' --topo-order \\\n+\t--simplify-merges --show-pulls \\\n+\t--ancestry-path A..HEAD -- file\n+\n+test_expect_success 'log --graph --simplify-merges --show-pulls' '\n+\tcat >expect <<-\\EOF &&\n+\t* N\n+\t*   R\n+\t|\\  \n+\t| * X\n+\t* |   M\n+\t|\\ \\  \n+\t| * | B\n+\t| |/  \n+\t* / A\n+\t|/  \n+\t* I\n+\tEOF\n+\tgit log --graph --pretty=\"%s\" \\\n+\t\t--simplify-merges --show-pulls \\\n+\t\t-- file >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n\nbase-commit: 274b9cc25322d9ee79aa8e6d4e86f0ffe5ced925\n-- \ngitgitgadget\n"},{"id":"395238","messageId":"xmqq369bjebo.fsf@gitster.c.googlers.com","threadId":"53185","inReplyTo":"pull.599.v3.git.1586521183873.gitgitgadget@gmail.com","subject":"Re: [PATCH v3] revision: --show-pulls adds helpful merges","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-10T20:06:19Z","receivedAt":"2020-04-10T20:06:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Derrick Stolee via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n>   A----C1----C2--- ... ---Cn----M------P1---P2--- ... ---Pn\n>    \\     \\     \\            \\  /      /    /            /\n>     \\     \\__.. \\            \\/ ..__T1    /           Tn\n>      \\           \\__..       /\\     ..__T2           /\n>       \\_____________________B  \\____________________/\n>\n> If the commits T1, T2, ... Tn did not change the file, then all of\n> P1 through Pn will be TREESAME to their first parent, but not\n> TREESAME to their second. This means that all of those merge commits\n> appear in the --full-history view, with edges that immediately\n> collapse into the lower history without introducing interesting\n> single-parent commits.\n>\n> The --simplify-merges option was introduced to remove these extra\n> merge commits. By noticing that the rewritten parents are reachable\n> from their first parents, those edges can be simplified away. Finally,\n> the commits now look like single-parent commits that are TREESAME to\n> their \"only\" parent. Thus, they are removed and this issue does not\n> cause issues anymore. However, this also ends up removing the commit\n> M from the history view! Even worse, the --simplify-merges option\n> requires walking the entire history before returning a single result.\n\nTrue.  It is not advisable to use --simplify-merges unless you are\nlimiting the history at the bottom for that reason.\n\n> In some sense, users are asking for the \"first\" merge commit to\n> bring in the change to their branch. As long as the parent order is\n> consistent, this can be handled with the following rule:\n>\n>   Include a merge commit if it is not TREESAME to its first\n>   parent, but is TREESAME to a later parent.\n\n\"but is\" -> \"even if it is\" would make it a bit more accurate, I\nwould think.  Normally, such a merge that is treesame to some side\nbranch is omitted from the output, but the rule wants it to be shown\neven if all the changes were brought in from one single parent.\n\n> Update Documentation/rev-list-options.txt with significant details\n> around this option. This requires updating the example in the\n> History Simplification section to demonstrate some of the problems\n> with TREESAME second parents.\n\nGood.\n\n> diff --git a/revision.c b/revision.c\n> index 8136929e236..f89dd6caa55 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -870,7 +870,19 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)\n>  \t\t\t}\n>  \t\t\tparent->next = NULL;\n>  \t\t\tcommit->parents = parent;\n> -\t\t\tcommit->object.flags |= TREESAME;\n> +\n> +\t\t\t/*\n> +\t\t\t * A merge commit is a \"diversion\" if it is not\n> +\t\t\t * TREESAME to its first parent but is TREESAME\n> +\t\t\t * to a later parent. In the simplified history,\n> +\t\t\t * we \"divert\" the history walk to the later\n> +\t\t\t * parent. These commits are shown when \"show_pulls\"\n> +\t\t\t * is enabled, so do not mark the object as\n> +\t\t\t * TREESAME here.\n\nAs we no longer use the word \"diversion\", this explanation should\nshift the focus from defining the word \"diversion\" and giving its\nbackground to explaining why the above parent rewriting is done and\nwhy the TREESAME marking is conditional, e.g.\n\n      \t\t\tThe tree of the merge and of the parent are\n      \t\t\tthe same; from here on, we stop following\n      \t\t\thistories of all other parents but this one\n      \t\t\tby culling commit->parents list.  We also\n      \t\t\tnormally mark the merge commit TREESAME as\n      \t\t\tthe merge itself did not introduce any\n      \t\t\tchange relative to the parent, but we\n      \t\t\trefrain from doing so for the first parent\n      \t\t\tunder \"--show-pulls\" mode, in order to let\n      \t\t\tthe output phase to show the merge, which is\n      \t\t\tthe last commit before we divert our walk to\n      \t\t\ta side history.\n\nor something along that line.\n\n> +\t\t\tif (!revs->show_pulls || !nth_parent)\n> +\t\t\t\tcommit->object.flags |= TREESAME;\n> +\n>  \t\t\treturn;\n\n> @@ -897,6 +909,10 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)\n>  \t\t\t\trelevant_change = 1;\n>  \t\t\telse\n>  \t\t\t\tirrelevant_change = 1;\n> +\n> +\t\t\tif (!nth_parent)\n> +\t\t\t\tcommit->object.flags |= PULL_MERGE;\n\nFor a three-parent merge that brings in changes to the first parent,\nif the result matches the second parent, we would have returned in\nthe previous hunk before having a chance to inspect the third one\nand marking the merge result with PULL_MERGE, but if you swap the\norder of the second and the third parent, the second parent, which\nhas different tree from the result, would not return in the previous\nhunk and cause the merge with PULL_MERGE here.  And then when we\ninspect the third parent, the previous hunk's return would kick in.\nSo for two merges that merge exactly the same two branches on top of\nexactly the same commit on the mainline, you sometimes mark the\nresult with PULL_MERGE and sometimes don't, depending on the order\nof the second and the third parent.\n\nThat feels iffy.  Treating the first parent differently from others\nis what we intend to do with this change, bhut this hunk treats the\nother parents differently depending on the merge order.\n\n> @@ -3019,7 +3037,8 @@ static struct commit_list **simplify_one(struct rev_info *revs, struct commit *c\n>  \tif (!cnt ||\n>  \t    (commit->object.flags & UNINTERESTING) ||\n>  \t    !(commit->object.flags & TREESAME) ||\n> -\t    (parent = one_relevant_parent(revs, commit->parents)) == NULL)\n> +\t    (parent = one_relevant_parent(revs, commit->parents)) == NULL ||\n> +\t    (revs->show_pulls && (commit->object.flags & PULL_MERGE)))\n>  \t\tst->simplified = commit;\n\n... hence, wouldn't we see different result here ...\n\n> @@ -3602,6 +3621,10 @@ enum commit_action get_commit_action(struct rev_info *revs, struct commit *commi\n>  \t\t\t/* drop merges unless we want parenthood */\n>  \t\t\tif (!want_ancestry(revs))\n>  \t\t\t\treturn commit_ignore;\n> +\n> +\t\t\tif (revs->show_pulls && (commit->object.flags & PULL_MERGE))\n> +\t\t\t\treturn commit_show;\n\n... and also here?\n\nThanks.\n"},{"id":"395248","messageId":"28075cfb-34bb-1109-5685-8d662e7559c3@gmail.com","threadId":"53185","inReplyTo":"xmqq369bjebo.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v3] revision: --show-pulls adds helpful merges","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-04-10T21:43:05Z","receivedAt":"2020-04-10T21:43:12Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 4/10/2020 4:06 PM, Junio C Hamano wrote:\n> \"Derrick Stolee via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n> \n>>   A----C1----C2--- ... ---Cn----M------P1---P2--- ... ---Pn\n>>    \\     \\     \\            \\  /      /    /            /\n>>     \\     \\__.. \\            \\/ ..__T1    /           Tn\n>>      \\           \\__..       /\\     ..__T2           /\n>>       \\_____________________B  \\____________________/\n>>\n>> If the commits T1, T2, ... Tn did not change the file, then all of\n>> P1 through Pn will be TREESAME to their first parent, but not\n>> TREESAME to their second. This means that all of those merge commits\n>> appear in the --full-history view, with edges that immediately\n>> collapse into the lower history without introducing interesting\n>> single-parent commits.\n>>\n>> The --simplify-merges option was introduced to remove these extra\n>> merge commits. By noticing that the rewritten parents are reachable\n>> from their first parents, those edges can be simplified away. Finally,\n>> the commits now look like single-parent commits that are TREESAME to\n>> their \"only\" parent. Thus, they are removed and this issue does not\n>> cause issues anymore. However, this also ends up removing the commit\n>> M from the history view! Even worse, the --simplify-merges option\n>> requires walking the entire history before returning a single result.\n> \n> True.  It is not advisable to use --simplify-merges unless you are\n> limiting the history at the bottom for that reason.\n\nI will modify my advice to include this \"limiting the history at the\nbottom\" advice.\n\n>> In some sense, users are asking for the \"first\" merge commit to\n>> bring in the change to their branch. As long as the parent order is\n>> consistent, this can be handled with the following rule:\n>>\n>>   Include a merge commit if it is not TREESAME to its first\n>>   parent, but is TREESAME to a later parent.\n> \n> \"but is\" -> \"even if it is\" would make it a bit more accurate, I\n> would think.  Normally, such a merge that is treesame to some side\n> branch is omitted from the output, but the rule wants it to be shown\n> even if all the changes were brought in from one single parent.\n\nThis is an excellent clarification, since the way I wrote it does not\ndescribe every merge that we include. I wrote it to include the \"extra\"\nmerges that are included. A better strategy is to include a complete\ndescription of every merge commit that appears in the output, as you\ndefined it.\n\n>> Update Documentation/rev-list-options.txt with significant details\n>> around this option. This requires updating the example in the\n>> History Simplification section to demonstrate some of the problems\n>> with TREESAME second parents.\n> \n> Good.\n> \n>> diff --git a/revision.c b/revision.c\n>> index 8136929e236..f89dd6caa55 100644\n>> --- a/revision.c\n>> +++ b/revision.c\n>> @@ -870,7 +870,19 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)\n>>  \t\t\t}\n>>  \t\t\tparent->next = NULL;\n>>  \t\t\tcommit->parents = parent;\n>> -\t\t\tcommit->object.flags |= TREESAME;\n>> +\n>> +\t\t\t/*\n>> +\t\t\t * A merge commit is a \"diversion\" if it is not\n>> +\t\t\t * TREESAME to its first parent but is TREESAME\n>> +\t\t\t * to a later parent. In the simplified history,\n>> +\t\t\t * we \"divert\" the history walk to the later\n>> +\t\t\t * parent. These commits are shown when \"show_pulls\"\n>> +\t\t\t * is enabled, so do not mark the object as\n>> +\t\t\t * TREESAME here.\n> \n> As we no longer use the word \"diversion\", this explanation should\n> shift the focus from defining the word \"diversion\" and giving its\n> background to explaining why the above parent rewriting is done and\n> why the TREESAME marking is conditional, e.g.\n\nSorry about this. I intended to replace all references to this old\nterminology, but failed.\n\n>       \t\t\tThe tree of the merge and of the parent are\n>       \t\t\tthe same; from here on, we stop following\n>       \t\t\thistories of all other parents but this one\n>       \t\t\tby culling commit->parents list.  We also\n>       \t\t\tnormally mark the merge commit TREESAME as\n>       \t\t\tthe merge itself did not introduce any\n>       \t\t\tchange relative to the parent, but we\n>       \t\t\trefrain from doing so for the first parent\n>       \t\t\tunder \"--show-pulls\" mode, in order to let\n>       \t\t\tthe output phase to show the merge, which is\n>       \t\t\tthe last commit before we divert our walk to\n>       \t\t\ta side history.\n> \n> or something along that line.\n\nThis is pretty good. I plan to take it with very minor modifications in my\nnext version.\n\n> \n>> +\t\t\tif (!revs->show_pulls || !nth_parent)\n>> +\t\t\t\tcommit->object.flags |= TREESAME;\n>> +\n>>  \t\t\treturn;\n> \n>> @@ -897,6 +909,10 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)\n>>  \t\t\t\trelevant_change = 1;\n>>  \t\t\telse\n>>  \t\t\t\tirrelevant_change = 1;\n>> +\n>> +\t\t\tif (!nth_parent)\n>> +\t\t\t\tcommit->object.flags |= PULL_MERGE;\n> \n> For a three-parent merge that brings in changes to the first parent,\n> if the result matches the second parent, we would have returned in\n> the previous hunk before having a chance to inspect the third one\n> and marking the merge result with PULL_MERGE, but if you swap the\n> order of the second and the third parent, the second parent, which\n> has different tree from the result, would not return in the previous\n> hunk and cause the merge with PULL_MERGE here.  And then when we\n> inspect the third parent, the previous hunk's return would kick in.\n> So for two merges that merge exactly the same two branches on top of\n> exactly the same commit on the mainline, you sometimes mark the\n> result with PULL_MERGE and sometimes don't, depending on the order\n> of the second and the third parent.\n> \n> That feels iffy.  Treating the first parent differently from others\n> is what we intend to do with this change, bhut this hunk treats the\n> other parents differently depending on the merge order.\n\nIt is worth adding a test with an octopus merge to really be clear\nabout the intended behavior. When describing the concept, I've been\ncareful to say \"TREESAME to the first parent\" and \"not TREESAME to\na later parent\" instead of assuming two parents. In the default\nmode, we should not reach here unless we found a TREESAME parent\nthat was not the first parent, since we stop visiting parents after\nwe find any TREESAME parent. But this would change for --full-history\nor --simplify-merges.\n\n>> @@ -3019,7 +3037,8 @@ static struct commit_list **simplify_one(struct rev_info *revs, struct commit *c\n>>  \tif (!cnt ||\n>>  \t    (commit->object.flags & UNINTERESTING) ||\n>>  \t    !(commit->object.flags & TREESAME) ||\n>> -\t    (parent = one_relevant_parent(revs, commit->parents)) == NULL)\n>> +\t    (parent = one_relevant_parent(revs, commit->parents)) == NULL ||\n>> +\t    (revs->show_pulls && (commit->object.flags & PULL_MERGE)))\n>>  \t\tst->simplified = commit;\n> \n> ... hence, wouldn't we see different result here ...\n> \n>> @@ -3602,6 +3621,10 @@ enum commit_action get_commit_action(struct rev_info *revs, struct commit *commi\n>>  \t\t\t/* drop merges unless we want parenthood */\n>>  \t\t\tif (!want_ancestry(revs))\n>>  \t\t\t\treturn commit_ignore;\n>> +\n>> +\t\t\tif (revs->show_pulls && (commit->object.flags & PULL_MERGE))\n>> +\t\t\t\treturn commit_show;\n> \n> ... and also here?\n\nIf there is a flaw in how I set PULL_MERGE, then hopefully the fix is where\nI assign the PULL_MERGE flag and these lines do not need to change.\n\nThanks,\n-Stolee\n"}]}