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
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
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
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
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
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
© 2016 - 2024 Red Hat, Inc.