git/diff.c, branch v2.45.3

diff: add diff.srcPrefix and diff.dstPrefix configuration variables

2024-03-15T17:04:45Z

Allow the default prefixes "a/" and "b/" to be tweaked by the diff.srcPrefix and diff.dstPrefix configuration variables. Signed-off-by: Peter Hutterer Signed-off-by: Junio C Hamano

Merge branch 'jx/dirstat-parseopt-help'

2024-02-15T23:14:48Z

The mark-up of diff options has been updated to help translators. * jx/dirstat-parseopt-help: diff: mark param1 and param2 as placeholders

diff: mark param1 and param2 as placeholders

2024-02-14T17:29:10Z

Some l10n translators translated the parameters "files", "param1" and "param2" in the following message: "synonym for --dirstat=files,param1,param2..." Translating "param1" and "param2" is OK, but changing the parameter "files" is wrong. The parameters that are not meant to be used verbatim should be marked as placeholders, but the verbatim parameter not marked as a placeholder should be left as is. This change is a complement for commit 51e846e673 (doc: enforce placeholders in documentation, 2023-12-25). With the help of Jean-Noël,some parameter combinations in one placeholder (e.g. "...") are splited into seperate placeholders. Helped-by: Jean-Noël Avila Signed-off-by: Jiang Xin Signed-off-by: Junio C Hamano

Merge branch 'jk/diff-external-with-no-index'

2024-02-06T22:31:21Z

"git diff --no-index file1 file2" segfaulted while invoking the external diff driver, which has been corrected. * jk/diff-external-with-no-index: diff: handle NULL meta-info when spawning external diff

diff: handle NULL meta-info when spawning external diff

2024-01-29T18:37:44Z

Running this: $ touch foo bar $ chmod +x foo $ git -c diff.external=echo diff --ext-diff --no-index foo bar results in a segfault. The issue is that run_diff_cmd() passes a NULL "xfrm_msg" variable to run_external_diff(), which feeds it to strvec_push(), causing the segfault. The bug dates back to 82fbf269b9 (run_external_diff: use an argv_array for the command line, 2014-04-19), though it mostly only ever worked accidentally. Before then, we just stuck the NULL pointer into a "const char **" array, so our NULL ended up acting as an extra end-of-argv sentinel (which was OK, because it was the last thing in the array). Curiously, though, this is only a problem with --no-index. We set up xfrm_msg by calling fill_metainfo(). This result may be empty, or may have text like "index 1234..5678\n", "rename from foo\nrename from bar\n", etc. In run_external_diff(), we only look at xfrm_msg if the "other" variable is not NULL. That variable is set when the paths of the two sides of the diff pair aren't the same (in which case the destination path becomes "other"). So normally it would kick in only for a rename, in which case xfrm_msg should not be NULL (it would have the rename information in it). But with a "--no-index" of two blobs, we of course have two different pathnames, and thus end up with a non-NULL "other" filename (which is always just a repeat of the file2-name), but possibly a NULL xfrm_msg. So how to fix it? I have a feeling that --no-index always passing "other" to the external diff command is probably a bug. There was no rename, and the name is always redundant with existing information we pass (and this may even cause us to pass a useless "xfrm_msg" that contains an "index 1234..5678" line). So one option would be to change that behavior. We don't seem to have ever documented the "other" or "xfrm_msg" parameters for external diffs. But I'm not sure what fallout we might have from changing that behavior now. So this patch takes the less-risky option, and simply teaches run_external_diff() to avoid passing xfrm_msg when it's NULL. That makes it agnostic to whether "other" and "xfrm_msg" always come as a pair. It fixes the segfault now, and if we want to change the --no-index "other" behavior on top, it will handle that, too. Reported-by: Wilfred Hughes Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

Merge branch 'en/header-cleanup'

2024-01-08T22:05:15Z

Remove unused header "#include". * en/header-cleanup: treewide: remove unnecessary includes in source files treewide: add direct includes currently only pulled in transitively trace2/tr2_tls.h: remove unnecessary include submodule-config.h: remove unnecessary include pkt-line.h: remove unnecessary include line-log.h: remove unnecessary include http.h: remove unnecessary include fsmonitor--daemon.h: remove unnecessary includes blame.h: remove unnecessary includes archive.h: remove unnecessary include treewide: remove unnecessary includes in source files treewide: remove unnecessary includes from header files

treewide: remove unnecessary includes in source files

2023-12-26T20:04:31Z

Each of these were checked with gcc -E -I. ${SOURCE_FILE} | grep ${HEADER_FILE} to ensure that removing the direct inclusion of the header actually resulted in that header no longer being included at all (i.e. that no other header pulled it in transitively). ...except for a few cases where we verified that although the header was brought in transitively, nothing from it was directly used in that source file. These cases were: * builtin/credential-cache.c * builtin/pull.c * builtin/send-pack.c Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano

diff: give more detailed messages for bogus diff.* config

2023-12-08T23:26:22Z

The config callbacks for a few diff.* variables simply return -1 when we encounter an error. The message you get mentions the offending location, like: fatal: bad config variable 'diff.algorithm' in file '.git/config' at line 7 but is vague about "bad" (as it must be, since the message comes from the generic config code). Most callbacks add their own messages here, so let's do the same. E.g.: error: unknown value for config 'diff.algorithm': foo fatal: bad config variable 'diff.algorithm' in file '.git/config' at line 7 I've written the string in a way that should be reusable for translators, and matches another similar message in transport.c (there doesn't yet seem to be a popular generic message to reuse here, so hopefully this will get the ball rolling). Note that in the case of diff.algorithm, our parse_algorithm_value() helper does detect a NULL value string. But it's still worth detecting it ourselves here, since we can give a more specific error message (and which is the usual one for unexpected implicit-bool values). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

config: handle NULL value when parsing non-bools

2023-12-08T23:24:39Z

When the config parser sees an "implicit" bool like: [core] someVariable it passes NULL to the config callback. Any callback code which expects a string must check for NULL. This usually happens via helpers like git_config_string(), etc, but some custom code forgets to do so and will segfault. These are all fairly vanilla cases where the solution is just the usual pattern of: if (!value) return config_error_nonbool(var); though note that in a few cases we have to split initializers like: int some_var = initializer(); into: int some_var; if (!value) return config_error_nonbool(var); some_var = initializer(); There are still some broken instances after this patch, which I'll address on their own in individual patches after this one. Reported-by: Carlos Andrés Ramírez Cataño Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

Merge branch 'jk/diff-result-code-cleanup' into maint-2.42

2023-11-02T07:53:16Z

"git diff --no-such-option" and other corner cases around the exit status of the "diff" command has been corrected. * jk/diff-result-code-cleanup: diff: drop useless "status" parameter from diff_result_code() diff: drop useless return values in git-diff helpers diff: drop useless return from run_diff_{files,index} functions diff: die when failing to read index in git-diff builtin diff: show usage for unknown builtin_diff_files() options diff-files: avoid negative exit value diff: spell DIFF_INDEX_CACHED out when calling run_diff_index()