git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: "brian m. carlson" <sandals@crustytoothpaste.net>
To: Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, peff@peff.net, me@ttaylorr.com,
	Derrick Stolee <dstolee@microsoft.com>
Subject: Re: [PATCH] revision: --include-diversions adds helpful merges
Date: Wed, 8 Apr 2020 02:13:30 +0000	[thread overview]
Message-ID: <20200408021330.GB6549@camp.crustytoothpaste.net> (raw)
In-Reply-To: <pull.599.git.1586308923544.gitgitgadget@gmail.com>

[-- 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 --]

  parent reply	other threads:[~2020-04-08  2:13 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-08  1:22 Derrick Stolee via GitGitGadget
2020-04-08  1:30 ` Junio C Hamano
2020-04-08  1:39   ` Derrick Stolee
2020-04-08 15:28     ` Derrick Stolee
2020-04-08 19:46       ` Junio C Hamano
2020-04-08 20:05         ` Jeff King
2020-04-08 20:22           ` Derrick Stolee
2020-04-08 21:35             ` Junio C Hamano
2020-04-08 23:59               ` Derrick Stolee
2020-04-09  0:08                 ` Junio C Hamano
2020-04-09 11:52                   ` Derrick Stolee
2020-04-09 14:28                   ` Philip Oakley
2020-04-09 15:56                     ` Junio C Hamano
2020-04-09 17:20                       ` Derrick Stolee
2020-04-09 18:24                         ` Jeff King
2020-04-09 18:55                           ` Junio C Hamano
2020-04-09 19:21                             ` Jeff King
2020-04-08  2:13 ` brian m. carlson [this message]
2020-04-08 18:48 ` Jeff King
2020-04-09  0:01 ` [PATCH v2] " Derrick Stolee via GitGitGadget
2020-04-10 12:19   ` [PATCH v3] revision: --show-pulls " Derrick Stolee via GitGitGadget
2020-04-10 20:06     ` Junio C Hamano
2020-04-10 21:43       ` Derrick Stolee

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200408021330.GB6549@camp.crustytoothpaste.net \
    --to=sandals@crustytoothpaste.net \
    --cc=dstolee@microsoft.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=me@ttaylorr.com \
    --cc=peff@peff.net \
    --subject='Re: [PATCH] revision: --include-diversions adds helpful merges' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).