From: Junio C Hamano Date: Fri, 10 Jul 2026 18:51:02 GMT Subject: Re: [PATCH v2] builtin/add.c: replace run_command() with direct apply_all_patches() call Message-ID: In-Reply-To: <20260710074105.50737-1-gatlavishweshwarreddy26@gmail.com> Gatla Vishweshwar Reddy writes: > @@ -187,7 +186,6 @@ static int edit_patch(struct repository *repo, > const char *prefix) > { > char *file = repo_git_path(repo, "ADD_EDIT.patch"); > - struct child_process child = CHILD_PROCESS_INIT; > struct rev_info rev; > int out; > struct stat st; > @@ -217,11 +215,17 @@ static int edit_patch(struct repository *repo, > if (!st.st_size) > die(_("empty patch. aborted")); > > - child.git_cmd = 1; > - strvec_pushl(&child.args, "apply", "--recount", "--cached", file, > - NULL); > - if (run_command(&child)) > + struct apply_state state; > + const char *apply_argv[] = { file, NULL }; These are -Wdeclaration-after-statement violations; we should move them to the beginning of the function alongside the other variable declarations. > + > + if (init_apply_state(&state, repo, prefix)) > + die(_("could not initialize apply state")); > + state.cached = 1; > + if (check_apply_state(&state, 0)) > + die(_("could not check apply state")); > + if (apply_all_patches(&state, 1, apply_argv, APPLY_OPT_RECOUNT)) > die(_("could not apply '%s'"), file); > + clear_apply_state(&state); Does it work properly when run in a subdirectory, such as "cd t && git add -e")? The apply_all_patches() function adjusts the path to patch files by calling prefix_filename() to prepend state->prefix, which represents our current directory. This is not a rhetorical question, as I am unsure what "file" actually holds at this point after calling repo_git_path(). I don't know if it is ADD_EDIT.patch relative to a specific directory, an absolute path to the file, or something else entirely. It would be highly beneficial to include a test or two verifying the behavour of 'add -e' from within a subdirectory. Thanks.