On Mon, Feb 26, 2024 at 08:56:09AM +0100, Paweł Dominiak wrote:
> But it seems like something git should handle on its own, so that diff
> would accommodate use in different circumstances with the same config.
Yes, definitely. Here's my patch with a commit message and a test.
-- >8 --
Subject: userdiff: skip textconv caching when not in a repository
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 <dominiak.pawel@xxxxxxxxx>
Signed-off-by: Jeff King <peff@xxxxxxxx>
---
t/t4042-diff-textconv-caching.sh | 22 ++++++++++++++++++++++
userdiff.c | 4 +++-
2 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/t/t4042-diff-textconv-caching.sh b/t/t4042-diff-textconv-caching.sh
index bf33aedf4b..8ebfa3c1be 100755
--- a/t/t4042-diff-textconv-caching.sh
+++ b/t/t4042-diff-textconv-caching.sh
@@ -118,4 +118,26 @@ test_expect_success 'log notes cache and still use cache for -p' '
git log --no-walk -p refs/notes/textconv/magic HEAD
'
+test_expect_success 'caching is silently ignored outside repo' '
+ mkdir -p non-repo &&
+ echo one >non-repo/one &&
+ echo two >non-repo/two &&
+ echo "* diff=test" >attr &&
+ test_expect_code 1 \
+ nongit git -c core.attributesFile="$PWD/attr" \
+ -c diff.test.textconv="tr a-z A-Z <" \
+ -c diff.test.cachetextconv=true \
+ diff --no-index one two >actual &&
+ cat >expect <<-\EOF &&
+ diff --git a/one b/two
+ index 5626abf..f719efd 100644
+ --- a/one
+ +++ b/two
+ @@ -1 +1 @@
+ -ONE
+ +TWO
+ EOF
+ test_cmp expect actual
+'
+
test_done
diff --git a/userdiff.c b/userdiff.c
index e399543823..fce3a31efa 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -3,6 +3,7 @@
#include "userdiff.h"
#include "attr.h"
#include "strbuf.h"
+#include "environment.h"
static struct userdiff_driver *drivers;
static int ndrivers;
@@ -460,7 +461,8 @@ struct userdiff_driver *userdiff_get_textconv(struct repository *r,
if (!driver->textconv)
return NULL;
- if (driver->textconv_want_cache && !driver->textconv_cache) {
+ if (driver->textconv_want_cache && !driver->textconv_cache &&
+ have_git_dir()) {
struct notes_cache *c = xmalloc(sizeof(*c));
struct strbuf name = STRBUF_INIT;
--
2.44.0.rc2.424.gbdbf4d014b
