git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: git@vger.kernel.org
Subject: Re: Reviewing merge commits, was Re: [rPATCH 13/12] Merge branch 'jc/fix-aggressive-protection-2.39'
Date: Thu, 23 May 2024 07:41:20 -0700	[thread overview]
Message-ID: <xmqqfru8y4j3.fsf@gitster.g> (raw)
In-Reply-To: <79a2c761-870a-cc0c-ce46-440af3bad152@gmx.de> (Johannes Schindelin's message of "Thu, 23 May 2024 12:36:00 +0200 (CEST)")

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Much of what is tricky about these merges
> happens outside conflict markers.

Yes.  In other words, what you do not see in these outputs can be
leaving semantic conflicts unresolved.  If we made a change to a
location where there wasn't a textual conflict (i.e., making an evil
merge) in order to adjust for a semantic differences (e.g., they
added a callsite to a function whose signature we updated), any of
"git show --cc", "git range-diff", and "git show --remerge-diff"
would show.  If you failed to do so and introduced a bug (and worse
yet, unlike "oh, we now require one more argument to the function"
that compilers would catch for us, there are differences that
compilers would not notice), none of the three will show.

> If it was up to me to verify such fixes, short of using Git and validating
> the correctness by performing the merge independently instead of trying to
> accomplish the validation by looking at a plain-text mail, I would compare
> the diff of `maint-2.39` to the diff of `maint-2.40`. Something like this
> [*1*]:
> ...
> Both the remerge-diff and the range-diff output do nothing, though, to
> help verifying that no-longer-needed `#include`s are removed (you can see
> that `#include "copy.h"` was removed from `hook.c`, but if that had been
> missed there would be no indicator thereof).

In short, you are agreeing that between "remerge-diff" and "range-diff"
there is no difference in power to let you notice what is *not* there,
and I am agreeing to your "what is *not* there is often more problematic",
so we are on the same page.

I do not think I am married to "remerge-diff" output, but it being
the same form of the familiar "single-parent" patch, I personally
find it far easier to read than "diff of diff".  We may differ at
that point, but it does not matter, as neither of our preferred
format is adequate for the job.

;-)


  reply	other threads:[~2024-05-23 14:41 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-21 19:56 [PATCH 00/12] Fix various overly aggressive protections in 2.45.1 and friends Junio C Hamano
2024-05-21 19:56 ` [PATCH 01/12] send-email: drop FakeTerm hack Junio C Hamano
2024-05-22  8:19   ` Dragan Simic
2024-05-21 19:56 ` [PATCH 02/12] send-email: avoid creating more than one Term::ReadLine object Junio C Hamano
2024-05-22  8:15   ` Dragan Simic
2024-05-21 19:56 ` [PATCH 03/12] ci: drop mention of BREW_INSTALL_PACKAGES variable Junio C Hamano
2024-05-21 19:56 ` [PATCH 04/12] ci: avoid bare "gcc" for osx-gcc job Junio C Hamano
2024-05-21 19:56 ` [PATCH 05/12] ci: stop installing "gcc-13" for osx-gcc Junio C Hamano
2024-05-21 19:56 ` [PATCH 06/12] hook: plug a new memory leak Junio C Hamano
2024-05-21 19:56 ` [PATCH 07/12] init: use the correct path of the templates directory again Junio C Hamano
2024-05-21 19:56 ` [PATCH 08/12] Revert "core.hooksPath: add some protection while cloning" Junio C Hamano
2024-05-21 19:56 ` [PATCH 09/12] tests: verify that `clone -c core.hooksPath=/dev/null` works again Junio C Hamano
2024-05-21 22:57   ` Brooke Kuhlmann
2024-05-21 19:56 ` [PATCH 10/12] clone: drop the protections where hooks aren't run Junio C Hamano
2024-05-21 19:56 ` [PATCH 11/12] Revert "Add a helper function to compare file contents" Junio C Hamano
2024-05-21 19:56 ` [PATCH 12/12] Revert "fetch/clone: detect dubious ownership of local repositories" Junio C Hamano
2024-05-21 20:43   ` Junio C Hamano
2024-05-22  7:27     ` Johannes Schindelin
2024-05-22 17:20       ` Junio C Hamano
2024-05-21 20:45 ` [rPATCH 13/12] Merge branch 'jc/fix-aggressive-protection-2.39' Junio C Hamano
2024-05-23 10:36   ` Reviewing merge commits, was " Johannes Schindelin
2024-05-23 14:41     ` Junio C Hamano [this message]
2024-05-21 20:45 ` [rPATCH 14/12] Merge branch 'jc/fix-aggressive-protection-2.40' Junio C Hamano
2024-05-21 21:33   ` Junio C Hamano
2024-05-21 21:14 ` [PATCH 00/12] Fix various overly aggressive protections in 2.45.1 and friends Johannes Schindelin
2024-05-21 21:46   ` Junio C Hamano
2024-05-21 22:13     ` Junio C Hamano
2024-05-22 10:01 ` Joey Hess
2024-05-23  5:49   ` Junio C Hamano
2024-05-23 16:31     ` Joey Hess
2024-05-27 19:51       ` Johannes Schindelin
2024-05-28  2:25         ` Joey Hess
2024-05-28 15:02         ` Phillip Wood
2024-05-28 16:13           ` Junio C Hamano
2024-05-28 17:47           ` Junio C Hamano
2024-05-23 23:32     ` Junio C Hamano

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=xmqqfru8y4j3.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    /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 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 NNTP newsgroup(s).