Libguestfs - Apr 2023 - [libnbd PATCH 16/18] tests/requires: wrap source code at 80 characters

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 at 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).
> +            "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 '! ...'.

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

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 at 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. :/

I'll redo this patch then.

Thanks!
Laszlo