All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] iotests: Drop readlink -f
@ 2020-09-14 14:56 Max Reitz
  2020-09-14 15:43 ` Thomas Huth
  2020-09-14 16:03 ` Eric Blake
  0 siblings, 2 replies; 6+ messages in thread
From: Max Reitz @ 2020-09-14 14:56 UTC (permalink / raw)
  To: qemu-block
  Cc: Peter Maydell, Thomas Huth, Alex Bennée, qemu-devel, Max Reitz

On macOS, (out of the box) readlink does not have -f.  We do not really
need readlink here, though, it was just a replacement for realpath
(which is not available on our BSD test systems), which we needed to
make the $(dirname) into an absolute path.

Instead of using either, just use "cd; pwd" like is done for
$source_iotests.

Fixes: b1cbc33a3971b6bb005d5ac3569feae35a71de0f
       ("iotests: Allow running from different directory")
Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Reported-by: Claudio Fontana <cfontana@suse.de>
Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/check | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index e14a1f354d..678b6e4910 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -44,7 +44,7 @@ then
         _init_error "failed to obtain source tree name from check symlink"
     fi
     source_iotests=$(cd "$source_iotests"; pwd) || _init_error "failed to enter source tree"
-    build_iotests=$(readlink -f $(dirname "$0"))
+    build_iotests=$(cd "$(dirname "$0")"; pwd)
 else
     # called from the source tree
     source_iotests=$PWD
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] iotests: Drop readlink -f
  2020-09-14 14:56 [PATCH v3] iotests: Drop readlink -f Max Reitz
@ 2020-09-14 15:43 ` Thomas Huth
  2020-09-14 16:03   ` Peter Maydell
  2020-09-14 16:04   ` Eric Blake
  2020-09-14 16:03 ` Eric Blake
  1 sibling, 2 replies; 6+ messages in thread
From: Thomas Huth @ 2020-09-14 15:43 UTC (permalink / raw)
  To: Max Reitz, qemu-block
  Cc: Peter Maydell, Alex Bennée, qemu-devel, Claudio Fontana

On 14/09/2020 16.56, Max Reitz wrote:
> On macOS, (out of the box) readlink does not have -f.  We do not really
> need readlink here, though, it was just a replacement for realpath
> (which is not available on our BSD test systems), which we needed to
> make the $(dirname) into an absolute path.
> 
> Instead of using either, just use "cd; pwd" like is done for
> $source_iotests.
> 
> Fixes: b1cbc33a3971b6bb005d5ac3569feae35a71de0f
>        ("iotests: Allow running from different directory")
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Reported-by: Claudio Fontana <cfontana@suse.de>
> Reported-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  tests/qemu-iotests/check | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> index e14a1f354d..678b6e4910 100755
> --- a/tests/qemu-iotests/check
> +++ b/tests/qemu-iotests/check
> @@ -44,7 +44,7 @@ then
>          _init_error "failed to obtain source tree name from check symlink"
>      fi
>      source_iotests=$(cd "$source_iotests"; pwd) || _init_error "failed to enter source tree"
> -    build_iotests=$(readlink -f $(dirname "$0"))
> +    build_iotests=$(cd "$(dirname "$0")"; pwd)

I assume the nested quotes are ok here? ... shell scripts always confuse
me in that regard...

 Thomas



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] iotests: Drop readlink -f
  2020-09-14 15:43 ` Thomas Huth
@ 2020-09-14 16:03   ` Peter Maydell
  2020-09-14 16:04   ` Eric Blake
  1 sibling, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2020-09-14 16:03 UTC (permalink / raw)
  To: Thomas Huth
  Cc: Qemu-block, Alex Bennée, QEMU Developers, Max Reitz,
	Claudio Fontana

On Mon, 14 Sep 2020 at 16:43, Thomas Huth <thuth@redhat.com> wrote:
>
> On 14/09/2020 16.56, Max Reitz wrote:
> > On macOS, (out of the box) readlink does not have -f.  We do not really
> > need readlink here, though, it was just a replacement for realpath
> > (which is not available on our BSD test systems), which we needed to
> > make the $(dirname) into an absolute path.
> >
> > Instead of using either, just use "cd; pwd" like is done for
> > $source_iotests.
> >
> > Fixes: b1cbc33a3971b6bb005d5ac3569feae35a71de0f
> >        ("iotests: Allow running from different directory")
> > Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> > Reported-by: Claudio Fontana <cfontana@suse.de>
> > Reported-by: Thomas Huth <thuth@redhat.com>
> > Signed-off-by: Max Reitz <mreitz@redhat.com>
> > ---
> >  tests/qemu-iotests/check | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> > index e14a1f354d..678b6e4910 100755
> > --- a/tests/qemu-iotests/check
> > +++ b/tests/qemu-iotests/check
> > @@ -44,7 +44,7 @@ then
> >          _init_error "failed to obtain source tree name from check symlink"
> >      fi
> >      source_iotests=$(cd "$source_iotests"; pwd) || _init_error "failed to enter source tree"
> > -    build_iotests=$(readlink -f $(dirname "$0"))
> > +    build_iotests=$(cd "$(dirname "$0")"; pwd)
>
> I assume the nested quotes are ok here? ... shell scripts always confuse
> me in that regard...

Yes, the quoting is right here
(cf https://unix.stackexchange.com/questions/118433/quoting-within-command-substitution-in-bash)
 -- $() creates a new quoting context and within it you quote
the command the same way you would if it were a standalone
commandline.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] iotests: Drop readlink -f
  2020-09-14 14:56 [PATCH v3] iotests: Drop readlink -f Max Reitz
  2020-09-14 15:43 ` Thomas Huth
@ 2020-09-14 16:03 ` Eric Blake
  2020-09-14 16:08   ` Peter Maydell
  1 sibling, 1 reply; 6+ messages in thread
From: Eric Blake @ 2020-09-14 16:03 UTC (permalink / raw)
  To: Max Reitz, qemu-block
  Cc: Peter Maydell, Thomas Huth, Alex Bennée, qemu-devel

On 9/14/20 9:56 AM, Max Reitz wrote:
> On macOS, (out of the box) readlink does not have -f.  We do not really
> need readlink here, though, it was just a replacement for realpath
> (which is not available on our BSD test systems), which we needed to
> make the $(dirname) into an absolute path.
> 
> Instead of using either, just use "cd; pwd" like is done for
> $source_iotests.
> 
> Fixes: b1cbc33a3971b6bb005d5ac3569feae35a71de0f
>         ("iotests: Allow running from different directory")
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Reported-by: Claudio Fontana <cfontana@suse.de>
> Reported-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>   tests/qemu-iotests/check | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> index e14a1f354d..678b6e4910 100755
> --- a/tests/qemu-iotests/check
> +++ b/tests/qemu-iotests/check
> @@ -44,7 +44,7 @@ then
>           _init_error "failed to obtain source tree name from check symlink"
>       fi
>       source_iotests=$(cd "$source_iotests"; pwd) || _init_error "failed to enter source tree"
> -    build_iotests=$(readlink -f $(dirname "$0"))
> +    build_iotests=$(cd "$(dirname "$0")"; pwd)

If CDPATH is set, this can produce wrong results.  Do we care?  Probably 
not, since (as you point out), $source_iotests has the same bug, and no 
one has complained yet.

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



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] iotests: Drop readlink -f
  2020-09-14 15:43 ` Thomas Huth
  2020-09-14 16:03   ` Peter Maydell
@ 2020-09-14 16:04   ` Eric Blake
  1 sibling, 0 replies; 6+ messages in thread
From: Eric Blake @ 2020-09-14 16:04 UTC (permalink / raw)
  To: Thomas Huth, Max Reitz, qemu-block
  Cc: Peter Maydell, Alex Bennée, qemu-devel, Claudio Fontana

On 9/14/20 10:43 AM, Thomas Huth wrote:

>> +++ b/tests/qemu-iotests/check
>> @@ -44,7 +44,7 @@ then
>>           _init_error "failed to obtain source tree name from check symlink"
>>       fi
>>       source_iotests=$(cd "$source_iotests"; pwd) || _init_error "failed to enter source tree"
>> -    build_iotests=$(readlink -f $(dirname "$0"))
>> +    build_iotests=$(cd "$(dirname "$0")"; pwd)
> 
> I assume the nested quotes are ok here? ... shell scripts always confuse
> me in that regard...

Yes.  Anything inside $() is a standalone script.  That's one of the 
reasons that $() is nicer than `` (where you did indeed have to worry 
about " inside `` interfering with "``" on the outside, in some shells).

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



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] iotests: Drop readlink -f
  2020-09-14 16:03 ` Eric Blake
@ 2020-09-14 16:08   ` Peter Maydell
  0 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2020-09-14 16:08 UTC (permalink / raw)
  To: Eric Blake
  Cc: Thomas Huth, Alex Bennée, QEMU Developers, Qemu-block, Max Reitz

On Mon, 14 Sep 2020 at 17:03, Eric Blake <eblake@redhat.com> wrote:
> If CDPATH is set, this can produce wrong results.  Do we care?  Probably
> not, since (as you point out), $source_iotests has the same bug, and no
> one has complained yet.

This has come up previously -- configure and probably most
of our other shell scripts will also break in the presence
of CDPATH. I stand by my position that CDPATH is a bad
misfeature that the shell should not be applying to
scripts. If anybody ever reports a bug we could I suppose
unconditionally unset CDPATH at the top of all our scripts.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-09-14 17:24 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-14 14:56 [PATCH v3] iotests: Drop readlink -f Max Reitz
2020-09-14 15:43 ` Thomas Huth
2020-09-14 16:03   ` Peter Maydell
2020-09-14 16:04   ` Eric Blake
2020-09-14 16:03 ` Eric Blake
2020-09-14 16:08   ` Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.