git/builtin/diff-index.c, branch v2.48.2

global: mark code units that generate warnings with `-Wsign-compare`

2024-12-06T11:20:02Z

Mark code units that generate warnings with `-Wsign-compare`. This allows for a structured approach to get rid of all such warnings over time in a way that can be easily measured. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

Merge branch 'jc/pass-repo-to-builtins'

2024-09-23T17:35:09Z

The convention to calling into built-in command implementation has been updated to pass the repository, if known, together with the prefix value. * jc/pass-repo-to-builtins: add: pass in repo variable instead of global the_repository builtin: remove USE_THE_REPOSITORY for those without the_repository builtin: remove USE_THE_REPOSITORY_VARIABLE from builtin.h builtin: add a repository parameter for builtin functions

Merge branch 'jc/range-diff-lazy-setup'

2024-09-16T21:22:55Z

Code clean-up. * jc/range-diff-lazy-setup: remerge-diff: clean up temporary objdir at a central place remerge-diff: lazily prepare temporary objdir on demand

builtin: remove USE_THE_REPOSITORY_VARIABLE from builtin.h

2024-09-13T21:32:24Z

Instead of including USE_THE_REPOSITORY_VARIABLE by default on every builtin, remove it from builtin.h and add it to all the builtins that include builtin.h (by definition, that means all builtins/*.c). Also, remove the include statement for repository.h since it gets brought in through builtin.h. The next step will be to migrate each builtin from having to use the_repository. Signed-off-by: John Cai Signed-off-by: Junio C Hamano

builtin: add a repository parameter for builtin functions

2024-09-13T21:27:08Z

In order to reduce the usage of the global the_repository, add a parameter to builtin functions that will get passed a repository variable. This commit uses UNUSED on most of the builtin functions, as subsequent commits will modify the actual builtins to pass the repository parameter down. Signed-off-by: John Cai Signed-off-by: Junio C Hamano

diff-index: integrate with the sparse index

2024-08-22T16:29:14Z

The sparse index allows focusing the index data structure on the files present in the sparse-checkout, leaving only tree entries for directories not within the sparse-checkout. Each builtin needs a repository setting to indicate that it has been tested with the sparse index before Git will allow the index to be loaded into memory in its sparse form. This is a safety precaution. There are still some builtins that haven't been integrated due to the complexity of the integration and the lack of significant use. However, 'git diff-index' was neglected only because of initial data showing low usage. The diff machinery was already integrated and there is no more work to be done there but add some tests to be sure 'git diff-index' behaves as expected. For this purpose, we can follow the testing pattern used in 51ba65b5c35 (diff: enable and test the sparse index, 2021-12-06). One difference here is that we only verify that the sparse index case agrees with the full index case, but do not generate the expected output. The 'git diff' tests use the '--name-status' option to ease the creation of the expected output, but that's not an option for 'diff-index'. Since the underlying diff machinery is the same, a simple comparison is sufficient to give some coverage. Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano

remerge-diff: clean up temporary objdir at a central place

2024-08-09T22:42:40Z

After running a diff between two things, or a series of diffs while walking the history, the diff computation is concluded by a call to diff_result_code() to extract the exit status of the diff machinery. The function can work on "struct diffopt", but all the callers historically and currently pass "struct diffopt" that is embedded in the "struct rev_info" that is used to hold the remerge_diff bit and the remerge_objdir variable that points at the temporary object directory in use. Redefine diff_result_code() to take the whole "struct rev_info" to give it an access to these members related to remerge-diff, so that it can get rid of the temporary object directory for any and all callers that used the feature. We can lose the equivalent code to do so from the code paths for individual commands, diff-tree, diff, and log. Signed-off-by: Junio C Hamano

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