Re: [Libguestfs] [libnbd PATCH 16/18] tests/requires: wrap source code at 80 characters
On 4/19/23 15:53, Laszlo Ersek wrote:
On 4/19/23 15:37, Eric Blake wrote:
> On Tue, Apr 18, 2023 at 07:26:29PM +0200, Laszlo Ersek wrote:
>> Embedding a shell script in a multi-line C string literal is an exercise
>> in pain. I can see why the original author (whom I shall not look up with
>> git-blame :) ) went for the easy route. Still, we want the source code to
>> fit in 80 columns.
>>
>> Note: in my "interop/test-suite.log", I see
>>
>>> SKIP: interop-qemu-nbd-tls-certs
>>> ================================
>>>
>>> requires test -d /home/lacos/src/v2v/libnbd/tests/pki
>>> Test skipped because prerequisite is missing or not working.
>>> SKIP interop-qemu-nbd-tls-certs (exit status: 77)
>>>
>>> SKIP: interop-qemu-nbd-tls-psk
>>> ==============================
>>>
>>> requires test -f /home/lacos/src/v2v/libnbd/tests/keys.psk
>>> Test skipped because prerequisite is missing or not working.
>>> SKIP interop-qemu-nbd-tls-psk (exit status: 77)
>>
>> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
>> Signed-off-by: Laszlo Ersek <lersek(a)redhat.com>
>> ---
>> tests/requires.c | 16 ++++++++++++++--
>> 1 file changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/requires.c b/tests/requires.c
>> index 199e30605473..cc5fd77b9f27 100644
>> --- a/tests/requires.c
>> +++ b/tests/requires.c
>> @@ -57,7 +57,13 @@ requires_qemu_nbd_tls_support (const char *qemu_nbd)
>> * interested in the error message that it prints.
>> */
>> snprintf (cmd, sizeof cmd,
>> - "if %s --object tls-creds-x509,id=tls0 |& grep -sq 'TLS
credentials support requires GNUTLS'; then exit 1; else exit 0; fi",
>> + "if %s --object tls-creds-x509,id=tls0 \\\n"
>> + " |& grep -sq 'TLS credentials support requires
GNUTLS'\n"
>
> Note: |& is a bashism; but system() might not invoke bash. So this is
> already non-portable code; and we should be fixing that. (I don't
> mind doing it as a followup if you end up pushing before seeing this
> message).
I noticed |& too; my thought was "TIL". :)
>
>> + "then\n"
>> + " exit 1\n"
>> + "else\n"
>> + " exit 0\n"
>> + "fi\n",
>
> For that matter, 'if ...; then exit 1; else exit 0; fi' can be
> compressed to '! ...'.
>
Yes, I noticed that too, but I assumed there was a finer point I was
missing. :/
Ah, it's not too relevant, but now I remember what was bothering me,
about "!": if you embed the original construct in a "set -e"
environment, it will exit with status 1 on the appropriate branch; but
if you embed the "! ..." construct, it won't exit!
Anyway, the context is not like that here. We pass the command string to
system(), and I understand "set -e" is not in effect there.
Laszlo