git/builtin/diff-files.c, branch v2.45.4

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: drop useless "status" parameter from diff_result_code()

2023-08-21T22:33:24Z

Many programs use diff_result_code() to get a user-visible program exit code from a diff result (e.g., checking opts.found_changes if --exit-code was requested). This function also takes a "status" parameter, which seems at first glance that it could be used to propagate an error encountered when computing the diff. But it doesn't work that way: - negative values are passed through as-is, but are not appropriate as program exit codes - when --exit-code or --check is in effect, we _ignore_ the passed-in status completely. So a failed diff which did not have a chance to set opts.found_changes would erroneously report "success, no changes" instead of propagating the error. After recent cleanups, neither of these bugs is possible to trigger, as every caller just passes in "0". So rather than fixing them, we can simply drop the useless parameter instead. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

diff: drop useless return from run_diff_{files,index} functions

2023-08-21T22:33:24Z

Neither of these functions ever returns a value other than zero. Instead, they expect unrecoverable errors to exit immediately, and things like "--exit-code" are stored inside the diff_options struct to be handled later via diff_result_code(). Some callers do check the return values, but many don't bother. Let's drop the useless return values, which are misleading callers about how the functions work. This could be seen as a step in the wrong direction, as we might want to eventually "lib-ify" these to more cleanly return errors up the stack, in which case we'd have to add the return values back in. But there are some benefits to doing this now: 1. In the current code, somebody could accidentally add a "return -1" to one of the functions, which would be erroneously ignored by many callers. By removing the return code, the compiler can notice the mismatch and force the developer to decide what to do. Obviously the other option here is that we could start consistently checking the error code in every caller. But it would be dead code, and we wouldn't get any compile-time help in catching new cases. 2. It communicates the situation to callers, who may want to choose a different function. These functions are really thin wrappers for doing git-diff-files and git-diff-index within the process. But callers who care about recovering from an error here are probably better off using the underlying library functions, many of which do return errors. If somebody eventually wants to teach these functions to propagate errors, they'll have to switch back to returning a value, effectively reverting this patch. But at least then they will be starting with a level playing field: they know that they will need to inspect each caller to see how it should handle the error. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

diff-files: avoid negative exit value

2023-08-21T22:33:24Z

If loading the index fails, we print an error and then return "-1" from the function. But since this is a builtin, we end up with exit(-1), which produces odd results since program exit codes are unsigned. Because of integer conversion, it usually becomes 255, which is at least still an error, but values above 128 are usually interpreted as signal death. Since we know the program is exiting immediately, we can just replace the error return with a die(). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

diff.h: remove unnecessary include of oidset.h

2023-06-21T20:39:53Z

This also made it clear that several .c files depended upon various things that oidset included, but had omitted the direct #include for those headers. Add those now. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano

cache.h: remove this no-longer-used header

2023-06-21T20:39:53Z

Since this header showed up in some places besides just #include statements, update/clean-up/remove those other places as well. Note that compat/fsmonitor/fsm-path-utils-darwin.c previously got away with violating the rule that all files must start with an include of git-compat-util.h (or a short-list of alternate headers that happen to include it first). This change exposed the violation and caused it to stop building correctly; fix it by having it include git-compat-util.h first, as per policy. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano

preload-index.h: move declarations for preload-index.c from elsewhere

2023-06-21T20:39:53Z

We already have a preload-index.c file; move the declarations for the functions in that file into a new preload-index.h. These were previously split between cache.h and repository.h. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano

diff-files: integrate with sparse index

2023-05-09T21:26:36Z

Remove full index requirement for `git diff-files`. Refactor the ensure_expanded and ensure_not_expanded functions by introducing a common helper function, ensure_index_state. Add test to ensure the index is no expanded in `git diff-files`. The `p2000` tests demonstrate a ~96% execution time reduction for 'git diff-files' and a ~97% execution time reduction for 'git diff-files' for a file using a sparse index: Test before after ----------------------------------------------------------------------- 2000.94: git diff-files (full-v3) 0.09 0.08 -11.1% 2000.95: git diff-files (full-v4) 0.09 0.09 +0.0% 2000.96: git diff-files (sparse-v3) 0.52 0.02 -96.2% 2000.97: git diff-files (sparse-v4) 0.51 0.02 -96.1% 2000.98: git diff-files -- f2/f4/a (full-v3) 0.06 0.07 +16.7% 2000.99: git diff-files -- f2/f4/a (full-v4) 0.08 0.08 +0.0% 2000.100: git diff-files -- f2/f4/a (sparse-v3) 0.46 0.01 -97.8% 2000.101: git diff-files -- f2/f4/a (sparse-v4) 0.51 0.02 -96.1% Signed-off-by: Shuqi Liang Signed-off-by: Junio C Hamano

cocci: apply "pending" index-compatibility to some "builtin/*.c"

2022-11-21T03:06:15Z

Apply "index-compatibility.pending.cocci" rule to "builtin/*", but exclude those where we conflict with in-flight changes. As a result some of them end up using only "the_index", so let's have them use the more narrow "USE_THE_INDEX_VARIABLE" rather than "USE_THE_INDEX_COMPATIBILITY_MACROS". Manual changes not made by coccinelle, that were squashed in: * Whitespace-wrap argument lists for repo_hold_locked_index(), repo_read_index_preload() and repo_refresh_and_write_index(), in cases where the line became too long after the transformation. * Change "refresh_cache()" to "refresh_index()" in a comment in "builtin/update-index.c". * For those whose call was followed by perror(""), change it to perror(""), referring to the new function. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano

built-ins: consistently add "\n" between "usage" and options

2022-10-13T16:32:54Z

Change commands in the "diff" family and "rev-list" to separate the usage information and option listing with an empty line. In the case of "git diff -h" we did this already (but let's use a consistent "\n" pattern there), for the rest these are now consistent with how the parse_options() API would emit usage. As we'll see in a subsequent commit this also helps to make the "git -h" output more easily machine-readable, as we can assume that the usage information is separated from the options by an empty line. Note that "COMMON_DIFF_OPTIONS_HELP" starts with a "\n", so the seeming omission of a "\n" here is correct, the second one is provided by the macro. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano