On Tue, Aug 25, 2026 at 8:14 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Nikita Leshenko <nikita@xxxxxxxxx> writes:
>
> > This does not change the behavior of how patches apply, but when the user
> > requested --3way it does cost one extra "git apply --build-fake-ancestor"
> > process and one extra apply per clean patch.
>
> If you have a 50-patch series that cleanly applies, we would incur
> overhead to spawn 49 extra "git apply --build-fake-ancestor"
> subprocesses, to write and unlink 49 temporary index files, and to
> perform 49 in-core patch applications, generating unneeded loose
> objects in the object database, and loading and unloading the index
> file one extra time per step. That is simply unacceptable.
Understood.
>
> Can't you do the equivalent lazily inside fall_back_threeway()
> instead? A rough outline may go like so:
>
> * Imagine that, after applying patches 1..(N-1) successfully, you
> are applying patch N.
>
> - First try direct application of the patch, and it fails.
>
> - You call fall_back_threeway().
>
> - build_fake_ancestor() is called for patch N; if the preimage
> blob exists, you are done, but the case you want to address is
> what to do when the preimage is missing. And in that case (and
> in that case only), can't you reconstruct the image chain
> lazily?
>
> Instead of returning error("could not build fake ancestor"):
>
> - You inspect patches in .git/rebase-apply/ for 1..(N-1)
> patches (i.e., those you have applied already) to find the
> relevant blob objects involved in reconstructing the
> preimage blob necessary to apply patch N. Some of the
> blobs may already exist in the object database (83b2a16
> in your example).
>
> - Apply these previous patches in-core to arrive at the
> preimage recorded in these earlier patches (applying patch 1
> to 83b2a16 would now give you cccad2b), until you see the
> preimage blob recorded in patch N. Write out that blob
> object (and not the blobs that the chain may have
> produced as a result of intermediate patches).
>
> - If the lazy reconstruction yielded the necessary blobs, try the
> build_fake_ancestor() call again, which should succeed. If
> not, you can return error("could not build fake ancestor").
>
> - And after patch N succeeds with 3-way fallback this way, you
> would also have the postimage blob recorded in the patch in
> your object database, which may help when you apply patch
> (N+1).
>
> When the patches cleanly apply, or if 3-way finds necessary blobs
> already, there is no additional overhead with the above approach.
>
> Hmm?
I'm concerned about the complexity of creating such lazy reconstruction
logic, especially for cases where multiple blobs from the patch are missing,
and their preimages were modified in different previous commits (which could
in turn have multiple blobs missing from other commits). You mentioned
writing only the strictly necessary blobs to the database so IIUC I'll need
pretty elaborate scanning logic to surgically perform the minimal number of
applies given a list of missing hashes.
This can be done, but IMO such complexity isn't warranted for this
relatively niche problem. (I haven't seen online discussion about this
exact flavor of the issue, even though I encounter it from time to time.)
How about this:
- If build_fake_ancestor() fails due to useless sha1 information, try to
apply ALL 1..(N-1) patches on their fake ancestors. This will build a
lot of unrelated blobs but will build the missing blob. This will allow
us to build fake ancestor and apply N on it.
- Record in am_state .git/rebase-apply called "postimage-attempted" (WIP
name) that we tried to apply on fake ancestors all the way to N. So if
patch M > N later fails due to missing sha1, we apply ALL (N+1)..(M-1)
patches on their fake ancestors.
- Optional optimization: if patch N was applied on its fake ancestor and
"postimage-attempted" is N-1, bump it to N.
This is less optimized than you suggested but it doesn't hurt the clean
path, and this logic kicks in only when patch application would have
otherwise failed.