All of lore.kernel.org
 help / color / mirror / Atom feed
From: Phillip Wood <phillip.wood123@gmail.com>
To: "D. Ben Knoble" <ben.knoble@gmail.com>,
	Harald Nordgren via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Harald Nordgren <haraldnordgren@gmail.com>
Subject: Re: [PATCH] branch: let --delete-merged find squash merged branches
Date: Sun, 4 Oct 2026 10:54:10 +0100	[thread overview]
Message-ID: <39a28064-1698-4971-a80f-4a4c4dcdd8d9@gmail.com> (raw)
In-Reply-To: <CALnO6CBwWy3aafyDJPKFk5vuWy2EF1n1Oc=W7+RVAE3rxpXwiw@mail.gmail.com>

On 29/09/2026 12:26, D. Ben Knoble wrote:
> On Tue, Sep 29, 2026 at 3:33 AM Harald Nordgren via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
> 
> …in the rebase case, I would expect something like git-log's
> --cherry-mark option (or really the algorithm behind it, git-cherry,
> and git-range-diff) 

That's what I was expecting as well. It would be worth carefully 
studying the implementation of git-cherry. "git cherry A...B" 
precalculates the patch-ids from the side of the merge base that has the 
fewest commits and then walks the other side to compare them. While it 
is walking the other side I think it also looks at which paths were 
changed to avoid calculating the patch-id for commits that cannot match. 
It also batches fetches the blobs it needs in partial clones.

As far as I can see the implementation here makes a separate upstream 
revision walk for each branch, and recalculates the upstream diffs each 
time which seems less efficient than it could be.
> to be useful for identifying rebased branches. But
> of course even rebase-merged branches can end up with minor
> differences (say, a commit was made upstream before that branch was
> rebased with an identical change; no conflict occurs, but the new
> commit differs from the old by not having that change).

Yes if a branch has been rebased before it is merged it may be altered 
such that we cannot detect it.
> In the squash case, I suppose the best we can do is check that all our
> changes were applied at some point between the merge-base and the tip.
> There probably won't be any tree-same commits, though maybe a
> (premature?) optimization can return early if the trees match exactly.

If we have

(topic)  D - C - B - A
                       \
  (main)    M - Q - P - O -
             \          /
               - - S - -

where M is a squashed merge of topic I think we have

     M^2^{tree} == topic^{tree}
     Merge-base(M^1, M^2) == Merge-base(topic, topic@{upstream})
     $(git rev-list --count --right-only M^1...M^2) == 1

If you know your repository only has squash merges that were not rebased 
it would be a lot more efficient to just look at the trees and 
merge-bases, especially in a blobless clone. Having an option to turn 
off the patch-id based detection would probably be useful in that case.

I think detecting branches that have been squashed and/or rebased is a 
useful improvement, but it needs careful implementation to be efficient 
enough that it is practical in large repositories and I'm unlikely to 
have time to closely review it.

Thanks

Phillip

> It looked like you don't distinguish the 2 cases in the code, and I
> think that's reasonable: we wouldn't know a priori whether to check
> for a rebased series or a squashed commit, so we'd have to run both
> checks, and the latter presumably subsumes the former.
> 
> Anyway, I can see how this would all be fairly expensive---on one repo
> I work in, git-range-diff can be somewhat slow depending on how many
> commits are in the range, I think. I don't know if it's worth trying
> to state that for folks, though? If we ever make improvements to
> performance, we'd have to remember to remove the "this may be slow"
> text.
> 
>> After the release of 2.56, I saw people liking the --delete-merged
>>     feature, but asking for this. A lot of people, me included prefer
>>     squash-merge and it currently doesn't work with --delete-merged.
> 
> Btw, I wonder if you can share where you saw this? 2.56 was released
> so recently I'm (pleasantly) surprised there's already feedback on
> this!
> 


  parent reply	other threads:[~2026-10-04  9:54 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-29  7:30 [PATCH] branch: let --delete-merged find squash merged branches Harald Nordgren via GitGitGadget
2026-09-29  7:46 ` Kristoffer Haugsbakk
2026-09-29  7:52   ` Harald Nordgren
2026-09-29  8:08     ` Kristoffer Haugsbakk
2026-09-29 11:18       ` D. Ben Knoble
2026-09-29 13:31         ` Kristoffer Haugsbakk
2026-09-29 11:26 ` D. Ben Knoble
2026-09-29 13:44   ` Harald Nordgren
2026-09-29 13:52   ` Harald Nordgren
2026-09-29 15:28     ` D. Ben Knoble
2026-09-29 18:34       ` Harald Nordgren
2026-10-04  9:54   ` Phillip Wood [this message]
2026-10-04 22:28     ` Harald Nordgren
2026-10-08 15:25       ` Phillip Wood
2026-10-08 18:42         ` Harald Nordgren
2026-10-09  8:16           ` Kristoffer Haugsbakk
2026-10-09 11:11             ` Harald Nordgren
2026-10-09 11:18           ` Harald Nordgren
2026-10-09 13:40             ` Phillip Wood
2026-10-09 13:37           ` Phillip Wood
2026-10-09 15:27             ` Harald Nordgren
2026-10-08 17:51     ` D. Ben Knoble
2026-10-09 13:34       ` Phillip Wood
2026-10-11  9:41 ` [PATCH v2 0/2] " Harald Nordgren via GitGitGadget
2026-10-11  9:41   ` [PATCH v2 1/2] " Harald Nordgren via GitGitGadget
2026-10-11  9:41   ` [PATCH v2 2/2] branch: add --no-squashed to skip the squash search Harald Nordgren via GitGitGadget
2026-10-12  0:30   ` [PATCH v2 0/2] branch: let --delete-merged find squash merged branches Junio C Hamano
2026-10-12  9:50     ` Phillip Wood
2026-10-12 14:31       ` Harald Nordgren
2026-10-12 16:50       ` Junio C Hamano
2026-10-12 17:44         ` Harald Nordgren
2026-10-13  2:23           ` Junio C Hamano
2026-10-12 12:41     ` Harald Nordgren
2026-10-12 15:49       ` D. Ben Knoble

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=39a28064-1698-4971-a80f-4a4c4dcdd8d9@gmail.com \
    --to=phillip.wood123@gmail.com \
    --cc=ben.knoble@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=haraldnordgren@gmail.com \
    --cc=phillip.wood@dunelm.org.uk \
    /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.