list mirror (unofficial, one of many)
 help / color / mirror / Atom feed
From: Arnaud Morin <>
To: Junio C Hamano <>
Cc: Jeff King <>,, Kevin Willford <>
Subject: Re: [PATCH] patch-ids: handle duplicate hashmap entries
Date: Wed, 13 Jan 2021 09:24:48 +0000
Message-ID: <20210113092448.GE32482@sync> (raw)
In-Reply-To: <>

Without this patch, that's even worst, consistency is broken.
Let me explain.

With your history example:

     ---o---o---M---o---o---W---o---o---M---o--- branch
          o---o---o---M---o--- master

If we imagine that master is having more commits count than branch.
The result of rev-list will be like you described:
$ git rev-list --left-right --cherry-pick branch...master

In other words, it's showing both W and M.

BUT, if we imagine now that master is having less commits count than branch.
$ git rev-list --left-right --cherry-pick branch...master

It's only showing W!

So the result is not consistent, depending on which branch is having
more commits.

With the patch, everything is consistent, and only W is kept in ouptut,
no matter the size of history:
$ git.p rev-list --left-right --cherry-pick branch...master


Arnaud Morin

On 12.01.21 - 11:13, Junio C Hamano wrote:
> Jeff King <> writes:
> > At the end, we'll have eliminated commits from both sides that have a
> > matching patch-id on the other side. But there's a subtle assumption
> > here: for any given patch-id, we must have exactly one struct
> > representing it. If two commits from A both have the same patch-id and
> > we allow duplicates in the hashmap, then we run into a problem:
> In practical terms, for one side of the history to have two
> identical patches, the most likely scenerio for that to happen is to
> have a patch, its reversion, and its reapplication, intermixed in
> its history with other commits, e.g.
>     ---o---o---M---o---o---W---o---o---M---o--- ...
>         \
>          o---o---o---M---o--- ...
> where "M" are commits that does an identical change, and "W"
> (upside-down "M") is its reversion.  On the top history, "M" was
> introduced, the bottom history cherry-picked, the top history found
> problems in it and reverted with "W", and later (perhaps with the
> help of preparatory patches on top of "W"), the top history now
> considers that "M" is safe, so reapplies it.
> And a "--cherry-pick" output that excludes "identical patches" that
> appear on both sides on such a history would hide all "M"'s, leaving
> a history like
>     ---o---o-------o---o---W---o---o-------o--- ...
>         \
>          o---o---o-------o--- ...
> But is this result what the user really wanted to see, I have to
> wonder.
> I do not see any problem in the patch itself.  We used to hide only
> one "M" from the history on the top half in the picture, leaving one
> "M" and "W", while hiding the sole "M" from the bottom half.  Now if
> we want to no longer show any "M", the updated code would correctly
> hide all of them.
> It just feels to me that the resulting view of the history look
> weird, leaving only the reversion of a patch that has never been
> applied in the first place.

  reply	other threads:[~2021-01-13  9:27 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-09 16:24 rev-list with multiple commits sharing same patch-id Arnaud Morin
2021-01-11  7:59 ` Arnaud Morin
2021-01-11  9:54 ` Christian Couder
2021-01-11 18:25   ` Arnaud Morin
2021-01-12 14:17 ` Jeff King
2021-01-12 15:11   ` Jeff King
2021-01-12 15:34     ` Arnaud Morin
2021-01-12 15:52       ` [PATCH] patch-ids: handle duplicate hashmap entries Jeff King
2021-01-12 16:24         ` Arnaud Morin
2021-01-12 19:13         ` Junio C Hamano
2021-01-13  9:24           ` Arnaud Morin [this message]
2021-01-13 12:59             ` Jeff King
2021-01-13 20:21               ` Junio C Hamano
2021-01-13 20:33                 ` Jeff King
2021-01-13 19:28             ` 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:

  List information:

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

  git send-email \
    --in-reply-to=20210113092448.GE32482@sync \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ \
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
 note: .onion URLs require Tor:

code repositories for the project(s) associated with this inbox:

AGPL code for this site: git clone