{"thread":{"id":"66215","subject":"[PATCH] am: record blobs of cleanly applied patches when using --3way","startedAt":"2026-08-25T08:56:30Z","lastAt":"2026-08-25T20:26:55Z","messageCount":3,"participants":["Nikita Leshenko","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"551169","messageId":"20260825085516.66088-1-nikita@island.io","threadId":"66215","inReplyTo":null,"subject":"[PATCH] am: record blobs of cleanly applied patches when using --3way","fromName":"Nikita Leshenko","fromEmail":"nikita@island.io","sentAt":"2026-08-25T08:55:16Z","receivedAt":"2026-08-25T08:56:30Z","isPatch":true,"body":"Make \"git am --3way\" succeed in an edge case where it currently fails.\n\nFirst, some background about the case:\n\nSay we have a patch with two commits, A and B, and both of them change the\nsame file.  We apply them to a different repo on a different version of the\nfile using --3way.\n\nIf both patches apply cleanly, we are done.\n\nIf A does not apply cleanly, git am falls back to 3-way merge.  To merge,\nGit uses the preimage hash from the patch:\n\n    A: index 83b2a16..cccad2b 100644\n    B: index cccad2b..0ce2f98 100644\n\ngit am looks up 83b2a16, applies A to it, and merges that result with the\ncurrent version of the file.  As an important side effect, applying A to\n83b2a16 also stores A's postimage, cccad2b, in the repository.  This means\nthat if B also doesn't apply cleanly, cccad2b (which is now B's preimage)\nexists in the repository so we can merge against it as well.\n\nLet's assume instead that A applies cleanly and B fails.  Because git am\ndidn't have to 3-way merge A, nothing created cccad2b this time.  What we\nhave after applying A is our file plus A's change, which is a different\nhash.  When B doesn't apply cleanly, git am fails because it doesn't know\nwhat cccad2b is:\n\n    Applying: A\n    Applying: B\n    error: sha1 information is lacking or useless (file).\n    error: could not build fake ancestor\n\nHowever, technically we have the information to build the fake ancestor!  We\nhave 83b2a16 in the repository, and we have A, so if we apply A we'll get\nthat hash.\n\nSo do exactly that: if the user requested --3way, apply the patch on the\nfake ancestor even after a patch applies cleanly, in order to produce\nintermediate hashes for later commits.  If the preimage is missing, or the\npatch does not apply, nothing is recorded and git am behaves as it does\ntoday.\n\nThis does not change the behavior of how patches apply, but when the user\nrequested --3way it does cost one extra \"git apply --build-fake-ancestor\"\nprocess and one extra apply per clean patch.\n\nSigned-off-by: Nikita Leshenko <nikita@island.io>\n---\n builtin/am.c  | 58 ++++++++++++++++++++++++++++++++++++++++++++-------\n t/t4150-am.sh | 28 +++++++++++++++++++++++++\n 2 files changed, 79 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/am.c b/builtin/am.c\nindex e9623b8307..37569fed65 100644\n--- a/builtin/am.c\n+++ b/builtin/am.c\n@@ -1488,7 +1488,8 @@ static int parse_mail_rebase(struct am_state *state, const char *mail)\n  * Applies current patch with git-apply. Returns 0 on success, -1 otherwise. If\n  * `index_file` is not NULL, the patch will be applied to that index.\n  */\n-static int run_apply(const struct am_state *state, const char *index_file)\n+static int run_apply(const struct am_state *state, const char *index_file,\n+\t\t     int quiet)\n {\n \tstruct strvec apply_paths = STRVEC_INIT;\n \tstruct strvec apply_opts = STRVEC_INIT;\n@@ -1528,7 +1529,7 @@ static int run_apply(const struct am_state *state, const char *index_file)\n \t * If we are allowed to fall back on 3-way merge, don't give false\n \t * errors during the initial attempt.\n \t */\n-\tif (state->threeway && !index_file)\n+\tif (quiet || (state->threeway && !index_file))\n \t\tapply_state.apply_verbosity = verbosity_silent;\n \n \tif (check_apply_state(&apply_state, force_apply))\n@@ -1559,11 +1560,13 @@ static int run_apply(const struct am_state *state, const char *index_file)\n /**\n  * Builds an index that contains just the blobs needed for a 3way merge.\n  */\n-static int build_fake_ancestor(const struct am_state *state, const char *index_file)\n+static int build_fake_ancestor(const struct am_state *state,\n+\t\t\t       const char *index_file, int quiet)\n {\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n \n \tcp.git_cmd = 1;\n+\tcp.no_stderr = quiet;\n \tstrvec_push(&cp.args, \"apply\");\n \tstrvec_pushv(&cp.args, state->git_apply_opts.v);\n \tstrvec_pushf(&cp.args, \"--build-fake-ancestor=%s\", index_file);\n@@ -1589,7 +1592,7 @@ static int fall_back_threeway(const struct am_state *state, const char *index_pa\n \tif (repo_get_oid(the_repository, \"HEAD\", &our_tree) < 0)\n \t\toidcpy(&our_tree, the_hash_algo->empty_tree);\n \n-\tif (build_fake_ancestor(state, index_path))\n+\tif (build_fake_ancestor(state, index_path, 0))\n \t\treturn error(\"could not build fake ancestor\");\n \n \tdiscard_index(the_repository->index);\n@@ -1617,7 +1620,7 @@ static int fall_back_threeway(const struct am_state *state, const char *index_pa\n \t\trelease_revisions(&rev_info);\n \t}\n \n-\tif (run_apply(state, index_path))\n+\tif (run_apply(state, index_path, 0))\n \t\treturn error(_(\"Did you hand edit your patch?\\n\"\n \t\t\t\t\"It does not apply to blobs recorded in its index.\"));\n \n@@ -1658,6 +1661,39 @@ static int fall_back_threeway(const struct am_state *state, const char *index_pa\n \treturn 0;\n }\n \n+/**\n+ * Applies the patch on the fake ancestor and stores the postimage in the\n+ * repository for future patches to reference.  Best effort, fails quietly.\n+ *\n+ * Motivation: Say a patch file has two commits, A and B, that both change the\n+ * same file.  We apply it with --3way.  If A applies cleanly, nothing stores\n+ * A's postimage.  If B then does not apply cleanly, git am cannot 3-way merge\n+ * it, because B's preimage is A's postimage.\n+ */\n+static void try_record_patch_postimage(const struct am_state *state)\n+{\n+\tstruct strbuf index_path = STRBUF_INIT;\n+\n+\tstrbuf_addstr(&index_path, am_path(state, \"patch-postimage-index\"));\n+\n+\tif (build_fake_ancestor(state, index_path.buf, 1))\n+\t\tgoto done;\n+\n+\t/*\n+         * Discard index because run_apply() reads `index_path` only if no index\n+         * is in core.\n+         */\n+\tdiscard_index(the_repository->index);\n+\trun_apply(state, index_path.buf, 1);\n+\n+\tdiscard_index(the_repository->index);\n+\trepo_read_index(the_repository);\n+\n+done:\n+\tunlink(index_path.buf);\n+\tstrbuf_release(&index_path);\n+}\n+\n /**\n  * Commits the current index with state->msg as the commit message and\n  * state->author_name, state->author_email and state->author_date as the author\n@@ -1886,9 +1922,17 @@ static void am_run(struct am_state *state, int resume)\n \n \t\tsay(state, stdout, _(\"Applying: %.*s\"), linelen(state->msg), state->msg);\n \n-\t\tapply_status = run_apply(state, NULL);\n+\t\tapply_status = run_apply(state, NULL, 0);\n \n-\t\tif (apply_status && state->threeway) {\n+\t\tif (!apply_status && state->threeway && state->cur < state->last) {\n+\t\t\t/*\n+\t\t\t * The patch applied cleanly, so no 3-way was performed.\n+\t\t\t * A patch later in the series may reference postimage\n+\t\t\t * hashes this patch would have produced, so record them\n+\t\t\t * while we still can.\n+\t\t\t */\n+\t\t\ttry_record_patch_postimage(state);\n+\t\t} else if (apply_status && state->threeway) {\n \t\t\tstruct strbuf sb = STRBUF_INIT;\n \n \t\t\tstrbuf_addstr(&sb, am_path(state, \"patch-merge-index\"));\ndiff --git a/t/t4150-am.sh b/t/t4150-am.sh\nindex ee96223668..e5ed666c2f 100755\n--- a/t/t4150-am.sh\n+++ b/t/t4150-am.sh\n@@ -641,6 +641,34 @@ test_expect_success 'am with config am.threeWay overridden by --no-3way' '\n \ttest_path_is_dir .git/rebase-apply\n '\n \n+test_expect_success 'am -3 records blobs a later patch needs' '\n+\ttest_when_finished \"rm -rf 3way-source 3way-target\" &&\n+\n+\t# Two patches touching the same file, the first of which applies\n+\t# cleanly to the target while the second one does not.\n+\tgit init 3way-source &&\n+\ttest_write_lines 1 2 3 4 5 6 7 8 9 >3way-source/file &&\n+\tgit -C 3way-source add file &&\n+\tgit -C 3way-source commit -m base &&\n+\ttest_write_lines 11 2 3 4 5 6 7 8 9 >3way-source/file &&\n+\tgit -C 3way-source commit -am first &&\n+\ttest_write_lines 11 2 3 4 5 66 7 8 9 >3way-source/file &&\n+\tgit -C 3way-source commit -am second &&\n+\tgit -C 3way-source format-patch --stdout -2 >3way-two.patches &&\n+\n+\tgit init 3way-target &&\n+\ttest_write_lines 1 2 3 4 5 6 7 8 9 >3way-target/file &&\n+\tgit -C 3way-target add file &&\n+\tgit -C 3way-target commit -m base &&\n+\ttest_write_lines 1 2 3 4 5 6 7 8 9XXX >3way-target/file &&\n+\tgit -C 3way-target commit -am \"change outside the first patch\" &&\n+\n+\tgit -C 3way-target am -3 ../3way-two.patches &&\n+\ttest_path_is_missing 3way-target/.git/rebase-apply &&\n+\ttest_write_lines 11 2 3 4 5 66 7 8 9XXX >expect &&\n+\ttest_cmp expect 3way-target/file\n+'\n+\n test_expect_success 'am can rename a file' '\n \ttest_grep \"^rename from\" rename.patch &&\n \trm -fr .git/rebase-apply &&\n-- \n2.55.0\n\n"},{"id":"551199","messageId":"xmqqmruam8at.fsf@gitster.g","threadId":"66215","inReplyTo":"20260825085516.66088-1-nikita@island.io","subject":"Re: [PATCH] am: record blobs of cleanly applied patches when using --3way","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-25T17:14:34Z","receivedAt":"2026-08-25T17:14:37Z","isPatch":true,"body":"Nikita Leshenko <nikita@island.io> writes:\n\n> Make \"git am --3way\" succeed in an edge case where it currently fails.\n>\n> First, some background about the case:\n>\n> Say we have a patch with two commits, A and B, and both of them change the\n> same file.  We apply them to a different repo on a different version of the\n> file using --3way.\n>\n> If both patches apply cleanly, we are done.\n>\n> If A does not apply cleanly, git am falls back to 3-way merge.  To merge,\n> Git uses the preimage hash from the patch:\n>\n>     A: index 83b2a16..cccad2b 100644\n>     B: index cccad2b..0ce2f98 100644\n>\n> git am looks up 83b2a16, applies A to it, and merges that result with the\n> current version of the file.  As an important side effect, applying A to\n> 83b2a16 also stores A's postimage, cccad2b, in the repository.  This means\n> that if B also doesn't apply cleanly, cccad2b (which is now B's preimage)\n> exists in the repository so we can merge against it as well.\n>\n> Let's assume instead that A applies cleanly and B fails.  Because git am\n> didn't have to 3-way merge A, nothing created cccad2b this time.  What we\n> have after applying A is our file plus A's change, which is a different\n> hash.  When B doesn't apply cleanly, git am fails because it doesn't know\n> what cccad2b is:\n>\n>     Applying: A\n>     Applying: B\n>     error: sha1 information is lacking or useless (file).\n>     error: could not build fake ancestor\n>\n> However, technically we have the information to build the fake ancestor!  We\n> have 83b2a16 in the repository, and we have A, so if we apply A we'll get\n> that hash.\n>\n> So do exactly that: if the user requested --3way, apply the patch on the\n> fake ancestor even after a patch applies cleanly, in order to produce\n> intermediate hashes for later commits.  If the preimage is missing, or the\n> patch does not apply, nothing is recorded and git am behaves as it does\n> today.\n>\n> This does not change the behavior of how patches apply, but when the user\n> requested --3way it does cost one extra \"git apply --build-fake-ancestor\"\n> process and one extra apply per clean patch.\n\nIf you have a 50-patch series that cleanly applies, we would incur\noverhead to spawn 49 extra \"git apply --build-fake-ancestor\"\nsubprocesses, to write and unlink 49 temporary index files, and to\nperform 49 in-core patch applications, generating unneeded loose\nobjects in the object database, and loading and unloading the index\nfile one extra time per step.  That is simply unacceptable.\n\nCan't you do the equivalent lazily inside fall_back_threeway()\ninstead?  A rough outline may go like so:\n\n * Imagine that, after applying patches 1..(N-1) successfully, you\n   are applying patch N.\n\n   - First try direct application of the patch, and it fails.\n\n   - You call fall_back_threeway().\n\n   - build_fake_ancestor() is called for patch N; if the preimage\n     blob exists, you are done, but the case you want to address is\n     what to do when the preimage is missing.  And in that case (and\n     in that case only), can't you reconstruct the image chain\n     lazily?\n\n     Instead of returning error(\"could not build fake ancestor\"):\n\n     - You inspect patches in .git/rebase-apply/ for 1..(N-1)\n       patches (i.e., those you have applied already) to find the\n       relevant blob objects involved in reconstructing the\n       preimage blob necessary to apply patch N.  Some of the\n       blobs may already exist in the object database (83b2a16\n       in your example).\n\n     - Apply these previous patches in-core to arrive at the\n       preimage recorded in these earlier patches (applying patch 1\n       to 83b2a16 would now give you cccad2b), until you see the\n       preimage blob recorded in patch N.  Write out that blob\n       object (and not the blobs that the chain may have\n       produced as a result of intermediate patches).\n\n   - If the lazy reconstruction yielded the necessary blobs, try the\n     build_fake_ancestor() call again, which should succeed.  If\n     not, you can return error(\"could not build fake ancestor\").\n\n   - And after patch N succeeds with 3-way fallback this way, you\n     would also have the postimage blob recorded in the patch in\n     your object database, which may help when you apply patch\n     (N+1).\n\nWhen the patches cleanly apply, or if 3-way finds necessary blobs\nalready, there is no additional overhead with the above approach.\n\nHmm?\n"},{"id":"551236","messageId":"CAEXts1trFiGJKZfgE=-HAkEcLPVB7Hsx88JX-NHXHX+G+=e_RQ@mail.gmail.com","threadId":"66215","inReplyTo":"xmqqmruam8at.fsf@gitster.g","subject":"Re: [PATCH] am: record blobs of cleanly applied patches when using --3way","fromName":"Nikita Leshenko","fromEmail":"nikita@island.io","sentAt":"2026-08-25T20:26:42Z","receivedAt":"2026-08-25T20:26:55Z","isPatch":true,"body":"On Tue, Aug 25, 2026 at 8:14 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Nikita Leshenko <nikita@island.io> writes:\n>\n> > This does not change the behavior of how patches apply, but when the user\n> > requested --3way it does cost one extra \"git apply --build-fake-ancestor\"\n> > process and one extra apply per clean patch.\n>\n> If you have a 50-patch series that cleanly applies, we would incur\n> overhead to spawn 49 extra \"git apply --build-fake-ancestor\"\n> subprocesses, to write and unlink 49 temporary index files, and to\n> perform 49 in-core patch applications, generating unneeded loose\n> objects in the object database, and loading and unloading the index\n> file one extra time per step.  That is simply unacceptable.\n\nUnderstood.\n\n>\n> Can't you do the equivalent lazily inside fall_back_threeway()\n> instead?  A rough outline may go like so:\n>\n>  * Imagine that, after applying patches 1..(N-1) successfully, you\n>    are applying patch N.\n>\n>    - First try direct application of the patch, and it fails.\n>\n>    - You call fall_back_threeway().\n>\n>    - build_fake_ancestor() is called for patch N; if the preimage\n>      blob exists, you are done, but the case you want to address is\n>      what to do when the preimage is missing.  And in that case (and\n>      in that case only), can't you reconstruct the image chain\n>      lazily?\n>\n>      Instead of returning error(\"could not build fake ancestor\"):\n>\n>      - You inspect patches in .git/rebase-apply/ for 1..(N-1)\n>        patches (i.e., those you have applied already) to find the\n>        relevant blob objects involved in reconstructing the\n>        preimage blob necessary to apply patch N.  Some of the\n>        blobs may already exist in the object database (83b2a16\n>        in your example).\n>\n>      - Apply these previous patches in-core to arrive at the\n>        preimage recorded in these earlier patches (applying patch 1\n>        to 83b2a16 would now give you cccad2b), until you see the\n>        preimage blob recorded in patch N.  Write out that blob\n>        object (and not the blobs that the chain may have\n>        produced as a result of intermediate patches).\n>\n>    - If the lazy reconstruction yielded the necessary blobs, try the\n>      build_fake_ancestor() call again, which should succeed.  If\n>      not, you can return error(\"could not build fake ancestor\").\n>\n>    - And after patch N succeeds with 3-way fallback this way, you\n>      would also have the postimage blob recorded in the patch in\n>      your object database, which may help when you apply patch\n>      (N+1).\n>\n> When the patches cleanly apply, or if 3-way finds necessary blobs\n> already, there is no additional overhead with the above approach.\n>\n> Hmm?\n\nI'm concerned about the complexity of creating such lazy reconstruction\nlogic, especially for cases where multiple blobs from the patch are missing,\nand their preimages were modified in different previous commits (which could\nin turn have multiple blobs missing from other commits).  You mentioned\nwriting only the strictly necessary blobs to the database so IIUC I'll need\npretty elaborate scanning logic to surgically perform the minimal number of\napplies given a list of missing hashes.\n\nThis can be done, but IMO such complexity isn't warranted for this\nrelatively niche problem.  (I haven't seen online discussion about this\nexact flavor of the issue, even though I encounter it from time to time.)\n\nHow about this:\n\n  - If build_fake_ancestor() fails due to useless sha1 information, try to\n    apply ALL 1..(N-1) patches on their fake ancestors.  This will build a\n    lot of unrelated blobs but will build the missing blob.  This will allow\n    us to build fake ancestor and apply N on it.\n\n  - Record in am_state .git/rebase-apply called \"postimage-attempted\" (WIP\n    name) that we tried to apply on fake ancestors all the way to N.  So if\n    patch M > N later fails due to missing sha1, we apply ALL (N+1)..(M-1)\n    patches on their fake ancestors.\n\n  - Optional optimization: if patch N was applied on its fake ancestor and\n    \"postimage-attempted\" is N-1, bump it to N.\n\nThis is less optimized than you suggested but it doesn't hurt the clean\npath, and this logic kicks in only when patch application would have\notherwise failed.\n"}]}