From: Junio C Hamano <firstname.lastname@example.org> To: Jeff King <email@example.com> Cc: Arnaud Morin <firstname.lastname@example.org>, email@example.com, Kevin Willford <firstname.lastname@example.org> Subject: Re: [PATCH] patch-ids: handle duplicate hashmap entries Date: Tue, 12 Jan 2021 11:13:06 -0800 Message-ID: <email@example.com> (raw) In-Reply-To: <X/3FwNPHqZqYfirstname.lastname@example.org> (Jeff King's message of "Tue, 12 Jan 2021 10:52:32 -0500") Jeff King <email@example.com> 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.
next prev parent reply other threads:[~2021-01-12 19:15 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 [this message] 2021-01-13 9:24 ` Arnaud Morin 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: 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 \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.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
firstname.lastname@example.org list mirror (unofficial, one of many) This inbox may be cloned and mirrored by anyone: git clone --mirror https://public-inbox.org/git 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/ https://public-inbox.org/git \ email@example.com public-inbox-index git Example config snippet for mirrors. Newsgroups are available over NNTP: nntp://news.public-inbox.org/inbox.comp.version-control.git nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git nntp://news.gmane.io/gmane.comp.version-control.git note: .onion URLs require Tor: https://www.torproject.org/ code repositories for the project(s) associated with this inbox: https://80x24.org/mirrors/git.git AGPL code for this site: git clone https://public-inbox.org/public-inbox.git