diff mbox

[kvm-unit-tests] Use /bin/env in shebang to make scripts more portable

Message ID 20170315102550.GA44201@Sergeys-MacBook-Pro-2.local (mailing list archive)
State New, archived
Headers show

Commit Message

Sergey Bronnikov March 15, 2017, 10:25 a.m. UTC 
Some operating systems installs bash executable file to other directory
than /bin. So it is better to use env utility to find bash.
---
 Makefile                | 2 +-
 arm/run                 | 2 +-
 configure               | 2 +-
 powerpc/run             | 2 +-
 run_tests.sh            | 2 +-
 scripts/mkstandalone.sh | 6 +++---
 x86/run                 | 2 +-
 7 files changed, 9 insertions(+), 9 deletions(-)

Comments

Peter Xu March 15, 2017, 10:48 a.m. UTC | #1 
On Wed, Mar 15, 2017 at 01:25:50PM +0300, Sergey Bronnikov wrote:
> Some operating systems installs bash executable file to other directory
> than /bin. So it is better to use env utility to find bash.

Signed-off line missing. Besides that:

Reviewed-by: Peter Xu <peterx@redhat.com>

-- peterx
Sergey Bronnikov March 15, 2017, 12:31 p.m. UTC | #2 
On 18:48 Wed 15 Mar , Peter Xu wrote:
> On Wed, Mar 15, 2017 at 01:25:50PM +0300, Sergey Bronnikov wrote:
> > Some operating systems installs bash executable file to other directory
> > than /bin. So it is better to use env utility to find bash.
> 
> Signed-off line missing. Besides that:

Resend patch with Signed-off.

> Reviewed-by: Peter Xu <peterx@redhat.com>
> 
> -- peterx
Thomas Huth March 16, 2017, 7:38 a.m. UTC | #3 
On 15.03.2017 11:25, Sergey Bronnikov wrote:
> Some operating systems installs bash executable file to other directory
> than /bin. So it is better to use env utility to find bash.

Which operating systems do you have in mind here?

 Thomas
Sergey Bronnikov March 17, 2017, 5:11 a.m. UTC | #4 
On 08:38 Thu 16 Mar , Thomas Huth wrote:
> On 15.03.2017 11:25, Sergey Bronnikov wrote:
> > Some operating systems installs bash executable file to other directory
> > than /bin. So it is better to use env utility to find bash.
> 
> Which operating systems do you have in mind here?

OpenBSD

>  Thomas
Thomas Huth March 17, 2017, 8:04 a.m. UTC | #5 
On 17.03.2017 06:11, Sergey Bronnikov wrote:
> On 08:38 Thu 16 Mar , Thomas Huth wrote:
>> On 15.03.2017 11:25, Sergey Bronnikov wrote:
>>> Some operating systems installs bash executable file to other directory
>>> than /bin. So it is better to use env utility to find bash.
>>
>> Which operating systems do you have in mind here?
> 
> OpenBSD

Ok, then I'd say this patch simply does not make sense. We're talking
about kvm-unit-tests here - and KVM is pretty much Linux-only as far as
I know. Every Linux distro that I've seen so far provides /bin/bash, so
I don't think we've got to change this.

 Thomas
Sergey Bronnikov March 17, 2017, 11:11 a.m. UTC | #6 
Hi, Thomas

On 09:04 Fri 17 Mar , Thomas Huth wrote:
> On 17.03.2017 06:11, Sergey Bronnikov wrote:
> > On 08:38 Thu 16 Mar , Thomas Huth wrote:
> >> On 15.03.2017 11:25, Sergey Bronnikov wrote:
> >>> Some operating systems installs bash executable file to other directory
> >>> than /bin. So it is better to use env utility to find bash.
> >>
> >> Which operating systems do you have in mind here?
> > 
> > OpenBSD
> 
> Ok, then I'd say this patch simply does not make sense. We're talking
> about kvm-unit-tests here - and KVM is pretty much Linux-only as far as
> I know.

according to description on the project page:
"Unit tests provide KVM and virtual hardware functional testing by
targeting the features through minimal implementations of their use per
the hardware specification."
http://www.linux-kvm.org/page/KVM-unit-tests
So testsuite is applicable to any virtual machine with hardware
emulation. OpenBSD has QEMU in ports and it's own hypervisor vmd(8)
(http://man.openbsd.org/vmd). It would be perfect to use testsuite
for regression testing of these hypervisors on OpenBSD too.

Moreover my patch don't broke anything on Linux. Why not to commit it
and make tests more portable?

$ qemu-system-x86_64 -nodefaults -device isa-debug-exit,iobase=0xf4,iosize=0x4 -vnc none -serial stdio -kernel x86/msr.flat     
enabling apic
PASS: IA32_SYSENTER_CS
PASS: MSR_IA32_SYSENTER_ESP
PASS: IA32_SYSENTER_EIP
PASS: MSR_IA32_MISC_ENABLE
PASS: MSR_IA32_CR_PAT
PASS: MSR_FS_BASE
PASS: MSR_GS_BASE
PASS: MSR_KERNEL_GS_BASE
PASS: MSR_EFER
PASS: MSR_LSTAR
PASS: MSR_CSTAR
PASS: MSR_SYSCALL_MASK
PASS: MSR_*STAR eager loading
SUMMARY: 13 tests
$ uname -a
OpenBSD openbsd.example.com 6.0 GENERIC.MP#2319 amd64
$

> Every Linux distro that I've seen so far provides /bin/bash, so
> I don't think we've got to change this.
> 
>  Thomas
Paolo Bonzini March 17, 2017, 11:18 a.m. UTC | #7 
On 17/03/2017 12:11, Sergey Bronnikov wrote:
>>> OpenBSD
>> Ok, then I'd say this patch simply does not make sense. We're talking
>> about kvm-unit-tests here - and KVM is pretty much Linux-only as far as
>> I know.
> according to description on the project page:
> "Unit tests provide KVM and virtual hardware functional testing by
> targeting the features through minimal implementations of their use per
> the hardware specification."
> http://www.linux-kvm.org/page/KVM-unit-tests
> So testsuite is applicable to any virtual machine with hardware
> emulation. OpenBSD has QEMU in ports and it's own hypervisor vmd(8)
> (http://man.openbsd.org/vmd). It would be perfect to use testsuite
> for regression testing of these hypervisors on OpenBSD too.

I agree with Sergey.  Most unit tests in kvm-unit-tests are not limited
to KVM, though some are.  Otherwise, the tests would be in linux.git
rather than a separate repo.

I'm not sure if it makes sense to have a test driver for vmd, but that
would be welcome as well (with the caveat that I would not use it).

Paolo
Thomas Huth March 17, 2017, 12:28 p.m. UTC | #8 
On 17.03.2017 12:11, Sergey Bronnikov wrote:
> Hi, Thomas
> 
> On 09:04 Fri 17 Mar , Thomas Huth wrote:
>> On 17.03.2017 06:11, Sergey Bronnikov wrote:
>>> On 08:38 Thu 16 Mar , Thomas Huth wrote:
>>>> On 15.03.2017 11:25, Sergey Bronnikov wrote:
>>>>> Some operating systems installs bash executable file to other directory
>>>>> than /bin. So it is better to use env utility to find bash.
>>>>
>>>> Which operating systems do you have in mind here?
>>>
>>> OpenBSD
>>
>> Ok, then I'd say this patch simply does not make sense. We're talking
>> about kvm-unit-tests here - and KVM is pretty much Linux-only as far as
>> I know.
> 
> according to description on the project page:
> "Unit tests provide KVM and virtual hardware functional testing by
> targeting the features through minimal implementations of their use per
> the hardware specification."
> http://www.linux-kvm.org/page/KVM-unit-tests
> So testsuite is applicable to any virtual machine with hardware
> emulation. OpenBSD has QEMU in ports and it's own hypervisor vmd(8)
> (http://man.openbsd.org/vmd). It would be perfect to use testsuite
> for regression testing of these hypervisors on OpenBSD too.
> 
> Moreover my patch don't broke anything on Linux. Why not to commit it
> and make tests more portable?

OK, if you think that kvm-unit-tests is really usable on a non-Linux
system this way, I'm fine with the change, too. I just wanted to make
sure that this is more than just unnecessary code churn.

 Thomas
diff mbox

Patch

diff --git a/Makefile b/Makefile
index 16ce297..2539f9a 100644
--- a/Makefile
+++ b/Makefile
@@ -1,5 +1,5 @@ 
 
-SHELL := /bin/bash
+SHELL := /usr/bin/env bash
 
 ifeq ($(wildcard config.mak),)
 $(error run ./configure first. See ./configure -h)
diff --git a/arm/run b/arm/run
index 2134c9e..0a9a3c8 100755
--- a/arm/run
+++ b/arm/run
@@ -1,4 +1,4 @@ 
-#!/bin/bash
+#!/usr/bin/env bash
 
 if [ -z "$STANDALONE" ]; then
 	if [ ! -f config.mak ]; then
diff --git a/configure b/configure
index 8821f37..d152414 100755
--- a/configure
+++ b/configure
@@ -1,4 +1,4 @@ 
-#!/bin/bash
+#!/usr/bin/env bash
 
 prefix=/usr/local
 cc=gcc
diff --git a/powerpc/run b/powerpc/run
index 6269abb..913bc5f 100755
--- a/powerpc/run
+++ b/powerpc/run
@@ -1,4 +1,4 @@ 
-#!/bin/bash
+#!/usr/bin/env bash
 
 if [ -z "$STANDALONE" ]; then
 	if [ ! -f config.mak ]; then
diff --git a/run_tests.sh b/run_tests.sh
index 09cd057..7ac2526 100755
--- a/run_tests.sh
+++ b/run_tests.sh
@@ -1,4 +1,4 @@ 
-#!/bin/bash
+#!/usr/bin/env bash
 
 verbose="no"
 
diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
index 3c1938e..55cfc4e 100755
--- a/scripts/mkstandalone.sh
+++ b/scripts/mkstandalone.sh
@@ -1,4 +1,4 @@ 
-#!/bin/bash
+#!/usr/bin/env bash
 
 if [ ! -f config.mak ]; then
 	echo "run ./configure && make first. See ./configure -h"
@@ -38,7 +38,7 @@  generate_test ()
 {
 	local args=( $(escape "${@}") )
 
-	echo "#!/bin/bash"
+	echo "#!/usr/bin/env bash"
 	echo "export STANDALONE=yes"
 	echo "export HOST=\$(uname -m | sed -e 's/i.86/i386/;s/arm.*/arm/;s/ppc64.*/ppc64/')"
 	echo "export PRETTY_PRINT_STACKS=no"
@@ -65,7 +65,7 @@  generate_test ()
 	temp_file bin "$kernel"
 	args[3]='$bin'
 
-	(echo "#!/bin/bash"
+	(echo "#!/usr/bin/env bash"
 	 cat scripts/arch-run.bash "$TEST_DIR/run") | temp_file RUNTIME_arch_run
 
 	echo "exec {stdout}>&1"
diff --git a/x86/run b/x86/run
index 867a1cc..486a370 100755
--- a/x86/run
+++ b/x86/run
@@ -1,4 +1,4 @@ 
-#!/bin/bash
+#!/usr/bin/env bash
 
 [ -z "$STANDALONE" ] && source scripts/arch-run.bash