Repository: pre-commit/pre-commit

Issue:
Binary file change by hook not detected due to Git textconv
### search you tried in the issue tracker

textconv

### describe your issue

I’ve set up Git to use `exiftool` as a text conversion filter for JPEGs, with this in `~/.config/git/config`:

```ini
[diff "exiftool"]
    textconv = exiftool --composite -x 'Exiftool:*' -x 'File:*' -g0
    cachetextconv = true
    xfuncname = "^-.*$"
```

…and this in `~/.config/git/attributes`:

```
*.jpeg diff=exiftool
```

In my repo, I’m using `jpegoptim` to optimize JPEG's with the below `.pre-commit-config.yaml`.

Take an unoptimized JPEG:

```console
$ git status
On branch main
Changes to be committed:
        new file:   donut.jpeg
```

Running pre-commit, the hook passes:

```console
$ pre-commit run jpegoptim
Optimize JPEGs...........................................................Passed
```

(There’s no option to make `jpegoptim` exit with a non-zero status code when it changes files.)

However, the tool did actually modify the image:

```console
$ git status
On branch main
Changes to be committed:
        new file:   donut.jpeg

Changes not staged for commit:
        modified:   donut.jpeg
```

This behaviour means commits don't include the optimized version of a flie, instead they’re left around unstaged.

pre-commit doesn't detect the change because the text conversions of the binary file before/after are the same, leading to an empty diff:

```
$ git diff donut.jpeg
```

But using [`--no-textconv`](https://git-scm.com/docs/git-diff#Documentation/git-diff.txt---no-textconv) shows the tool actually made a change:

$ git diff --no-textconv donut.jpeg |cat
diff --git a/donut.jpeg b/donut.jpeg
index 3bd0743..d66a2f4 100644
Binary files a/donut.jpeg and b/donut.jpeg differ
```

The `git diff` call that pre-commit uses to check for changes, in [`_git_diff()`](https://github.com/pre-commit/pre-commit/blob/e846829992a84ce8066e6513a72a357709eec56c/pre_commit/commands/run.py#L273C6-L277) already uses `--no-ext-diff` to make diffs reproducible
I think it should also use `--no-textconv`.
This would also make it a little faster.

### pre-commit --version

3.0.3 / main

### .pre-commit-config.yaml

```yaml
repos:
-   repo: local
    hooks:
    -   id: jpegoptim
        name: Optimize JPEGs
        entry: jpegoptim
        language: system
        types: [jpeg]
```


### ~/.cache/pre-commit/pre-commit.log (if present)

_No response_

Discussion:

Patch:
diff --git a/pre_commit/commands/run.py b/pre_commit/commands/run.py
index e44e703..a7eb4f4 100644
--- a/pre_commit/commands/run.py
+++ b/pre_commit/commands/run.py
@@ -272,7 +272,8 @@ def _all_filenames(args: argparse.Namespace) -> Collection[str]:
 
 def _get_diff() -> bytes:
     _, out, _ = cmd_output_b(
-        'git', 'diff', '--no-ext-diff', '--ignore-submodules', check=False,
+        'git', 'diff', '--no-ext-diff', '--no-textconv', '--ignore-submodules',
+        check=False,
     )
     return out
 
@@ -326,8 +327,7 @@ def _has_unmerged_paths() -> bool:
 
 def _has_unstaged_config(config_file: str) -> bool:
     retcode, _, _ = cmd_output_b(
-        'git', 'diff', '--no-ext-diff', '--exit-code', config_file,
-        check=False,
+        'git', 'diff', '--quiet', '--no-ext-diff', config_file, check=False,
     )
     # be explicit, other git errors don't mean it has an unstaged config.
     return retcode == 1


Test patch:
diff --git a/tests/commands/run_test.py b/tests/commands/run_test.py
index 03d741e..f1085d9 100644
--- a/tests/commands/run_test.py
+++ b/tests/commands/run_test.py
@@ -766,6 +766,47 @@ def test_lots_of_files(store, tempdir_factory):
         )
 
 
+def test_no_textconv(cap_out, store, repo_with_passing_hook):
+    # git textconv filters can hide changes from hooks
+    with open('.gitattributes', 'w') as fp:
+        fp.write('*.jpeg diff=empty\n')
+
+    with open('.git/config', 'a') as fp:
+        fp.write('[diff "empty"]\n')
+        fp.write('textconv = "true"\n')
+
+    config = {
+        'repo': 'local',
+        'hooks': [
+            {
+                'id': 'extend-jpeg',
+                'name': 'extend-jpeg',
+                'language': 'system',
+                'entry': (
+                    f'{shlex.quote(sys.executable)} -c "import sys; '
+                    'open(sys.argv[1], \'ab\').write(b\'\\x00\')"'
+                ),
+                'types': ['jpeg'],
+            },
+        ],
+    }
+    add_config_to_repo(repo_with_passing_hook, config)
+
+    stage_a_file('example.jpeg')
+
+    _test_run(
+        cap_out,
+        store,
+        repo_with_passing_hook,
+        {},
+        (
+            b'Failed',
+        ),
+        expected_ret=1,
+        stage=False,
+    )
+
+
 def test_stages(cap_out, store, repo_with_passing_hook):
     config = {
         'repo': 'local',