Re: [PATCH] merge-ort: fix corner case recursive submodule/directory conflict handling

From: Elijah Newren <newren@gmail.com>
Date: Dec 30, 2025, 22:53 UTC
Message-ID: <CABPp-BEQ6hyUpjsK18dKwHmZ+cjDm8-ah8472+JOpKh65a-W6A@mail.gmail.com>
In-Reply-To: <9fae6398-cdc5-4ccf-8bd6-47425efb0c5b@app.fastmail.com>

On Tue, Dec 30, 2025 at 8:04 AM Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com> wrote:

>
> > [PATCH] merge-ort: fix corner case recursive submodule/directory conflict handling
>
> s/corner case/corner case in/ ?

Sure.

> > We would also be able to do tree-level merging if we somehow apriori
>
> s/apriori/a priori/

Thanks.

> > which was implemented back in 7bee6c100431 (merge-ort: avoid recursing
> > into directories when we don't need to, 2021-07-16)  Crucially, this
>
> Missing period before “Crucially”.

Thanks.

Show 8 quoted lines

>
> > restarting only occurs if the number of paths we could skip recursing
> > into exceeds the number we still need to recurse into by some safety
> > factor (wanted_factor in handle_deferred_entries()); forgetting this
> > fact is a great way to repeatedly fail to create a minimal testcase for
> > several days and go down alternate wrong paths).
>
> Missing open paren?

Or just extra closing paren; I'll remove the final one.

> >   * do not pre-emptively mark the path as cleanly merged if the
>
> s/pre-emptively/preemptively/ (according to Merriam Webster)

Looks like it's not consistent among different style guides, but I'll go with your preference.

Show 15 quoted lines

> >     remaining path is a file; allow it to be processed in
> >     process_entries() later to determine if it was clean
> >   * clear the parts of dirmask or filemask corresponding to the matching
> >     sides of history, since we are resolving those away
> >   * clear the df_conflict bit afterwards; since we cleared away the two
> >     matching sides and only have one side left, that one side can't
> >     have a directory/file conflict with itself.
> >
> > Also add the above minimal testcase showcasing this bug to t6422, **with
> > a sufficient number of paths under the folder/ directory to actually
> > trigger it**.  (I wish I could have all those days back from all the
> > wrong paths I went down due to not having enough files under that
> > directory...)
>
> :)

Thanks for taking a look!