Thomas Braun <thomas.braun@xxxxxxxxxxxxxxxxxxx> writes:
> Subject: Re: [PATCH v2] log -G: Ignore binary files
s/Ig/ig/; (will locally munge--this alone is no reason to reroll).
The code changes looked sensible.
> diff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh
> index 844df760f7..5c3e2a16b2 100755
> --- a/t/t4209-log-pickaxe.sh
> +++ b/t/t4209-log-pickaxe.sh
> @@ -106,4 +106,44 @@ test_expect_success 'log -S --no-textconv (missing textconv tool)' '
> rm .gitattributes
> '
>
> +test_expect_success 'log -G ignores binary files' '
> + git checkout --orphan orphan1 &&
> + printf "a\0a" >data.bin &&
> + git add data.bin &&
> + git commit -m "message" &&
> + git log -Ga >result &&
> + test_must_be_empty result
> +'
As this is the first mention of data.bin, this is adding a new file
data.bin that has two 'a' but is a binary file. And that is the
only commit in the history leading to orphan1.
The fact that "log -Ga" won't find any means it missed the creation
event, because the blob is binary. Good.
> +test_expect_success 'log -G looks into binary files with -a' '
> + git checkout --orphan orphan2 &&
> + printf "a\0a" >data.bin &&
> + git add data.bin &&
> + git commit -m "message" &&
This starts from the state left by the previous test piece, i.e. we
have a binary data.bin file with two 'a' in it. We pretend to
modify and add, but these two steps are no-op if the previous
succeeded, but even if the previous step failed, we get what we want
in the data.bin file. And then we make an initial commit the same
way.
> + git log -a -Ga >actual &&
> + git log >expected &&
And we ran the same test but this time with "-a" to tell Git that
binary-ness should not matter. It will find the sole commit. Good.
> + test_cmp actual expected
> +'
> +
> +test_expect_success 'log -G looks into binary files with textconv filter' '
> + git checkout --orphan orphan3 &&
> + echo "* diff=bin" > .gitattributes &&
s/> />/; (will locally munge--this alone is no reason to reroll).
> + printf "a\0a" >data.bin &&
> + git add data.bin &&
> + git commit -m "message" &&
> + git -c diff.bin.textconv=cat log -Ga >actual &&
This exposes a slight iffy-ness in the design. The textconv filter
used here does not strip the "binary-ness" from the payload, but it
is enough to tell the machinery that -G should look into the
difference. Is that really desirable, though?
IOW, if this weren't the initial commit (which is handled by the
codepath to special-case creation and deletion in diff_grep()
function), would "log -Ga" show it without "-a"? Should it?
I think this test piece (and probably the previous ones for "-a" vs
"no -a" without textconv, as well) should be using a history with
three commits, where
- the root commit introduces "a\0a" to data.bin (creation event)
- the second commit adds another instance of "a\0a" to data.bin
(forces comparison)
- the third commit removes data.bin (deletion event)
and make sure that the three are treated identically. If "log -Ga"
finds one (with the combination of other conditions like use of
textconv or -a option), it should find all three, and vice versa.
> + git log >expected &&
> + test_cmp actual expected
> +'
> +
> +test_expect_success 'log -S looks into binary files' '
> + git checkout --orphan orphan4 &&
> + printf "a\0a" >data.bin &&
> + git add data.bin &&
> + git commit -m "message" &&
> + git log -Sa >actual &&
> + git log >expected &&
> + test_cmp actual expected
> +'
Likewise. This would also benefit from a three-commit history.
Perhaps you can create such a history at the beginning of these
additions as another "setup -G/-S binary test" step and test
different variations in subsequent tests without the setup?
> test_done
