running sh scripts through shellcheck

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

running sh scripts through shellcheck

Samba - samba-technical mailing list

OK, I have got to ctdb/tests/complex/30_nfs_tickle_killtcp.sh where I
have found this:

echo "Getting MAC address associated with ${test_ip}..."
releasing_mac=$(ip neigh show "$test_prefix" | awk '$4 == "lladdr" {print $5}')
[ -n "$releasing_mac" ] || die "Couldn't get MAC address for ${test_prefix}"
echo "MAC address is: ${releasing_mac}"

So, who doesn't know that 'die' is a perl thing ?

There doesn't seem to be a 'die' function.

The script is also a 'bash' script instead of a 'sh' script.

Rowland

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: running sh scripts through shellcheck

Samba - samba-technical mailing list
Hi Rowland,

On Fri, 11 Aug 2017 09:20:33 +0100, Rowland Penny via samba-technical
<[hidden email]> wrote:

> OK, I have got to ctdb/tests/complex/30_nfs_tickle_killtcp.sh where I
> have found this:
>
> echo "Getting MAC address associated with ${test_ip}..."
> releasing_mac=$(ip neigh show "$test_prefix" | awk '$4 == "lladdr" {print $5}')
> [ -n "$releasing_mac" ] || die "Couldn't get MAC address for ${test_prefix}"
> echo "MAC address is: ${releasing_mac}"
>
> So, who doesn't know that 'die' is a perl thing ?
>
> There doesn't seem to be a 'die' function.

There is a die() function in ctdb/tests/scripts/common.sh.  It is
included somewhat indirectly.  ;-)

> The script is also a 'bash' script instead of a 'sh' script.

Yes, for historical reasons the CTDB integration tests are bash
scripts.  Nearly all other non-test scripts and unit test scripts are
"sh".  The "onnode" command is one exception because it is (nearly?)
impossible to write in POSIX shell.

Please don't spend too much time trying to get the CTDB tests to pass
shellcheck.  You'll go crazier than the person who wrote most of
the tests.  ;-)

One reason for the complexity of the CTDB test subsystem is that it
can be run in 3 ways:

* From within the git tree

* From within an unpacked tarball

* Installed on a test node - the "simple" and "complex" suites can be
  run against a real cluster

In ctdb/tests/shellcheck/ there is a little testsuite that runs all of
the non-test CTDB scripts through shellcheck.  There are a few
exceptions listed in ctdb/tests/shellcheck/scripts/local.sh so that we
don't run all of the shellcheck "codes".

This can be run via:

  ctdb/tests/run_tests.sh shellcheck

You might notice that I posted some patches to the list today to
improve CTDB's shellcheck goodness.  If you're testing with a recent
version of shellcheck then you might need those patches.  My other post
comments on a test that fails due a regression in shellcheck - that
will be fixed in the next release of shellcheck.

Writing scripts is so much fun...  :-)

peace & happiness,
martin

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: running sh scripts through shellcheck

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
On Fri, 11 Aug 2017 09:20:33 +0100
Rowland Penny via samba-technical <[hidden email]>
wrote:

>
> OK, I have got to ctdb/tests/complex/30_nfs_tickle_killtcp.sh where I
> have found this:
>
> echo "Getting MAC address associated with ${test_ip}..."
> releasing_mac=$(ip neigh show "$test_prefix" | awk '$4 ==
> "lladdr" {print $5}') [ -n "$releasing_mac" ] || die "Couldn't get
> MAC address for ${test_prefix}" echo "MAC address is:
> ${releasing_mac}"
>
> So, who doesn't know that 'die' is a perl thing ?
>
> There doesn't seem to be a 'die' function.
>
> The script is also a 'bash' script instead of a 'sh' script.
>
> Rowland
>


Forget it, found 'die' in ctdb/tests/scripts/common.sh, still think
calling it 'die' was a bad idea though

It still shouldn't be a 'bash' script, it isn't portable, which brings
me to ctdb/tests/complex/32_cifs_tickle.sh

Changing #!/bin/bash to #!/bin/sh causes this:

In ctdb/tests/complex/32_cifs_tickle.sh line 69:
if [ "${out/SRC: ${src_socket} /}" != "$out" ] ; then
      ^-- SC2039: #!/bin/sh was specified, but string replacement is not standard.

I will have to find the relevant code, decipher it and re-write it for
'sh'

Rowland
 

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: running sh scripts through shellcheck

Samba - samba-technical mailing list
Hi Rowland,

I've just read your other reply, but will send this anyway given that
I've added some extra explanation.  :-)

On Fri, 11 Aug 2017 10:45:58 +0100, Rowland Penny via samba-technical
<[hidden email]> wrote:

> On Fri, 11 Aug 2017 09:20:33 +0100
> Rowland Penny via samba-technical <[hidden email]>
> wrote:
>
> >
> > OK, I have got to ctdb/tests/complex/30_nfs_tickle_killtcp.sh where I
> > have found this:
> >
> > echo "Getting MAC address associated with ${test_ip}..."
> > releasing_mac=$(ip neigh show "$test_prefix" | awk '$4 ==
> > "lladdr" {print $5}') [ -n "$releasing_mac" ] || die "Couldn't get
> > MAC address for ${test_prefix}" echo "MAC address is:
> > ${releasing_mac}"
> >
> > So, who doesn't know that 'die' is a perl thing ?
> >
> > There doesn't seem to be a 'die' function.
> >
> > The script is also a 'bash' script instead of a 'sh' script.

> Forget it, found 'die' in ctdb/tests/scripts/common.sh, still think
> calling it 'die' was a bad idea though

Well, you immediately knew what it meant...  ;-)

> It still shouldn't be a 'bash' script, it isn't portable, which brings
> me to ctdb/tests/complex/32_cifs_tickle.sh
>
> Changing #!/bin/bash to #!/bin/sh causes this:
>
> In ctdb/tests/complex/32_cifs_tickle.sh line 69:
> if [ "${out/SRC: ${src_socket} /}" != "$out" ] ; then
>       ^-- SC2039: #!/bin/sh was specified, but string replacement is not standard.
>
> I will have to find the relevant code, decipher it and re-write it for
> 'sh'

CTDB's public IP address handling isn't portable.  It relies on Linux
things like the "ip" command from the "iproute" package.  Given that a
significant subset of CTDB's functionality is not portable, it isn't
important to make the tests portable.

While there might me a small number of shellcheck hits in each
individual test, you'll find a lot more in the include file
ctdb/tests/script/integration.bash.  It just isn't worth it at this
stage...  ;-)

peace & happiness,
martin

Loading...