[v3,1/2] iotests: Use absolute paths for executables
diff mbox

Message ID 20170529152316.27647-2-mreitz@redhat.com
State New
Headers show

Commit Message

Max Reitz May 29, 2017, 3:23 p.m. UTC
A user may specify a relative path for accessing qemu, qemu-img, etc.
through environment variables ($QEMU_PROG and friends) or a symlink.

If a test decides to change its working directory, relative paths will
cease to work, however. Work around this by making all of the paths to
programs that should undergo testing absolute. Besides "realpath", we
also have to use "which" to support programs in $PATH.

As a side effect, this fixes specifying these programs as symlinks for
out-of-tree builds: Before, you would have to create two symlinks, one
in the build and one in the source tree (the first one for common.config
to find, the second one for the iotest to use). Now it is sufficient to
create one in the build tree because common.config will resolve it.

Reported-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/common.config | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Eric Blake May 29, 2017, 3:42 p.m. UTC | #1
On 05/29/2017 10:23 AM, Max Reitz wrote:
> A user may specify a relative path for accessing qemu, qemu-img, etc.
> through environment variables ($QEMU_PROG and friends) or a symlink.
> 
> If a test decides to change its working directory, relative paths will
> cease to work, however. Work around this by making all of the paths to
> programs that should undergo testing absolute. Besides "realpath", we
> also have to use "which" to support programs in $PATH.

'type -p' is more portable than 'which' - especially since our scripts
are bash scripts, and type is a bash builtin while which is not.

> 
> As a side effect, this fixes specifying these programs as symlinks for
> out-of-tree builds: Before, you would have to create two symlinks, one
> in the build and one in the source tree (the first one for common.config
> to find, the second one for the iotest to use). Now it is sufficient to
> create one in the build tree because common.config will resolve it.
> 
> Reported-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  tests/qemu-iotests/common.config | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config
> index d1b45f5..08aac56 100644
> --- a/tests/qemu-iotests/common.config
> +++ b/tests/qemu-iotests/common.config
> @@ -103,6 +103,12 @@ if [ -z "$QEMU_VXHS_PROG" ]; then
>      export QEMU_VXHS_PROG="`set_prog_path qnio_server`"
>  fi
>  
> +export QEMU_PROG=$(realpath "$(which "$QEMU_PROG")")
> +export QEMU_IMG_PROG=$(realpath "$(which "$QEMU_IMG_PROG")")
> +export QEMU_IO_PROG=$(realpath "$(which "$QEMU_IO_PROG")")
> +export QEMU_NBD_PROG=$(realpath "$(which "$QEMU_NBD_PROG")")
> +export QEMU_VXHS_PROG=$(realpath "$(which "$QEMU_VXHS_PROG")")

If you switch all of these to $(realpath -- "$(type -p "$QEMU_...")"),
you can add:

Reviewed-by: Eric Blake <eblake@redhat.com>
Max Reitz May 29, 2017, 3:46 p.m. UTC | #2
On 2017-05-29 17:42, Eric Blake wrote:
> On 05/29/2017 10:23 AM, Max Reitz wrote:
>> A user may specify a relative path for accessing qemu, qemu-img, etc.
>> through environment variables ($QEMU_PROG and friends) or a symlink.
>>
>> If a test decides to change its working directory, relative paths will
>> cease to work, however. Work around this by making all of the paths to
>> programs that should undergo testing absolute. Besides "realpath", we
>> also have to use "which" to support programs in $PATH.
> 
> 'type -p' is more portable than 'which' - especially since our scripts
> are bash scripts, and type is a bash builtin while which is not.
> 
>>
>> As a side effect, this fixes specifying these programs as symlinks for
>> out-of-tree builds: Before, you would have to create two symlinks, one
>> in the build and one in the source tree (the first one for common.config
>> to find, the second one for the iotest to use). Now it is sufficient to
>> create one in the build tree because common.config will resolve it.
>>
>> Reported-by: Kevin Wolf <kwolf@redhat.com>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>  tests/qemu-iotests/common.config | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config
>> index d1b45f5..08aac56 100644
>> --- a/tests/qemu-iotests/common.config
>> +++ b/tests/qemu-iotests/common.config
>> @@ -103,6 +103,12 @@ if [ -z "$QEMU_VXHS_PROG" ]; then
>>      export QEMU_VXHS_PROG="`set_prog_path qnio_server`"
>>  fi
>>  
>> +export QEMU_PROG=$(realpath "$(which "$QEMU_PROG")")
>> +export QEMU_IMG_PROG=$(realpath "$(which "$QEMU_IMG_PROG")")
>> +export QEMU_IO_PROG=$(realpath "$(which "$QEMU_IO_PROG")")
>> +export QEMU_NBD_PROG=$(realpath "$(which "$QEMU_NBD_PROG")")
>> +export QEMU_VXHS_PROG=$(realpath "$(which "$QEMU_VXHS_PROG")")
> 
> If you switch all of these to $(realpath -- "$(type -p "$QEMU_...")"),
> you can add:

I'd love to, but this is what type -p outputs for me:

$ type -p qemu-img
qemu-img is /usr/bin/qemu-img

So I would need to parse the result (and it depends on the locale). If
that is indeed so, I'd rather stay with which, to be honest...

Max
Eric Blake May 29, 2017, 3:55 p.m. UTC | #3
On 05/29/2017 10:46 AM, Max Reitz wrote:

>> If you switch all of these to $(realpath -- "$(type -p "$QEMU_...")"),
>> you can add:
> 
> I'd love to, but this is what type -p outputs for me:
> 
> $ type -p qemu-img
> qemu-img is /usr/bin/qemu-img

Huh? That's plain 'type' output.  Are you sure you're testing 'type -p'?

$ PATH=$PATH    # to forcefully clear bash's cache
$ type qemu-img
qemu-img is /usr/bin/qemu-img
$ type -p qemu-img
/usr/bin/qemu-img
$ qemu-img --help >/dev/null   # to repopulate qemu-img into the cache
$ type qemu-img
qemu-img is hashed (/usr/bin/qemu-img)
$ type -p qemu-img
/usr/bin/qemu-img

> 
> So I would need to parse the result (and it depends on the locale). If
> that is indeed so, I'd rather stay with which, to be honest...

Plain 'type' does have to be parsed, but 'type -p' is required to be
machine-usable.
Max Reitz May 29, 2017, 3:58 p.m. UTC | #4
On 2017-05-29 17:55, Eric Blake wrote:
> On 05/29/2017 10:46 AM, Max Reitz wrote:
> 
>>> If you switch all of these to $(realpath -- "$(type -p "$QEMU_...")"),
>>> you can add:
>>
>> I'd love to, but this is what type -p outputs for me:
>>
>> $ type -p qemu-img
>> qemu-img is /usr/bin/qemu-img
> 
> Huh? That's plain 'type' output.  Are you sure you're testing 'type -p'?
> 
> $ PATH=$PATH    # to forcefully clear bash's cache
> $ type qemu-img
> qemu-img is /usr/bin/qemu-img
> $ type -p qemu-img
> /usr/bin/qemu-img
> $ qemu-img --help >/dev/null   # to repopulate qemu-img into the cache
> $ type qemu-img
> qemu-img is hashed (/usr/bin/qemu-img)
> $ type -p qemu-img
> /usr/bin/qemu-img
> 
>>
>> So I would need to parse the result (and it depends on the locale). If
>> that is indeed so, I'd rather stay with which, to be honest...
> 
> Plain 'type' does have to be parsed, but 'type -p' is required to be
> machine-usable.

Oops. I tested it (both with -p and without) on zsh, then on bash, and I
forgot the -p on bash. Well, I'm going to trust you, then. O:-)

Max

Patch
diff mbox

diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config
index d1b45f5..08aac56 100644
--- a/tests/qemu-iotests/common.config
+++ b/tests/qemu-iotests/common.config
@@ -103,6 +103,12 @@  if [ -z "$QEMU_VXHS_PROG" ]; then
     export QEMU_VXHS_PROG="`set_prog_path qnio_server`"
 fi
 
+export QEMU_PROG=$(realpath "$(which "$QEMU_PROG")")
+export QEMU_IMG_PROG=$(realpath "$(which "$QEMU_IMG_PROG")")
+export QEMU_IO_PROG=$(realpath "$(which "$QEMU_IO_PROG")")
+export QEMU_NBD_PROG=$(realpath "$(which "$QEMU_NBD_PROG")")
+export QEMU_VXHS_PROG=$(realpath "$(which "$QEMU_VXHS_PROG")")
+
 _qemu_wrapper()
 {
     (