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

Re: [PATCH v5 2/3] difftool: create a tmpdir path without repeated slashes

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Sep 24, 2021, 10:35 UTC
Message-ID
<871r5e45jw.fsf@evledraar.gmail.com>
In-Reply-To
<20210923041252.52596-3-davvid@gmail.com>
On Wed, Sep 22 2021, David Aguilar wrote:
Show 9 quoted lines
> @@ -580,7 +574,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
>  		flags = 0;
>  	} else
>  		setenv("GIT_DIFFTOOL_DIRDIFF", "true", 1);
> -	rc = run_command_v_opt(helper_argv, flags);
> +	ret = run_command_v_opt(helper_argv, flags);
>  
>  	/* TODO: audit for interaction with sparse-index. */
>  	ensure_full_index(&wtindex);

Just on this part: There was already a logic error in the pre-image where we'd return error() return values up to cmd_main() et al. This means that (on POSIX) we return 255, not 1, per the C standard doing this is "implementation defined".

I think you want the "rc" variable back, and either in another commit or in some other cleanup this on top or ahead of this patch:

diff --git a/builtin/difftool.c b/builtin/difftool.c
index a867c49067c..6605d17e6bc 100644
--- a/builtin/difftool.c
+++ b/builtin/difftool.c
@@ -345,6 +345,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
 	const char *helper_argv[] = { "difftool--helper", NULL, NULL, NULL };
 	struct hashmap wt_modified, tmp_modified;
 	int indices_loaded = 0;
+	int rc;
 
 	workdir = get_git_work_tree();
 
@@ -574,7 +575,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
 		flags = 0;
 	} else
 		setenv("GIT_DIFFTOOL_DIRDIFF", "true", 1);
-	ret = run_command_v_opt(helper_argv, flags);
+	rc = run_command_v_opt(helper_argv, flags);
 
 	/* TODO: audit for interaction with sparse-index. */
 	ensure_full_index(&wtindex);
@@ -660,7 +661,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
 	strbuf_release(&tmpdir);
 	UNLEAK(working_tree_dups);
 
-	return ret;
+	return ret < 0 ? 1 : rc;
 }
 
 static int run_file_diff(int prompt, const char *prefix,
Previous: David AguilarNext: David Aguilar
Message 6 of 9 in “difftool dir-diff symlink bug fix and cleanup patches”
  1. 0/3 difftool dir-diff symlink bug fix and cleanup patchesDavid Aguilar, Sep 23, 2021
  2. 1/3 difftool: fix symlink-file writing in dir-diff modeDavid Aguilar, Sep 23, 2021
  3. Junio C HamanoSep 23, 2021
  4. David AguilarSep 30, 2021
  5. 2/3 difftool: create a tmpdir path without repeated slashesDavid Aguilar, Sep 23, 2021
  6. Ævar Arnfjörð BjarmasonSep 24, 2021
  7. David AguilarSep 30, 2021
  8. 3/3 difftool: add a missing space to the run_dir_diff() commentsDavid Aguilar, Sep 23, 2021
  9. Junio C HamanoSep 23, 2021

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.