{"thread":{"id":"65962","subject":"[PATCH] builtin/add.c: replace run_command() with direct apply_all_patches() call","startedAt":"2026-07-09T19:26:31Z","lastAt":"2026-08-26T17:16:06Z","messageCount":9,"participants":["Gatla Vishweshwar Reddy","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"547645","messageId":"20260709192619.46791-1-gatlavishweshwarreddy26@gmail.com","threadId":"65962","inReplyTo":null,"subject":"[PATCH] builtin/add.c: replace run_command() with direct apply_all_patches() call","fromName":"Gatla Vishweshwar Reddy","fromEmail":"gatlavishweshwarreddy26@gmail.com","sentAt":"2026-07-09T19:26:19Z","receivedAt":"2026-07-09T19:26:31Z","isPatch":true,"body":"When the user runs \"git add -e\", the diff of the working tree changes\nis written to a temporary file, opened in an editor, and then applied\nback to the index. The application step was done by spawning a child\nprocess running \"git apply --recount --cached <file>\", which is an\nunnecessary subprocess since the apply machinery is available as a\nnative C API.\n\nReplace the run_command() call with a direct call to apply_all_patches()\nusing an initialized apply_state with the cached and recount options set\nappropriately. This avoids the overhead of forking a subprocess, keeps\nthe operation within the same process, and makes the intent of the code\nclearer to the reader.\n\nRemove the now-unused includes of \"run-command.h\" and \"strvec.h\" since\nno other code in this file requires them after this change.\n\nSigned-off-by: Gatla Vishweshwar Reddy <gatlavishweshwarreddy26@gmail.com>\n---\n builtin/add.c | 16 +++++++++-------\n 1 file changed, 9 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex c859f66519..8172c0c935 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -13,7 +13,6 @@\n #include \"dir.h\"\n #include \"gettext.h\"\n #include \"pathspec.h\"\n-#include \"run-command.h\"\n #include \"object-file.h\"\n #include \"odb.h\"\n #include \"odb/transaction.h\"\n@@ -23,9 +22,9 @@\n #include \"diff.h\"\n #include \"read-cache.h\"\n #include \"revision.h\"\n-#include \"strvec.h\"\n #include \"submodule.h\"\n #include \"add-interactive.h\"\n+#include \"apply.h\"\n \n static const char * const builtin_add_usage[] = {\n \tN_(\"git add [<options>] [--] <pathspec>...\"),\n@@ -187,7 +186,6 @@ static int edit_patch(struct repository *repo,\n \t\t      const char *prefix)\n {\n \tchar *file = repo_git_path(repo, \"ADD_EDIT.patch\");\n-\tstruct child_process child = CHILD_PROCESS_INIT;\n \tstruct rev_info rev;\n \tint out;\n \tstruct stat st;\n@@ -217,11 +215,15 @@ static int edit_patch(struct repository *repo,\n \tif (!st.st_size)\n \t\tdie(_(\"empty patch. aborted\"));\n \n-\tchild.git_cmd = 1;\n-\tstrvec_pushl(&child.args, \"apply\", \"--recount\", \"--cached\", file,\n-\t\t     NULL);\n-\tif (run_command(&child))\n+\tstruct apply_state state;\n+\tconst char *apply_argv[] = { file, NULL };\n+\n+\tif (init_apply_state(&state, repo, prefix))\n+\t\tdie(_(\"could not initialize apply state\"));\n+\tstate.cached = 1;\n+\tif (apply_all_patches(&state, 1, apply_argv, APPLY_OPT_RECOUNT))\n \t\tdie(_(\"could not apply '%s'\"), file);\n+\tclear_apply_state(&state);\n \n \tunlink(file);\n \tfree(file);\n-- \n2.54.0\n\n"},{"id":"547676","messageId":"xmqqmrvzfitd.fsf@gitster.g","threadId":"65962","inReplyTo":"20260709192619.46791-1-gatlavishweshwarreddy26@gmail.com","subject":"Re: [PATCH] builtin/add.c: replace run_command() with direct apply_all_patches() call","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-10T06:41:02Z","receivedAt":"2026-07-10T06:41:05Z","isPatch":true,"body":"Gatla Vishweshwar Reddy <gatlavishweshwarreddy26@gmail.com> writes:\n\n> When the user runs \"git add -e\", the diff of the working tree changes\n> is written to a temporary file, opened in an editor, and then applied\n> back to the index. The application step was done by spawning a child\n\n\"was\" -> \"is\"; in the first part of the log message that gives an\nobservation, we describe the status quo in the present tense.\n\n> process running \"git apply --recount --cached <file>\", which is an\n> unnecessary subprocess since the apply machinery is available as a\n> native C API.\n> @@ -187,7 +186,6 @@ static int edit_patch(struct repository *repo,\n>  \t\t      const char *prefix)\n>  {\n>  \tchar *file = repo_git_path(repo, \"ADD_EDIT.patch\");\n> -\tstruct child_process child = CHILD_PROCESS_INIT;\n>  \tstruct rev_info rev;\n>  \tint out;\n>  \tstruct stat st;\n> @@ -217,11 +215,15 @@ static int edit_patch(struct repository *repo,\n>  \tif (!st.st_size)\n>  \t\tdie(_(\"empty patch. aborted\"));\n>  \n> -\tchild.git_cmd = 1;\n> -\tstrvec_pushl(&child.args, \"apply\", \"--recount\", \"--cached\", file,\n> -\t\t     NULL);\n> -\tif (run_command(&child))\n> +\tstruct apply_state state;\n> +\tconst char *apply_argv[] = { file, NULL };\n> +\n> +\tif (init_apply_state(&state, repo, prefix))\n> +\t\tdie(_(\"could not initialize apply state\"));\n> +\tstate.cached = 1;\n> +\tif (apply_all_patches(&state, 1, apply_argv, APPLY_OPT_RECOUNT))\n>  \t\tdie(_(\"could not apply '%s'\"), file);\n> +\tclear_apply_state(&state);\n\nCompared to existing callers of the apply_all_patches() API\nfunction, this implementation curiously lacks a prior call to\ncheck_apply_state().\n\nHas this been tested, and do we have sufficient test coverage for it?\n\nCalling check_apply_state() should flip state->check_index on, given\nthat state.cached is set to 1 above. If I remember correctly, having\nthis bit enabled is required for apply_patch() to toggle the\n.update_index member, which in turn allows apply_all_patches() to\nupdate the index with the patch results. Please double-check this\nlogic, since it has been a while since I looked at these specific\ncode paths.\n\nIf my assumption holds, this patch might inadvertently stop writing\nthe result to the index, even though the original intent of\nreplacing 'apply --cached' was clearly to update it.\n\nThanks.\n\n\n>  \n>  \tunlink(file);\n>  \tfree(file);\n"},{"id":"547686","messageId":"20260710074105.50737-1-gatlavishweshwarreddy26@gmail.com","threadId":"65962","inReplyTo":"xmqqmrvzfitd.fsf@gitster.g","subject":"[PATCH v2] builtin/add.c: replace run_command() with direct apply_all_patches() call","fromName":"Gatla Vishweshwar Reddy","fromEmail":"gatlavishweshwarreddy26@gmail.com","sentAt":"2026-07-10T07:32:06Z","receivedAt":"2026-07-10T07:41:49Z","isPatch":true,"body":"When the user runs \"git add -e\", the diff of the working tree changes\nis written to a temporary file, opened in an editor, and then applied\nback to the index. The application step is done by spawning a child\nprocess running \"git apply --recount --cached <file>\", which is an\nunnecessary subprocess since the apply machinery is available as a\nnative C API.\n\nReplace the run_command() call with a direct call to apply_all_patches()\nusing an initialized apply_state with the cached and recount options set\nappropriately. This avoids the overhead of forking a subprocess, keeps\nthe operation within the same process, and makes the intent of the code\nclearer to the reader.\n\nRemove the now-unused includes of \"run-command.h\" and \"strvec.h\" since\nno other code in this file requires them after this change.\n\nSigned-off-by: Gatla Vishweshwar Reddy <gatlavishweshwarreddy26@gmail.com>\n---\n\nChanges in v2:\n- Fixed commit message: \"was done\" -> \"is done\" (present tense)\n- Added check_apply_state() call after setting state.cached = 1,\n  which sets state.check_index = 1 required for index updates\n\nIn response to review:\n\n- check_apply_state() with cached=1 correctly\n  sets check_index=1, ensuring apply_all_patches() updates the index\n  as intended. Verified by reading apply.c lines 172-175.\n\n- Tested with t3700-add.sh: all 58 tests pass\n\n\n builtin/add.c | 18 +++++++++++-------\n 1 file changed, 11 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex c859f66519..a7266020cd 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -13,7 +13,6 @@\n #include \"dir.h\"\n #include \"gettext.h\"\n #include \"pathspec.h\"\n-#include \"run-command.h\"\n #include \"object-file.h\"\n #include \"odb.h\"\n #include \"odb/transaction.h\"\n@@ -23,9 +22,9 @@\n #include \"diff.h\"\n #include \"read-cache.h\"\n #include \"revision.h\"\n-#include \"strvec.h\"\n #include \"submodule.h\"\n #include \"add-interactive.h\"\n+#include \"apply.h\"\n\n static const char * const builtin_add_usage[] = {\n \tN_(\"git add [<options>] [--] <pathspec>...\"),\n@@ -187,7 +186,6 @@ static int edit_patch(struct repository *repo,\n \t\t      const char *prefix)\n {\n \tchar *file = repo_git_path(repo, \"ADD_EDIT.patch\");\n-\tstruct child_process child = CHILD_PROCESS_INIT;\n \tstruct rev_info rev;\n \tint out;\n \tstruct stat st;\n@@ -217,11 +215,17 @@ static int edit_patch(struct repository *repo,\n \tif (!st.st_size)\n \t\tdie(_(\"empty patch. aborted\"));\n\n-\tchild.git_cmd = 1;\n-\tstrvec_pushl(&child.args, \"apply\", \"--recount\", \"--cached\", file,\n-\t\t     NULL);\n-\tif (run_command(&child))\n+\tstruct apply_state state;\n+\tconst char *apply_argv[] = { file, NULL };\n+\n+\tif (init_apply_state(&state, repo, prefix))\n+\t\tdie(_(\"could not initialize apply state\"));\n+\tstate.cached = 1;\n+\tif (check_apply_state(&state, 0))\n+\t\tdie(_(\"could not check apply state\"));\n+\tif (apply_all_patches(&state, 1, apply_argv, APPLY_OPT_RECOUNT))\n \t\tdie(_(\"could not apply '%s'\"), file);\n+\tclear_apply_state(&state);\n\n \tunlink(file);\n \tfree(file);\n--\n2.54.0\n\n"},{"id":"547793","messageId":"xmqqechad6g9.fsf@gitster.g","threadId":"65962","inReplyTo":"20260710074105.50737-1-gatlavishweshwarreddy26@gmail.com","subject":"Re: [PATCH v2] builtin/add.c: replace run_command() with direct apply_all_patches() call","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-10T18:51:02Z","receivedAt":"2026-07-10T18:51:05Z","isPatch":true,"body":"Gatla Vishweshwar Reddy <gatlavishweshwarreddy26@gmail.com> writes:\n\n> @@ -187,7 +186,6 @@ static int edit_patch(struct repository *repo,\n>  \t\t      const char *prefix)\n>  {\n>  \tchar *file = repo_git_path(repo, \"ADD_EDIT.patch\");\n> -\tstruct child_process child = CHILD_PROCESS_INIT;\n>  \tstruct rev_info rev;\n>  \tint out;\n>  \tstruct stat st;\n> @@ -217,11 +215,17 @@ static int edit_patch(struct repository *repo,\n>  \tif (!st.st_size)\n>  \t\tdie(_(\"empty patch. aborted\"));\n>\n> -\tchild.git_cmd = 1;\n> -\tstrvec_pushl(&child.args, \"apply\", \"--recount\", \"--cached\", file,\n> -\t\t     NULL);\n> -\tif (run_command(&child))\n> +\tstruct apply_state state;\n> +\tconst char *apply_argv[] = { file, NULL };\n\nThese are -Wdeclaration-after-statement violations; we should move\nthem to the beginning of the function alongside the other variable\ndeclarations.\n\n> +\n> +\tif (init_apply_state(&state, repo, prefix))\n> +\t\tdie(_(\"could not initialize apply state\"));\n> +\tstate.cached = 1;\n> +\tif (check_apply_state(&state, 0))\n> +\t\tdie(_(\"could not check apply state\"));\n> +\tif (apply_all_patches(&state, 1, apply_argv, APPLY_OPT_RECOUNT))\n>  \t\tdie(_(\"could not apply '%s'\"), file);\n> +\tclear_apply_state(&state);\n\nDoes it work properly when run in a subdirectory, such as \"cd t &&\ngit add -e\")?  The apply_all_patches() function adjusts the path to\npatch files by calling prefix_filename() to prepend state->prefix,\nwhich represents our current directory.\n\nThis is not a rhetorical question, as I am unsure what \"file\"\nactually holds at this point after calling repo_git_path().  I don't\nknow if it is ADD_EDIT.patch relative to a specific directory, an\nabsolute path to the file, or something else entirely.  It would be\nhighly beneficial to include a test or two verifying the behavour of\n'add -e' from within a subdirectory.\n\nThanks.\n"},{"id":"547795","messageId":"20260710195949.54928-1-gatlavishweshwarreddy26@gmail.com","threadId":"65962","inReplyTo":"xmqqechad6g9.fsf@gitster.g","subject":"[PATCH v3] builtin/add.c: replace run_command() with direct apply_all_patches() call","fromName":"Gatla Vishweshwar Reddy","fromEmail":"gatlavishweshwarreddy26@gmail.com","sentAt":"2026-07-10T19:58:20Z","receivedAt":"2026-07-10T19:59:57Z","isPatch":true,"body":"When the user runs \"git add -e\", the diff of the working tree changes\nis written to a temporary file, opened in an editor, and then applied\nback to the index. The application step is done by spawning a child\nprocess running \"git apply --recount --cached <file>\", which is an\nunnecessary subprocess since the apply machinery is available as a\nnative C API.\n\nReplace the run_command() call with a direct call to apply_all_patches()\nusing an initialized apply_state with the cached and recount options set\nappropriately. This avoids the overhead of forking a subprocess, keeps\nthe operation within the same process, and makes the intent of the code\nclearer to the reader.\n\nRemove the now-unused includes of \"run-command.h\" and \"strvec.h\" since\nno other code in this file requires them after this change.\n\nSigned-off-by: Gatla Vishweshwar Reddy <gatlavishweshwarreddy26@gmail.com>\n---\n\nChanges in v3:\n- Moved struct apply_state and apply_argv declarations to the top of\n  the function to fix -Wdeclaration-after-statement violations\n\nIn response to review:\n- repo_git_path() returns an absolute path built from gitdir.\n  prefix_filename() in apply_all_patches() explicitly skips absolute\n  paths (see abspath.c lines 271-272 where is_absolute_path(arg)\n  causes the prefix to be skipped). Running \"git add -e\" from a\n  subdirectory is therefore safe.\n- A dedicated test for \"git add -e\" from a subdirectory would be\n  valuable. I looked but found no existing \"add -e\" tests in the test\n  suite to use as a reference. I would appreciate guidance on the\n  preferred approach, or I can attempt to write one if you can point\n  me to a similar test pattern.\n\n builtin/add.c | 19 ++++++++++++-------\n 1 file changed, 12 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex c859f66519..1858adf289 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -13,7 +13,6 @@\n #include \"dir.h\"\n #include \"gettext.h\"\n #include \"pathspec.h\"\n-#include \"run-command.h\"\n #include \"object-file.h\"\n #include \"odb.h\"\n #include \"odb/transaction.h\"\n@@ -23,9 +22,9 @@\n #include \"diff.h\"\n #include \"read-cache.h\"\n #include \"revision.h\"\n-#include \"strvec.h\"\n #include \"submodule.h\"\n #include \"add-interactive.h\"\n+#include \"apply.h\"\n\n static const char * const builtin_add_usage[] = {\n \tN_(\"git add [<options>] [--] <pathspec>...\"),\n@@ -187,7 +186,8 @@ static int edit_patch(struct repository *repo,\n \t\t      const char *prefix)\n {\n \tchar *file = repo_git_path(repo, \"ADD_EDIT.patch\");\n-\tstruct child_process child = CHILD_PROCESS_INIT;\n+\tstruct apply_state state;\n+\tconst char *apply_argv[2];\n \tstruct rev_info rev;\n \tint out;\n \tstruct stat st;\n@@ -217,11 +217,16 @@ static int edit_patch(struct repository *repo,\n \tif (!st.st_size)\n \t\tdie(_(\"empty patch. aborted\"));\n\n-\tchild.git_cmd = 1;\n-\tstrvec_pushl(&child.args, \"apply\", \"--recount\", \"--cached\", file,\n-\t\t     NULL);\n-\tif (run_command(&child))\n+\tapply_argv[0] = file;\n+\tapply_argv[1] = NULL;\n+\tif (init_apply_state(&state, repo, prefix))\n+\t\tdie(_(\"could not initialize apply state\"));\n+\tstate.cached = 1;\n+\tif (check_apply_state(&state, 0))\n+\t\tdie(_(\"could not check apply state\"));\n+\tif (apply_all_patches(&state, 1, apply_argv, APPLY_OPT_RECOUNT))\n \t\tdie(_(\"could not apply '%s'\"), file);\n+\tclear_apply_state(&state);\n\n \tunlink(file);\n \tfree(file);\n--\n2.54.0\n\n"},{"id":"547810","messageId":"xmqqechab03t.fsf@gitster.g","threadId":"65962","inReplyTo":"20260710195949.54928-1-gatlavishweshwarreddy26@gmail.com","subject":"Re: [PATCH v3] builtin/add.c: replace run_command() with direct apply_all_patches() call","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-11T04:51:02Z","receivedAt":"2026-07-11T04:51:05Z","isPatch":true,"body":"Gatla Vishweshwar Reddy <gatlavishweshwarreddy26@gmail.com> writes:\n\n> In response to review:\n> - repo_git_path() returns an absolute path built from gitdir.\n>   prefix_filename() in apply_all_patches() explicitly skips absolute\n>   paths (see abspath.c lines 271-272 where is_absolute_path(arg)\n>   causes the prefix to be skipped). Running \"git add -e\" from a\n>   subdirectory is therefore safe.\n\nI agree that we are safe when it is absolute (no room for prefix to\ntake part); my question was more about repo_git_path() that derives\nits value from repo->gitdir which may or may not be absolute.\n\nDoes it always give you absolute, or sometimes it is relative and\nsometimes it is absolute?\n\n> - A dedicated test for \"git add -e\" from a subdirectory would be\n>   valuable. I looked but found no existing \"add -e\" tests in the test\n>   suite to use as a reference.\n\n\"git grep -e 'add -e' t/\" finds t3702.\n\n"},{"id":"547811","messageId":"20260711061246.58079-1-gatlavishweshwarreddy26@gmail.com","threadId":"65962","inReplyTo":"xmqqechab03t.fsf@gitster.g","subject":"[PATCH v4] builtin/add.c: replace run_command() with direct apply_all_patches() call","fromName":"Gatla Vishweshwar Reddy","fromEmail":"gatlavishweshwarreddy26@gmail.com","sentAt":"2026-07-11T06:06:15Z","receivedAt":"2026-07-11T06:13:03Z","isPatch":true,"body":"When the user runs \"git add -e\", the diff of the working tree changes\nis written to a temporary file, opened in an editor, and then applied\nback to the index. The application step is done by spawning a child\nprocess running \"git apply --recount --cached <file>\", which is an\nunnecessary subprocess since the apply machinery is available as a\nnative C API.\n\nReplace the run_command() call with a direct call to apply_all_patches()\nusing an initialized apply_state with the cached and recount options set\nappropriately. This avoids the overhead of forking a subprocess, keeps\nthe operation within the same process, and makes the intent of the code\nclearer to the reader.\n\nRemove the now-unused includes of \"run-command.h\" and \"strvec.h\" since\nno other code in this file requires them after this change.\n\nSigned-off-by: Gatla Vishweshwar Reddy <gatlavishweshwarreddy26@gmail.com>\n---\n\n---\nChanges in v4:\n- Pass NULL instead of prefix to init_apply_state() since the file\n  path from repo_git_path() is a git-internal path that should not\n  be prefixed. This is safe regardless of whether repo->gitdir is\n  absolute or relative, as prefix_filename(NULL, arg) returns the\n  path unchanged (abspath.c line 269).\n- Add a test in t3702-add-edit.sh verifying that \"git add -e\" works\n  correctly when run from a subdirectory.\n- Tested with t3702-add-edit.sh: all 4 tests pass.\n\nIn response to review:\n- You are right that repo->gitdir may not always be absolute\n  (setup.c line 1109). Passing NULL as prefix to init_apply_state()\n  avoids the issue entirely — prefix_filename(NULL, arg) sets\n  pfx_len=0 and returns the path unchanged regardless of whether\n  it is absolute or relative.\n\n- t3702-add-edit.sh was found via \"git grep -e 'add -e' t/\" as\n  suggested. A new test using GIT_EDITOR=cat verifies that\n  \"git add -e\" works correctly from a subdirectory.\n\n builtin/add.c       | 19 ++++++++++++-------\n t/t3702-add-edit.sh | 10 ++++++++++\n 2 files changed, 22 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex c859f66519..20a86a1611 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -13,7 +13,6 @@\n #include \"dir.h\"\n #include \"gettext.h\"\n #include \"pathspec.h\"\n-#include \"run-command.h\"\n #include \"object-file.h\"\n #include \"odb.h\"\n #include \"odb/transaction.h\"\n@@ -23,9 +22,9 @@\n #include \"diff.h\"\n #include \"read-cache.h\"\n #include \"revision.h\"\n-#include \"strvec.h\"\n #include \"submodule.h\"\n #include \"add-interactive.h\"\n+#include \"apply.h\"\n\n static const char * const builtin_add_usage[] = {\n \tN_(\"git add [<options>] [--] <pathspec>...\"),\n@@ -187,7 +186,8 @@ static int edit_patch(struct repository *repo,\n \t\t      const char *prefix)\n {\n \tchar *file = repo_git_path(repo, \"ADD_EDIT.patch\");\n-\tstruct child_process child = CHILD_PROCESS_INIT;\n+\tstruct apply_state state;\n+\tconst char *apply_argv[2];\n \tstruct rev_info rev;\n \tint out;\n \tstruct stat st;\n@@ -217,11 +217,16 @@ static int edit_patch(struct repository *repo,\n \tif (!st.st_size)\n \t\tdie(_(\"empty patch. aborted\"));\n\n-\tchild.git_cmd = 1;\n-\tstrvec_pushl(&child.args, \"apply\", \"--recount\", \"--cached\", file,\n-\t\t     NULL);\n-\tif (run_command(&child))\n+\tapply_argv[0] = file;\n+\tapply_argv[1] = NULL;\n+\tif (init_apply_state(&state, repo, NULL))\n+\t\tdie(_(\"could not initialize apply state\"));\n+\tstate.cached = 1;\n+\tif (check_apply_state(&state, 0))\n+\t\tdie(_(\"could not check apply state\"));\n+\tif (apply_all_patches(&state, 1, apply_argv, APPLY_OPT_RECOUNT))\n \t\tdie(_(\"could not apply '%s'\"), file);\n+\tclear_apply_state(&state);\n\n \tunlink(file);\n \tfree(file);\ndiff --git a/t/t3702-add-edit.sh b/t/t3702-add-edit.sh\nindex 8bacacbac6..f628564005 100755\n--- a/t/t3702-add-edit.sh\n+++ b/t/t3702-add-edit.sh\n@@ -124,5 +124,15 @@ test_expect_success 'add -e notices editor failure' '\n \ttest_must_fail env GIT_EDITOR=false git add -e &&\n \ttest_expect_code 1 git diff --exit-code\n '\n+test_expect_success 'add -e works from a subdirectory' '\n+\tgit reset --hard &&\n+\techo change >>file &&\n+\tmkdir -p subdir &&\n+\t(\n+\t\tcd subdir &&\n+\t\tGIT_EDITOR=cat git add -e ../file\n+\t) &&\n+\tgit diff --cached | grep -q \"^+change\"\n+'\n\n test_done\n--\n2.54.0\n\n"},{"id":"549238","messageId":"xmqq8q6to4em.fsf@gitster.g","threadId":"65962","inReplyTo":"20260711061246.58079-1-gatlavishweshwarreddy26@gmail.com","subject":"Re: [PATCH v4] builtin/add.c: replace run_command() with direct apply_all_patches() call","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-29T21:46:25Z","receivedAt":"2026-07-29T21:46:27Z","isPatch":true,"body":"Gatla Vishweshwar Reddy <gatlavishweshwarreddy26@gmail.com> writes:\n\n> When the user runs \"git add -e\", the diff of the working tree changes\n> is written to a temporary file, opened in an editor, and then applied\n> back to the index. The application step is done by spawning a child\n> process running \"git apply --recount --cached <file>\", which is an\n> unnecessary subprocess since the apply machinery is available as a\n> native C API.\n>\n> Replace the run_command() call with a direct call to apply_all_patches()\n> using an initialized apply_state with the cached and recount options set\n> appropriately. This avoids the overhead of forking a subprocess, keeps\n> the operation within the same process, and makes the intent of the code\n> clearer to the reader.\n>\n> Remove the now-unused includes of \"run-command.h\" and \"strvec.h\" since\n> no other code in this file requires them after this change.\n>\n> Signed-off-by: Gatla Vishweshwar Reddy <gatlavishweshwarreddy26@gmail.com>\n> ---\n>\n> ---\n> Changes in v4:\n> - Pass NULL instead of prefix to init_apply_state() since the file\n>   path from repo_git_path() is a git-internal path that should not\n>   be prefixed. This is safe regardless of whether repo->gitdir is\n>   absolute or relative, as prefix_filename(NULL, arg) returns the\n>   path unchanged (abspath.c line 269).\n> - Add a test in t3702-add-edit.sh verifying that \"git add -e\" works\n>   correctly when run from a subdirectory.\n> - Tested with t3702-add-edit.sh: all 4 tests pass.\n\nNow the way \"apply\" API is used in this new code path should be\npretty much parallel to existing \"git apply\" and \"git am\" code\npaths, we should be fine.  I do not use \"git add -e\", but those who\ndo who may care more more deeply about keeping this feature working\nthan I do may want to lend an extra pair of eyes on this patch.\n\nThanks.\n\n> diff --git a/builtin/add.c b/builtin/add.c\n> index c859f66519..20a86a1611 100644\n> --- a/builtin/add.c\n> +++ b/builtin/add.c\n> @@ -13,7 +13,6 @@\n>  #include \"dir.h\"\n>  #include \"gettext.h\"\n>  #include \"pathspec.h\"\n> -#include \"run-command.h\"\n>  #include \"object-file.h\"\n>  #include \"odb.h\"\n>  #include \"odb/transaction.h\"\n> @@ -23,9 +22,9 @@\n>  #include \"diff.h\"\n>  #include \"read-cache.h\"\n>  #include \"revision.h\"\n> -#include \"strvec.h\"\n>  #include \"submodule.h\"\n>  #include \"add-interactive.h\"\n> +#include \"apply.h\"\n>\n>  static const char * const builtin_add_usage[] = {\n>  \tN_(\"git add [<options>] [--] <pathspec>...\"),\n> @@ -187,7 +186,8 @@ static int edit_patch(struct repository *repo,\n>  \t\t      const char *prefix)\n>  {\n>  \tchar *file = repo_git_path(repo, \"ADD_EDIT.patch\");\n> -\tstruct child_process child = CHILD_PROCESS_INIT;\n> +\tstruct apply_state state;\n> +\tconst char *apply_argv[2];\n>  \tstruct rev_info rev;\n>  \tint out;\n>  \tstruct stat st;\n> @@ -217,11 +217,16 @@ static int edit_patch(struct repository *repo,\n>  \tif (!st.st_size)\n>  \t\tdie(_(\"empty patch. aborted\"));\n>\n> -\tchild.git_cmd = 1;\n> -\tstrvec_pushl(&child.args, \"apply\", \"--recount\", \"--cached\", file,\n> -\t\t     NULL);\n> -\tif (run_command(&child))\n> +\tapply_argv[0] = file;\n> +\tapply_argv[1] = NULL;\n> +\tif (init_apply_state(&state, repo, NULL))\n> +\t\tdie(_(\"could not initialize apply state\"));\n> +\tstate.cached = 1;\n> +\tif (check_apply_state(&state, 0))\n> +\t\tdie(_(\"could not check apply state\"));\n> +\tif (apply_all_patches(&state, 1, apply_argv, APPLY_OPT_RECOUNT))\n>  \t\tdie(_(\"could not apply '%s'\"), file);\n> +\tclear_apply_state(&state);\n>\n>  \tunlink(file);\n>  \tfree(file);\n> diff --git a/t/t3702-add-edit.sh b/t/t3702-add-edit.sh\n> index 8bacacbac6..f628564005 100755\n> --- a/t/t3702-add-edit.sh\n> +++ b/t/t3702-add-edit.sh\n> @@ -124,5 +124,15 @@ test_expect_success 'add -e notices editor failure' '\n>  \ttest_must_fail env GIT_EDITOR=false git add -e &&\n>  \ttest_expect_code 1 git diff --exit-code\n>  '\n> +test_expect_success 'add -e works from a subdirectory' '\n> +\tgit reset --hard &&\n> +\techo change >>file &&\n> +\tmkdir -p subdir &&\n> +\t(\n> +\t\tcd subdir &&\n> +\t\tGIT_EDITOR=cat git add -e ../file\n> +\t) &&\n> +\tgit diff --cached | grep -q \"^+change\"\n> +'\n>\n>  test_done\n"},{"id":"551299","messageId":"xmqqbjaoiyzx.fsf@gitster.g","threadId":"65962","inReplyTo":"xmqq8q6to4em.fsf@gitster.g","subject":"Re: [PATCH v4] builtin/add.c: replace run_command() with direct apply_all_patches() call","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-26T17:16:02Z","receivedAt":"2026-08-26T17:16:06Z","isPatch":true,"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Now the way \"apply\" API is used in this new code path should be\n> pretty much parallel to existing \"git apply\" and \"git am\" code\n> paths, we should be fine.  I do not use \"git add -e\", but those who\n> do who may care more more deeply about keeping this feature working\n> than I do may want to lend an extra pair of eyes on this patch.\n\nAnd nobody seems to be interested in seeing this topic move forward,\nunfortunately.  After reading the patch again, I think this is safe\nand correct, and if I merge the topic, one of three things can\nhappen.\n\n (1) the patch does not regress anything unexpectedly, or\n\n (2) the patch breaks \"add -e\" completely but the feature is not\n     used by anybody and nobody will notice, or\n\n (3) the patch breaks \"add -e\" and the its users will start\n     complaining too late.\n\nReverting a merge would not be too involved as the patch is\nsmall-ish, so even in case (3) it won't be too much trouble to deal\nwith fallouts.  So let me mark the topic for 'next' for now.\n\nIt still is not too late for \"add -e\" users to interject, though.\n\nThanks.\n"}]}