[v2,1/4] qemu-iotests: remove bash shebang from library files
diff mbox series

Message ID 20191009194740.8079-2-crosa@redhat.com
State New
Headers show
Series
  • iotests: trivial cleanups
Related show

Commit Message

Cleber Rosa Oct. 9, 2019, 7:47 p.m. UTC
Due to not being able to find a reason to have shebangs on files that
are not executable.

While at it, add a mode hint to emacs, which would be clueless or
plain wrong about these containing shell code.

Suggested-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/qemu-iotests/common.config  | 2 +-
 tests/qemu-iotests/common.filter  | 2 +-
 tests/qemu-iotests/common.nbd     | 3 +--
 tests/qemu-iotests/common.pattern | 2 +-
 tests/qemu-iotests/common.qemu    | 2 +-
 tests/qemu-iotests/common.rc      | 2 +-
 tests/qemu-iotests/common.tls     | 2 +-
 7 files changed, 7 insertions(+), 8 deletions(-)

Comments

Cleber Rosa Oct. 9, 2019, 8:54 p.m. UTC | #1
----- Original Message -----
> From: "Eric Blake" <eblake@redhat.com>
> To: "Cleber Rosa" <crosa@redhat.com>, qemu-devel@nongnu.org
> Cc: qemu-block@nongnu.org, "Max Reitz" <mreitz@redhat.com>, "Kevin Wolf" <kwolf@redhat.com>, qemu-trivial@nongnu.org,
> "Michael Tokarev" <mjt@tls.msk.ru>, "Laurent Vivier" <laurent@vivier.eu>
> Sent: Wednesday, October 9, 2019 3:51:56 PM
> Subject: Re: [PATCH v2 1/4] qemu-iotests: remove bash shebang from library files
> 
> On 10/9/19 2:47 PM, Cleber Rosa wrote:
> > Due to not being able to find a reason to have shebangs on files that
> > are not executable.
> > 
> > While at it, add a mode hint to emacs, which would be clueless or
> > plain wrong about these containing shell code.
> > 
> > Suggested-by: Eric Blake <eblake@redhat.com>
> > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > ---
> >   tests/qemu-iotests/common.config  | 2 +-
> >   tests/qemu-iotests/common.filter  | 2 +-
> >   tests/qemu-iotests/common.nbd     | 3 +--
> >   tests/qemu-iotests/common.pattern | 2 +-
> >   tests/qemu-iotests/common.qemu    | 2 +-
> >   tests/qemu-iotests/common.rc      | 2 +-
> >   tests/qemu-iotests/common.tls     | 2 +-
> >   7 files changed, 7 insertions(+), 8 deletions(-)
> > 
> > diff --git a/tests/qemu-iotests/common.config
> > b/tests/qemu-iotests/common.config
> > index 9bd1a5a6fc..b85a6a6f96 100644
> > --- a/tests/qemu-iotests/common.config
> > +++ b/tests/qemu-iotests/common.config
> > @@ -1,4 +1,4 @@
> > -#!/usr/bin/env bash
> > +# -*- emacs mode: sh -*-
> 
> I thought my version:
> # hey emacs, this file will be sourced by bash -*- mode: sh -*-
> was cuter, but that's not a requirement, and yours works  ;)
> 

I will not make any disputes on that! :)

Thanks for the reviews!
- Cleber.

> Reviewed-by: Eric Blake <eblake@redhat.com>
> 
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
>
Kevin Wolf Oct. 11, 2019, 9:36 a.m. UTC | #2
Am 09.10.2019 um 21:47 hat Cleber Rosa geschrieben:
> Due to not being able to find a reason to have shebangs on files that
> are not executable.
> 
> While at it, add a mode hint to emacs, which would be clueless or
> plain wrong about these containing shell code.

vim still doesn't like the change.

Of course, we could also add another line for vim and for every other
editor in use, but actually, I think I'd prefer just dropping this
patch. It even makes each file a few bytes larger instead of saving
something. Shebang lines are a shorter and more portable format
indicator than the alternatives.

So I think in the end we have found a good reason to keep them. :-)

Kevin
Nir Soffer Oct. 11, 2019, 11:27 a.m. UTC | #3
On Fri, Oct 11, 2019, 12:36 Kevin Wolf <kwolf@redhat.com> wrote:

> Am 09.10.2019 um 21:47 hat Cleber Rosa geschrieben:
> > Due to not being able to find a reason to have shebangs on files that
> > are not executable.
> >
> > While at it, add a mode hint to emacs, which would be clueless or
> > plain wrong about these containing shell code.
>
> vim still doesn't like the change.
>
> Of course, we could also add another line for vim and for every other
> editor in use, but actually, I think I'd prefer just dropping this
> patch. It even makes each file a few bytes larger instead of saving
> something. Shebang lines are a shorter and more portable format
> indicator than the alternatives.
>
> So I think in the end we have found a good reason to keep them. :-)
>

What about .sh suffix? Should be most portable way.

>
Cleber Rosa Oct. 11, 2019, 8:05 p.m. UTC | #4
On Fri, Oct 11, 2019 at 02:27:25PM +0300, Nir Soffer wrote:
> On Fri, Oct 11, 2019, 12:36 Kevin Wolf <kwolf@redhat.com> wrote:
> 
> > Am 09.10.2019 um 21:47 hat Cleber Rosa geschrieben:
> > > Due to not being able to find a reason to have shebangs on files that
> > > are not executable.
> > >
> > > While at it, add a mode hint to emacs, which would be clueless or
> > > plain wrong about these containing shell code.
> >
> > vim still doesn't like the change.
> >
> > Of course, we could also add another line for vim and for every other
> > editor in use, but actually, I think I'd prefer just dropping this
> > patch. It even makes each file a few bytes larger instead of saving
> > something. Shebang lines are a shorter and more portable format
> > indicator than the alternatives.
> >
> > So I think in the end we have found a good reason to keep them. :-)
> >
> 
> What about .sh suffix? Should be most portable way.
> 
> >

That's the approach I tend to follow for my sh code.  Explicit is
better than implicit if you ask me.

Kevin,

Do you have any strong feelings here?  I'd be fine with either this
or dropping the patch.

Thanks,
- Cleber.
Kevin Wolf Oct. 11, 2019, 8:30 p.m. UTC | #5
Am 11.10.2019 um 22:05 hat Cleber Rosa geschrieben:
> On Fri, Oct 11, 2019 at 02:27:25PM +0300, Nir Soffer wrote:
> > On Fri, Oct 11, 2019, 12:36 Kevin Wolf <kwolf@redhat.com> wrote:
> > 
> > > Am 09.10.2019 um 21:47 hat Cleber Rosa geschrieben:
> > > > Due to not being able to find a reason to have shebangs on files that
> > > > are not executable.
> > > >
> > > > While at it, add a mode hint to emacs, which would be clueless or
> > > > plain wrong about these containing shell code.
> > >
> > > vim still doesn't like the change.
> > >
> > > Of course, we could also add another line for vim and for every other
> > > editor in use, but actually, I think I'd prefer just dropping this
> > > patch. It even makes each file a few bytes larger instead of saving
> > > something. Shebang lines are a shorter and more portable format
> > > indicator than the alternatives.
> > >
> > > So I think in the end we have found a good reason to keep them. :-)
> > 
> > What about .sh suffix? Should be most portable way.
> 
> That's the approach I tend to follow for my sh code.  Explicit is
> better than implicit if you ask me.

I would certainly agree for new files.

> Kevin,
> 
> Do you have any strong feelings here?  I'd be fine with either this
> or dropping the patch.

No strong feelings. The result of renaming the files would be a bit
nicer than what we have today, but renaming always comes with a cost
when working with the version history later. Hard to tell if it's a net
gain or loss in the end.

Myself, I would probably pick the lazy way and stick with "if it ain't
broke, don't fix it", but I'm not objecting to a change either.

Kevin

Patch
diff mbox series

diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config
index 9bd1a5a6fc..b85a6a6f96 100644
--- a/tests/qemu-iotests/common.config
+++ b/tests/qemu-iotests/common.config
@@ -1,4 +1,4 @@ 
-#!/usr/bin/env bash
+# -*- emacs mode: sh -*-
 #
 # Copyright (C) 2009 Red Hat, Inc.
 # Copyright (c) 2000-2003,2006 Silicon Graphics, Inc.  All Rights Reserved.
diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
index 9f418b4881..a0b52b0ac8 100644
--- a/tests/qemu-iotests/common.filter
+++ b/tests/qemu-iotests/common.filter
@@ -1,4 +1,4 @@ 
-#!/usr/bin/env bash
+# -*- emacs mode: sh -*-
 #
 # Copyright (C) 2009 Red Hat, Inc.
 # Copyright (c) 2000-2001 Silicon Graphics, Inc.  All Rights Reserved.
diff --git a/tests/qemu-iotests/common.nbd b/tests/qemu-iotests/common.nbd
index 24b01b60aa..a98d05d404 100644
--- a/tests/qemu-iotests/common.nbd
+++ b/tests/qemu-iotests/common.nbd
@@ -1,5 +1,4 @@ 
-#!/usr/bin/env bash
-# -*- shell-script-mode -*-
+# -*- emacs mode: sh -*-
 #
 # Helpers for NBD server related config
 #
diff --git a/tests/qemu-iotests/common.pattern b/tests/qemu-iotests/common.pattern
index 4f5e5bcea0..4028b32c54 100644
--- a/tests/qemu-iotests/common.pattern
+++ b/tests/qemu-iotests/common.pattern
@@ -1,4 +1,4 @@ 
-#!/usr/bin/env bash
+# -*- emacs mode: sh -*-
 #
 # Copyright (C) 2009 Red Hat, Inc.
 #
diff --git a/tests/qemu-iotests/common.qemu b/tests/qemu-iotests/common.qemu
index 8d2021a7eb..66569f6bdd 100644
--- a/tests/qemu-iotests/common.qemu
+++ b/tests/qemu-iotests/common.qemu
@@ -1,4 +1,4 @@ 
-#!/usr/bin/env bash
+# -*- emacs mode: sh -*-
 #
 # This allows for launching of multiple QEMU instances, with independent
 # communication possible to each instance.
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index e45cdfa66b..aa4a7fcc11 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -1,4 +1,4 @@ 
-#!/usr/bin/env bash
+# -*- emacs mode: sh -*-
 #
 # Copyright (C) 2009 Red Hat, Inc.
 # Copyright (c) 2000-2006 Silicon Graphics, Inc.  All Rights Reserved.
diff --git a/tests/qemu-iotests/common.tls b/tests/qemu-iotests/common.tls
index 54c331d7a5..759a3d6d71 100644
--- a/tests/qemu-iotests/common.tls
+++ b/tests/qemu-iotests/common.tls
@@ -1,4 +1,4 @@ 
-#!/usr/bin/env bash
+# -*- emacs mode: sh -*-
 #
 # Helpers for TLS related config
 #