git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2] builtin/add.c: replace run_command() with direct apply_all_patches() call

From
Junio C Hamano <gitster@pobox.com>
Date
Jul 10, 2026, 18:51 UTC
Message-ID
<xmqqechad6g9.fsf@gitster.g>
In-Reply-To
<20260710074105.50737-1-gatlavishweshwarreddy26@gmail.com>
Gatla Vishweshwar Reddy <gatlavishweshwarreddy26@gmail.com> writes:
Show 18 quoted lines
> @@ -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.

Show 9 quoted lines
> +
> +	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.
Previous: Gatla Vishweshwar ReddyNext: Gatla Vishweshwar Reddy
Message 4 of 9 in “builtin/add.c: replace run_command() with direct apply_all_patches() call”
  1. builtin/add.c: replace run_command() with direct apply_all_patches() callGatla Vishweshwar Reddy, Jul 9, 2026
  2. Junio C HamanoJul 10, 2026
  3. builtin/add.c: replace run_command() with direct apply_all_patches() callGatla Vishweshwar Reddy, Jul 10, 2026
  4. Junio C HamanoJul 10, 2026
  5. builtin/add.c: replace run_command() with direct apply_all_patches() callGatla Vishweshwar Reddy, Jul 10, 2026
  6. Junio C HamanoJul 11, 2026
  7. builtin/add.c: replace run_command() with direct apply_all_patches() callGatla Vishweshwar Reddy, Jul 11, 2026
  8. Junio C HamanoJul 29, 2026
  9. Junio C HamanoAug 26, 2026

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.