Re: [Qemu-devel] [PATCH v4 4/5] iotests: Add VMDK tests for blockdev-create

[Eric Blake <eblake@redhat.com>]

On 12/7/18 10:42 AM, Kevin Wolf wrote:
> Am 07.12.2018 um 16:40 hat Eric Blake geschrieben:
>> On 12/7/18 8:45 AM, Kevin Wolf wrote:
>>> Am 07.12.2018 um 14:12 hat Markus Armbruster geschrieben:
>>>> git-am complains
>>>>
>>>>       Applying: iotests: Add VMDK tests for blockdev-create
>>>>       .git/rebase-apply/patch:281: trailing whitespace.
>>>>                   format:
>>>>       .git/rebase-apply/patch:308: trailing whitespace.
>>>>                   format:
>>>>       .git/rebase-apply/patch:335: trailing whitespace.
>>>>                   format:
>>>>       .git/rebase-apply/patch:600: new blank line at EOF.
>>>>       +
>>>>       warning: 4 lines add whitespace errors.
>>>
>>> This is in the reference output, so trailing whitespace/blank lines are
>>> actually correct.
>>
>> Ah, but doesn't ./check already ignore differences in trailing whitespace
>> present in the actual running that is not present in the *.out files,
>> precisely so we don't have to check in trailing whitespace reference
>> outputs?
> 
> It does ignore whitespace changes, so even if we remove that whitespace,
> the test won't fail. But I don't think that's a good reason to check in
> inaccurate reference output.
> 
> There are a few test cases that have a reference output like this and
> it's always annoying: When I later add a new subtest, I add the new test
> code, review the ./check output and if it looks good, I do something
> like 'cp 237.out.bad 237.out'. At that point, I'll have to manually
> revert completely unrelated whitespace changes again.

Is it worth teaching iotests.img_info_log() to strip trailing 
whitespace?  Or even to teach qemu-img itself to quit generating 
trailing whitespace?

> 
> Some 'git am' warnings feel like the lesser evil to me.

True, and a comment in the commit message about intentionally triggering 
a known checkpatch flag goes a long ways to document why you want the 
trailing whitespace (assuming we don't instead decide to tackle a root 
cause of having the whitespace in the first place).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org