public inbox for git@vger.kernel.org
 help / color / mirror / Atom feed
From: Magnus Holmgren <magnus.holmgren@milientsoftware.com>
To: git@vger.kernel.org, Bagas Sanjaya <bagasdotme@gmail.com>
Cc: "Santi Béjar" <santi@agolina.net>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Sergey Organov" <sorganov@gmail.com>,
	"Christian Couder" <christian.couder@gmail.com>,
	"Jeff King" <peff@peff.net>, "René Scharfe" <l.s.r@web.de>
Subject: Re: Please explain avoiding history simplifications when diffing merges
Date: Mon, 25 Sep 2023 18:11:26 +0200	[thread overview]
Message-ID: <6191124.FXmgHNBoPt@utklippan> (raw)
In-Reply-To: <ZQbNtgd82iARQ39D@debian.me>

söndag 17 september 2023 11:58:14 CEST skrev  Bagas Sanjaya:
> On Fri, Sep 15, 2023 at 05:10:28PM +0200, Magnus Holmgren wrote:
> > Friday, 8 September 2023 11:09:20 CEST, I wrote
> > 
> > > QGit was bitten by
> > > https://github.com/git/git/commit/0dec322d31db3920872f43bdd2a7ddd282a5be
> > > 67
> > 
> > Maybe I should link to the QGit issue:
> > https://github.com/tibirna/qgit/issues/129
> > 
> > > It looks like passing --simplify-merges to override the default solves
> > > the
> > > problem, but I still want to ask here because I'm not sure I fully
> > > understand
> > > 
> > > the reasoning:
> > > > the default history simplification would remove merge commits from
> > > > consideration if the file "path" matched the second parent.
> > 
> > As I wrote at the above URL, I realized that the old git log output
> > without -- simplify-merges and the output with --simplify-merges aren't
> > quite the same. The old output indeed omits some interesting merge
> > commits, which may explain why the change was made, but git log
> > --simplify-merges does include them, so it seems a reasonable default to
> > me.
> 
> Can you provide examples?

git init git-test; cd git-test
echo foo > test; git add test; git commit -m Initial
git branch testbranch
git branch irrelevantbranch
echo foobar > test; git commit -am Change
git switch testbranch
echo barfoo > test; git commit -am Diverge
git switch irrelevantbranch
echo whatever > otherfile; git add otherfile
git commit -m "Add other file"
git switch master
git merge -m "Merge branch 'testbranch'" -s ours testbranch
git merge -m "Merge branch 'irrelevantbranch' irrelevantbranch
git log test  # old git doesn't show either merge commit, new git shows both
git log --simplify-merges test  # both versions show the first merge commit
git log --diff-merges=separate -p --simplify-merges test # only in new git
# note that only one diff is generated because the other would be empty,
# although the man page says "Separate log entry and diff is generated for
# each parent."

> > However, QGit has a problem: git log --diff-merges=separate includes a
> > separate diff for each parent, but only for each parent with differences
> > compared to the merge commit, *and* there's no custom format placeholder
> > for the current parent, only for the list of parents (%P/%p). How should
> > one go about adding that? I figure the format_commit_context struct in
> > pretty.c needs another field.
> 
> What are you trying to accomplish with your proposed formatting verbs?

QGit builds annotated file histories by parsing git log output. To accomplish 
that, it needs to use a custom, more machine-readable format, and to handle 
merge commits correctly, it either needs one diff per parent in the order the 
parents are listed, or it needs to know which parent each diff is relative to.

Because old git log (without --simplify-merges) only included merge commits 
where the file in question is different to all parents, it didn't get confused 
by missing diffs - it was all or nothing - but some merge commits were missing 
in the file history view.

QGit should be rewritten to use libgit2, but regardless, I think all the 
information you can get with a standard format should be available to custom 
formats as well.

-- 
Magnus Holmgren
./¯\_/¯\. Milient




      reply	other threads:[~2023-09-25 16:11 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-08  9:09 Please explain avoiding history simplifications when diffing merges Magnus Holmgren
2023-09-15 15:10 ` Magnus Holmgren
2023-09-17  9:58   ` Bagas Sanjaya
2023-09-25 16:11     ` Magnus Holmgren [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=6191124.FXmgHNBoPt@utklippan \
    --to=magnus.holmgren@milientsoftware.com \
    --cc=bagasdotme@gmail.com \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=l.s.r@web.de \
    --cc=peff@peff.net \
    --cc=santi@agolina.net \
    --cc=sorganov@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox