Bug 252515 - diff incorrectly reports files as different when with -q -w options set
Summary: diff incorrectly reports files as different when with -q -w options set
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 12.2-RELEASE
Hardware: amd64 Any
: --- Affects Many People
Assignee: Ed Maste
URL: https://reviews.freebsd.org/D28064
Keywords:
Depends on:
Blocks:
 
Reported: 2021-01-08 13:06 UTC by Scott Aitken
Modified: 2021-01-15 15:10 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Scott Aitken 2021-01-08 13:06:28 UTC
Two files that differ only in white space are NOT reported as different with just the `-w' option set but ARE reported as different when `-q' is also set.

# echo "hello  there" > /tmp/h1
# echo "hello there" > /tmp/h2
# diff -w /tmp/h1 /tmp/h2
# diff -qw /tmp/h1 /tmp/h2
Files /tmp/h1 and /tmp/h2 differ

Exit codes are also wrong (the second diff command above returns 1).

This is irrespective of long or short options names.

My reading of the man page suggests the `-q' option should not change the behaviour of `-w'.
Comment 1 Ed Maste freebsd_committer 2021-01-08 17:54:55 UTC
Interesting; GNU diff behaves as you expected:

$ diff -qw a b; echo $?
Files a and b differ
1
$ gdiff -qw a b; echo $?
0

That said I think the behaviour of BSD diff is more useful; in any case the man page needs to be more clear.
Comment 2 Scott Aitken 2021-01-09 01:19:55 UTC
Sorry, I’m confused. 

Are you suggesting:
a) the current behaviour is correct and the man page needs remediation. (ie: the '-q’ should modify the matching logic), or
b) the current behaviour is incorrect, and the man page is correct. (ie: diff should return the same exit code regardless of whether told to be quiet with `-q’). 

Thanks.
Comment 3 Ed Maste freebsd_committer 2021-01-09 01:49:35 UTC
My preference, if GNU diff did not exist, would be to leave the code as is, and change the man page to be clear. I would argue that -q and -w are independently useful, and being able to use -q with whitespace ignored or not is valuable.

That said, compatibility with the (likely rather popular) diff is also valuable.

We need to do either (a) or (b), and likely need to do a little more research first on diff implementations on other platforms (e.g. MacOS).
Comment 4 Ed Maste freebsd_committer 2021-01-09 02:05:39 UTC
Oops, sorry for my confusing reply, I had it backwards.
Comment 5 Scott Aitken 2021-01-09 03:55:27 UTC
I'm still confused.

Here's my logic:
`-w' changes the diff detection algorithm
`-q' changes what is displayed to the user

I can't see why these two should interact at all.

Also the PoLA should have preserved the behaviour of `-q', or at the very least this odd interaction should have been captured in the release notes.

MacOS (and FreeBSD 11):
Darwin Kernel Version 20.2.0
# echo "hello  there" > /tmp/h1
# echo "hello there" > /tmp/h2
# diff -w /tmp/h1 /tmp/h2
# diff -qw /tmp/h1 /tmp/h2
#

NB: MacOS uses the same diff as FreeBSD < 12: diffutils 2.8.1
Comment 6 Ed Maste freebsd_committer 2021-01-09 16:26:55 UTC
The only confusion is my misunderstanding.
The patch in https://reviews.freebsd.org/D28064 should fix it.
Comment 7 commit-hook freebsd_committer 2021-01-09 18:35:06 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=12a8d3027d414abd15798de1b6d8b01151bdac50

commit 12a8d3027d414abd15798de1b6d8b01151bdac50
Author:     Ed Maste <emaste@FreeBSD.org>
AuthorDate: 2021-01-09 16:22:28 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2021-01-09 18:34:06 +0000

    diff: honour flags with -q

    Previously -q (just print a line when files differ) ignored flags like
    -w (ignore whitespace).  Avoid the D_BRIEF short-circuit when flags are
    in effect.

    PR:             252515
    Reported by:    Scott Aitken
    Reviewed by:    kevans
    MFC after:      1 week
    Sponsored by:   The FreeBSD Foundation
    Differential Revision: https://reviews.freebsd.org/D28064

 usr.bin/diff/diffreg.c          |  4 +++-
 usr.bin/diff/tests/diff_test.sh | 10 ++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)
Comment 8 Scott Aitken 2021-01-10 02:49:24 UTC
Patch works for me, thanks.
Comment 9 commit-hook freebsd_committer 2021-01-15 14:27:21 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=7366c22508e98357a17664b683841b47d268b5f8

commit 7366c22508e98357a17664b683841b47d268b5f8
Author:     Ed Maste <emaste@FreeBSD.org>
AuthorDate: 2021-01-09 16:22:28 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2021-01-15 14:25:35 +0000

    diff: honour flags with -q

    Previously -q (just print a line when files differ) ignored flags like
    -w (ignore whitespace).  Avoid the D_BRIEF short-circuit when flags are
    in effect.

    PR:             252515
    Reported by:    Scott Aitken
    Reviewed by:    kevans
    Sponsored by:   The FreeBSD Foundation
    Differential Revision: https://reviews.freebsd.org/D28064

    (cherry picked from commit 12a8d3027d414abd15798de1b6d8b01151bdac50)

 usr.bin/diff/diffreg.c          |  4 +++-
 usr.bin/diff/tests/diff_test.sh | 10 ++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)
Comment 10 Ed Maste freebsd_committer 2021-01-15 15:10:03 UTC
(In reply to Scott Aitken from comment #8)
Thanks for the bug report and for testing! The fix is now merged to stable/12 and will appear in FreeBSD 12.3 and 13.0.