All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org,  Johannes Sixt <j6t@kdbg.org>,
	 Elijah Newren <newren@gmail.com>,
	 Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: [PATCH v3] log: --remerge-diff needs to keep around commit parents
Date: Fri, 13 Dec 2024 07:02:59 -0800	[thread overview]
Message-ID: <xmqq5xnnbp7g.fsf@gitster.g> (raw)
In-Reply-To: <pull.1825.v3.git.1733999352289.gitgitgadget@gmail.com> (Johannes Schindelin via GitGitGadget's message of "Thu, 12 Dec 2024 10:29:12 +0000")

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> This problem is most obvious when traversing the commits in reverse: In
> that instance, if a parent of a merge commit has been shown as part of
> the `git log` command, by the time the merge commit's diff needs to be
> computed, that parent commit's list of parent commits will have been set
> to `NULL` and as a result no merge base will be found (even if one
> should be found).
>
> Traversing commits in reverse is far from the only circumstance in which
> this problem occurs, though. There are many avenues to traversing at
> least one commit in the revision walk that will later be part of a merge
> base computation, for example when not even walking any revisions in
> `git show <merge1> <merge2>` where `<merge1>` is part of the commit
> graph between the parents of `<merge2>`.
>
> Another way to force a scenario where a commit is traversed before it
> has to be traversed again as part of a merge base computation is to
> start with two revisions (where the first one is reachable from the
> second but not in a first-parent ancestry) and show the commit log with
> `--topo-order` and `--first-parent`.
>
> Let's fix this by special-casing the `remerge_diff` mode, similar to
> what we did with reflogs in f35650dff6a4 (log: do not free parents when
> walking reflog, 2017-07-07).

Nicely described.

>     This fixes a bug that is pretty much as old as the remerge-diff
>     machinery itself. I noticed it while writing my reply to Hannes Sixt's
>     concerns about my range-diff --diff-merges=<mode> patch

> diff --git a/t/t4069-remerge-diff.sh b/t/t4069-remerge-diff.sh
> index 07323ebafe0..a68c6bfa036 100755
> --- a/t/t4069-remerge-diff.sh
> +++ b/t/t4069-remerge-diff.sh
> @@ -317,4 +317,11 @@ test_expect_success 'remerge-diff turns off history simplification' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_success 'remerge-diff with --reverse' '
> +	git log -1 --remerge-diff --oneline ab_resolution^ >expect &&
> +	git log -1 --remerge-diff --oneline ab_resolution >>expect &&
> +	git log -2 --remerge-diff --oneline ab_resolution --reverse >actual &&
> +	test_cmp expect actual
> +'
> +

Will queue.  Let me mark it for 'next'.

Thanks.


      reply	other threads:[~2024-12-13 15:03 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-08 13:43 [PATCH] log: --remerge-diff needs to keep around commit parents Johannes Schindelin via GitGitGadget
2024-11-08 15:47 ` Elijah Newren
2024-11-11  1:52   ` Junio C Hamano
2024-11-11  1:46 ` Junio C Hamano
2024-11-11 18:32   ` Johannes Schindelin
2024-11-11 22:13     ` Junio C Hamano
2024-11-11 18:33 ` [PATCH v2] " Johannes Schindelin via GitGitGadget
2024-12-12 10:29   ` [PATCH v3] " Johannes Schindelin via GitGitGadget
2024-12-13 15:02     ` Junio C Hamano [this message]

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

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

  git send-email \
    --in-reply-to=xmqq5xnnbp7g.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=j6t@kdbg.org \
    --cc=johannes.schindelin@gmx.de \
    --cc=newren@gmail.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.