Re: [PATCH] revision: --include-diversions adds helpful merges

["brian m. carlson" <sandals@crustytoothpaste.net>]

[-- Attachment #1: Type: text/plain, Size: 4814 bytes --]

On 2020-04-08 at 01:22:03, Derrick Stolee via GitGitGadget wrote:
> The --simplify-merges option was introduced to remove these extra
> merge commits. By noticing that the rewritten parents are reachable
> from their first parents, those edges can be simplified away. Finally,
> the commits now look like single-parent commits that are TREESAME to
> their "only" parent. Thus, they are removed and this issue does not
> cause issues anymore. However, this also ends up removing the commit
> M from the history view! Even worse, the --simplify-merges option
> requires walking the entire history before returning a single result.

I was not aware --simplify-merges required walking the entire history.
Now I know why my alias using it performs so poorly on $HUGE_REPOSITORY
with thousands of extra backmerges at $DAYJOB.

Thanks for teaching me something.

> Many Git users are using Git alongside a Git service that provides
> code storage alongside a code review tool commonly called "Pull
> Requests" or "Merge Requests" against a target branch.  When these
> requests are accepted and merged, they typically create a merge
> commit whose first parent is the previous branch tip and the second
> parent is the tip of the topic branch used for the request. This
> presents a valuable order to the parents, but also makes that merge
> commit slightly special. Users may want to see not only which
> commits changed a file, but which pull requests merged those commits
> into their branch. In the previous example, this would mean the
> users want to see the merge commit "M" in addition to the single-
> parent commit "C".

I should not hesitate to point out that this history is also true of the
Git Project's repository, although of course the merges may be of less
interest here.

> In some sense, users are asking for the "first" merge commit to
> bring in the change to their branch. As long as the parent order is
> consistent, this can be handled with the following rule:
> 
>   Include a merge commit if it is not TREESAME to its first
>   parent, but is TREESAME to a later parent.
> 
> I call such merge commits "diversions" because they divert the
> history walk away from the first-parent history. As such, this
> change adds the "--include-diversions" option to rev-list and log.
> To test these options, extend the standard test example to include
> a merge commit that is not TREESAME to its first parent. It is
> surprising that that option was not already in the example, as it
> is instructive.
> 
> In particular, this extension demonstrates a common issue with file
> history simplification. When a user resolves a merge conflict using
> "-Xours" or otherwise ignoring one side of the conflict, they create
> a TREESAME edge that probably should not be TREESAME. This leads
> users to become frustrated and complain that "my change disappeared!"
> In my experience, showing them history with --full-history and
> --simplify-merges quickly reveals the problematic merge. As mentioned,
> this option is expensive to compute. The --include-diversions option
> _might_ show the merge commit (usually titled "resolving conflicts")
> more quickly. Of course, this depends on the user having the correct
> parent order, which is backwards when using "git pull".

I can't comment on the contents of the patch, since I'm really not at
all familiar with the revision machinery, but I do think this change is a
good idea.  I see this as a very common use case, and I think this
commit message explains the rationale well.

> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
>     Add a new history mode
> 
>     This --include-diversions option could use a better name.

As we all know, I'm terrible at naming things, so I have no suggestions
here.  I'm happy with the name as it stands, but am of course open to
other ideas.

> diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
> index bfd02ade991..0c878be94a9 100644
> --- a/Documentation/rev-list-options.txt
> +++ b/Documentation/rev-list-options.txt
> @@ -342,6 +342,12 @@ Default mode::
>  	branches if the end result is the same (i.e. merging branches
>  	with the same content)
> 
> +--include-diversions::
> +	Include all commits from the default mode, but also any merge
> +	commits that are not TREESAME to the first parent but are
> +	TREESAME to a later parent. This mode is helpful for showing
> +	the merge commits that "first introduced" a change to a branch.

I wasn't sure if this use of "TREESAME" would be confusing, but it looks
like we already use it extensively throughout the documentation, so it's
probably fine.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]