`--in-diff` fails on diffs mentioning binary files

[kpreid]

Suppose that test.diff contains git diff output such as:

diff --git a/test-renderers/expected/renderers/fog-None-wgpu.png b/test-renderers/expected/renderers/fog-None-wgpu.png
index 616d6ea8..afd1b043 100644
Binary files a/test-renderers/expected/renderers/fog-None-wgpu.png and b/test-renderers/expected/renderers/fog-None-wgpu.png differ

Then cargo mutants --in-diff test.diff will fail:

Error: Failed to parse diff: Line 1: Error while parsing: diff --git a/test-renderers/expected/renderers/fog-None-wgpu.png b/test-renderers/expected/renderers/fog-None-wgpu.png
index 616d6ea8..afd1b043 100644
Binary files a/test-renderers/expected/renderers/fog-None-wgpu.png and b/test-renderers/expected/renderers/fog-None-wgpu.png differ

If the diff contains files preceding this one, the outcome is similar, but as a panic:

thread 'main' panicked at /Users/kpreid/.cargo/registry/src/index.crates.io-6f17d22bba15001f/patch-0.7.0/src/parser.rs:84:5:
bug: failed to parse entire input. Remaining: 'diff --git a/test-renderers/expected/renderers/fog-None-wgpu.png b/test-renderers/expected/renderers/fog-None-wgpu.png
index 616d6ea8..afd1b043 100644
Binary files a/test-renderers/expected/renderers/fog-None-wgpu.png and b/test-renderers/expected/renderers/fog-None-wgpu.png differ
'

Instead, this diff should successfully parse (and, since binary files are not Rust source code that can be mutated, it should be skipped).

[sourcefrog]

Thanks, good find!

The diffs are parsed using https://docs.rs/patch/.

One option might be to fix that crate to understand the "Binary files" message. It looks like https://github.com/uniphil/patch-rs/pull/22/files?diff=split&w=1 would fix it to at least not panic in the second case.

Another pragmatic option would be for cargo-mutants to stripe these lines before trying to parse the diff file.

As a workaround you could use the --exclude PAT option to diff.

[eirnym]

FYI: it's not alwasy possible as gh pr diff doesn't support file exclusion.

[sourcefrog]

@eirnym This shouldn't be a problem any more though?

[eirnym]

There's an unpleasant workaround (with fetching while repo), but I'd prefer more robust solution with cargo mutants just ignoring binary files in diff.

I understand that problem is in the dependency, but I'd reopen the issue till dependency issue isn't closed. This would inform users, that problem exists and known.

[eirnym]

FYI: I don't feel that patch-rs you're using is still supported. Last change or comment from the author was in 2022, unlike the other repositories they have.

[sourcefrog]

There's an unpleasant workaround (with fetching while repo), but I'd prefer more robust solution with cargo mutants just ignoring binary files in diff.

I don't understand why you need a workaround. This bug is fixed. Does this not work for you?

; echo 'Binary files a and b differ' >/tmp/example.diff
; cargo mutants --list --in-diff /tmp/example.diff
 INFO diff file is empty; no mutants will match

There's a test for this

https://github.com/sourcefrog/cargo-mutants/blob/49940940bd9846a25e4c2db1c4f00e39a668ed0a/tests/in_diff.rs#L72-L88

[eirnym]

The desired solution is to download diff via gh command.

Even I explicitly marked these new files via git attributes as binary, they appear I the patch as they are not UTF-8. (They have 1 byte prefix containing 0xFF)

The best solution is to load the diff and exclude files cargo mutants not needed and then complain on UTF-8.

According to Cargo.toml, this tool uses patch-rs, which accumulates multiple complains including yours, but since 2022 that library is unsupported by its sole maintainer. I believe they don't receive or ignore all notifications for one or another reason.

[sourcefrog]

That sounds like a different bug. Please file a new bug and attach some reproduction instructions or a trimmed example of a patch that causes a problem? Is it about files whose diff contains non-UTF8 characters? Or the filename?

[kpreid]

Original reporter here. It turns out that this bug is not fixed. I hit it again after updating to [email protected]:

thread 'main' (5150) panicked at /home/runner/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/patch-0.7.0/src/parser.rs:84:5:
bug: failed to parse entire input. Remaining: 'diff --git a/test-renderers/expected/renderers/antialias-Always-wgpu.png b/test-renderers/expected/renderers/antialias-Always-wgpu.png
index e10d3ac4..1a930be5 100644
'

The added filter does successfully filter out the “Binary files differ” line, but it leaves behind the two lines that occur before that line, introducing the file.

[sourcefrog]

Oh thanks.

[benalleng]

I recently hit a similar bug on a .toml file, is there any reason to not simply filter the original diff to search only for rust files like so? A bit of a hack but seeing as this is a patch-rs problem at the end of the day that's how I've opted to get around this.

git diff origin/master -- '*.rs' | tee git.diff

[sourcefrog]

It seems like the diff from https://github.com/sourcefrog/cargo-mutants/issues/391#issuecomment-3605050253 also fails in gitpatch 0.7.1...

[sourcefrog]

I recently hit a similar bug on a .toml file, is there any reason to not simply filter the original diff to search only for rust files like so? A bit of a hack but seeing as this is a patch-rs problem at the end of the day that's how I've opted to get around this.

git diff origin/master -- '*.rs' | tee git.diff

Yes, that's a good workaround!