git/userdiff.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

environment: guard state depending on a repository

2024-09-12T17:15:42Z

In "environment.h" we have quite a lot of functions and variables that either explicitly or implicitly depend on `the_repository`. The implicit set of stateful declarations includes for example variables which get populated when parsing a repository's Git configuration. This set of variables is broken by design, as their state often depends on the last repository config that has been parsed. So they may or may not represent the state of `the_repository`. Fixing that is quite a big undertaking, and later patches in this series will demonstrate a solution for a first small set of those variables. So for now, let's guard these with `USE_THE_REPOSITORY_VARIABLE` so that callers are aware of the implicit dependency. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

userdiff: fix leaking memory for configured diff drivers

2024-08-14T17:08:01Z

The userdiff structures may be initialized either statically on the stack or dynamically via configuration keys. In the latter case we end up leaking memory because we didn't have any infrastructure to discern those strings which have been allocated statically and those which have been allocated dynamically. Refactor the code such that we have two pointers for each of these strings: one that holds the value as accessed by other subsystems, and one that points to the same string in case it has been allocated. Like this, we can safely free the second pointer and thus plug those memory leaks. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

Merge branch 'rs/diff-exit-code-with-external-diff'

2024-06-20T22:45:08Z

"git diff --exit-code --ext-diff" learned to take the exit status of the external diff driver into account when deciding the exit status of the overall "git diff" invocation when configured to do so. * rs/diff-exit-code-with-external-diff: diff: let external diffs report that changes are uninteresting userdiff: add and use struct external_diff t4020: test exit code with external diffs

diff: let external diffs report that changes are uninteresting

2024-06-10T16:20:46Z

The options --exit-code and --quiet instruct git diff to indicate whether it found any significant changes by exiting with code 1 if it did and 0 if there were none. Currently this doesn't work if external diff programs are involved, as we have no way to learn what they found. Add that ability in the form of the new configuration options diff.trustExitCode and diff..trustExitCode and the environment variable GIT_EXTERNAL_DIFF_TRUST_EXIT_CODE. They pair with the config options diff.external and diff..command and the environment variable GIT_EXTERNAL_DIFF, respectively. The new options are off by default, keeping the old behavior. Enabling them indicates that the external diff returns exit code 1 if it finds significant changes and 0 if it doesn't, like diff(1). The name of the new options is taken from the git difftool and mergetool options of similar purpose. (There they enable passing on the exit code of a diff tool and to infer whether a merge done by a merge tool is successful.) The new feature sets the diff flag diff_from_contents in diff_setup_done() if we need the exit code and are allowed to call external diffs. This disables the optimization that avoids calling the program with --quiet. Add it back by skipping the call if the external diff is not able to report empty diffs. We can only do that check after evaluating the file-specific attributes in run_external_diff(). If we do run the external diff with --quiet, send its output to /dev/null. I considered checking the output of the external diff to check whether its empty. It was added as 11be65cfa4 (diff: fix --exit-code with external diff, 2024-05-05) and quickly reverted, as it does not work with external diffs that do not write to stdout. There's no reason why a graphical diff tool would even need to write anything there at all. I also considered using a non-zero exit code for empty diffs, which could be done without adding new configuration options. We'd need to disable the optimization that allows git diff --quiet to skip calling external diffs, though -- that might be quite surprising if graphical diff programs are involved. And assigning the opposite meaning of the exit codes compared to diff(1) and git diff --exit-code to the external diff can cause unnecessary confusion. Suggested-by: Phillip Wood Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano

userdiff: add and use struct external_diff

2024-06-10T16:19:20Z

Wrap the string specifying the external diff command in a new struct to simplify adding attributes, which the next patch will do. Make sure external_diff() still returns NULL if neither the environment variable GIT_EXTERNAL_DIFF nor the configuration option diff.external is set, to continue allowing its use in a boolean context. Use a designated initializer for the default builtin userdiff driver to adjust to the type change of the second struct member. Spelling out only the non-zero members improves readability as a nice side-effect. No functional change intended. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano

global: convert intentionally-leaking config strings to consts

2024-06-07T17:30:48Z

There are multiple cases where we intentionally leak config strings: - `struct gpg_format` is used to track programs that can be used for signing commits, either via gpg(1), gpgsm(1) or ssh-keygen(1). The user can override the commands via several config variables. As the array is populated once, only, and the struct memers are never written to or free'd. - `struct ll_merge_driver` is used to track merge drivers. Same as with the GPG format, these drivers are populated once and then reused. Its data is never written to or free'd, either. - `struct userdiff_funcname` and `struct userdiff_driver` can be configured via `diff..*` to add additional drivers. Again, these have a global lifetime and are never written to or free'd. All of these are intentionally kept alive and are never written to. Furthermore, all of these are being assigned both string constants in some places, and allocated strings in other places. This will cause warnings once we enable `-Wwrite-strings`, so let's mark the respective fields as `const char *` and cast away the constness when assigning those values. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

userdiff: better method/property matching for C#

2024-04-05T22:21:43Z

- Support multi-line methods by not requiring closing parenthesis. - Support multiple generics (comma was missing before). - Add missing `foreach`, `lock` and `fixed` keywords to skip over. - Remove `instanceof` keyword, which isn't C#. - Also detect non-method keywords not positioned at the start of a line. - Added tests; none existed before. The overall strategy is to focus more on what isn't expected for method/property definitions, instead of what is, but is fully optional. Signed-off-by: Steven Jeuris Acked-by: Johannes Sixt Signed-off-by: Junio C Hamano

Merge branch 'jk/textconv-cache-outside-repo-fix'

2024-03-05T17:44:42Z

The code incorrectly attempted to use textconv cache when asked, even when we are not running in a repository, which has been corrected. * jk/textconv-cache-outside-repo-fix: userdiff: skip textconv caching when not in a repository

userdiff: skip textconv caching when not in a repository

2024-02-26T17:40:55Z

The textconv caching system uses git-notes to store its cache entries. But if you're using "diff --no-index" outside of a repository, then obviously that isn't going to work. Since caching is just an optimization, it's OK for us to skip it. However, the current behavior is much worse: we call notes_cache_init() which tries to look up the ref, and the low-level ref code hits a BUG(), killing the program. Instead, we should notice before setting up the cache that it there's no repository, and just silently skip it. Reported-by: Paweł Dominiak Signed-off-by: Jeff King Signed-off-by: Junio C Hamano