[GRASS-dev] [bug #5499] (grass) bash scripts starting with #!/bin/sh

[Andreas_Lange_via_RT]

guest wrote (Sun, Feb 18 2007 16:47:52):

I experienced problems with Ubuntu 6.10 installation. The r.in.wms script
refused to work with errors like this:
/usr/lib/grass/etc/r.in.wms//wms.request: 326: SIZE_ARRAY[0]=: not found
/usr/lib/grass/scripts/r.tileset: 193: declare: not found
/usr/lib/grass/scripts/r.tileset: 258: Syntax error: Bad for loop variable

It took me about a couple of hours to find out that the reason is that
Ubuntu developers decided to change the symbolic link /bin/sh from /bin/bash
to /bin/dash, which is POSIX-compliant, but much less feature-rich than
bash.

Setting it back to /bin/bash solved the problem, but still indicating the
right interpreter in the headers will also be great.

Hi,

Thanks for the report and the hint of what is going on. Well spotted.

However, please don't report to this tracker in future - read the
http://grass.itc.it/bugtracking/index.php. We are working on disabling the old
RT for good.

Since the issue has been reported twice to RT anyway, I'll let myself to
continue here, especially that it seems big.

AFAIK, GRASS shell scripts are supposed to work with any POSIX 1003.2
compliant sh interpreter. Apparently some still don't, eg. r.tileset and
r.in.wms as we can see.

As the bash->dash switch in Ubuntu 6.1 seems inrevertable (see:
https://launchpad.net/ubuntu/+source/dash/+bug/61463, the comment by Matthew
Garrett at 2006-11-29 05:23:16 UTC, in particular), and it looks justified
from technical point of view, we have 3 options it seems:

1. Have (some) GRASS sh scripts broken for Ubuntu (what if other distros
follow Ubuntu?). What with platforms not providing bash?

2. Revise all the scripts and fix all bashisms; buts:
- who would do it?
- it takes quite some knowledge to decide what needs to be fixed, and it could
take much time
- what with the addons? remove all that are not POSIX-compliant? fix them
ourselves? Anyway, much work.

3. Change all GRASS sh scripts to require #/bin/bash. Least work. But what
with platforms not providing bash?

Maciek

-------------------------------------------- Managed by Request Tracker

[Glynn_Clements1]

Maciek Sieczka via RT wrote:

AFAIK, GRASS shell scripts are supposed to work with any POSIX 1003.2
compliant sh interpreter. Apparently some still don't, eg. r.tileset and
r.in.wms as we can see.

As the bash->dash switch in Ubuntu 6.1 seems inrevertable (see:
https://launchpad.net/ubuntu/+source/dash/+bug/61463, the comment by Matthew
Garrett at 2006-11-29 05:23:16 UTC, in particular), and it looks justified
from technical point of view,

Note that NetBSD has always used "ash" as its /bin/sh, as do a number
of embedded Linux distributions. Also, most commercial Unices use
something other than bash as their /bin/sh.

In general, assuming /bin/sh == bash is a bad idea.

we have 3 options it seems:

1. Have (some) GRASS sh scripts broken for Ubuntu (what if other distros
follow Ubuntu?). What with platforms not providing bash?

The only sensible option, IMHO.

2. Revise all the scripts and fix all bashisms; buts:
- who would do it?
- it takes quite some knowledge to decide what needs to be fixed, and it could
take much time
- what with the addons? remove all that are not POSIX-compliant? fix them
ourselves? Anyway, much work.

Change them to #!/usr/local/bin/bash. Users who have bash will have to
change the path; users who don't have bash will at least get a
reasonably straightforward error message, e.g.:

  /usr/local/bin/bash: bad interpreter: No such file or directory

rather than obscure problems with the script (or, worse still, no
error but bogus results).

Either way, most users will have an incentive to either fix the script
or at least provide us with a continual stream of reminders that our
scripts are broken.

3. Change all GRASS sh scripts to require #/bin/bash. Least work. But what
with platforms not providing bash?

Or not installing it in /bin (i.e. probably anything where bash isn't
the primary shell, e.g. most commercial Unices).

Note that /bin (and /sbin, /lib etc) are meant for programs which need
to work when /usr doesn't exist (e.g. if /usr is on NFS, anything
which is required to configure and enable networking can't go under
/usr). If bash isn't a core part of the OS (i.e. it isn't /bin/sh),
there's a good chance that it won't be in /bin.

--
Glynn Clements <glynn@gclements.plus.com>

[H_B]

> I experienced problems with Ubuntu 6.10 installation. The r.in.wms
> script refused to work with errors like this:
> /usr/lib/grass/etc/r.in.wms//wms.request: 326: SIZE_ARRAY[0]=: not
> found /usr/lib/grass/scripts/r.tileset: 193: declare: not found
> /usr/lib/grass/scripts/r.tileset: 258: Syntax error: Bad for loop
> variable
>
> It took me about a couple of hours to find out that the reason is
> that Ubuntu developers decided to change the symbolic link /bin/sh
> from /bin/bash to /bin/dash, which is POSIX-compliant, but much less
> feature-rich than bash.
>
> Setting it back to /bin/bash solved the problem, but still
> indicating the right interpreter in the headers will also be great.

Maciek:

AFAIK, GRASS shell scripts are supposed to work with any POSIX 1003.2
compliant sh interpreter. Apparently some still don't, eg. r.tileset
and r.in.wms as we can see.

As the bash->dash switch in Ubuntu 6.1 seems inrevertable (see:
https://launchpad.net/ubuntu/+source/dash/+bug/61463, the comment by
Matthew Garrett at 2006-11-29 05:23:16 UTC, in particular), and it
looks justified from technical point of view, we have 3 options it
seems:

1. Have (some) GRASS sh scripts broken for Ubuntu (what if other
distros follow Ubuntu?). What with platforms not providing bash?

Markus and others have gone to a lot of trouble to remove all bashisms
from all the scripts/. AFAIK this is an isolated problem with r.tileset.
All we need to do is fix that and the job is done, all other scripts
should be free of Bashisms already.

2. Revise all the scripts and fix all bashisms; buts:
- who would do it?

Preferably the module's author. "No Bashisms" is in SUBMITTING_SCRIPTS.
But shell scripts are pretty easy to debug by adding -x.

Everyone can play along at home by changing the first line of the script
to "#!/bin/ash -x" and follow the output trail.

- it takes quite some knowledge to decide what needs to be fixed,

perhaps. some hints on common bashisms:

--
export VAR=foo
must be written as
VAR=foo
export VAR
--
$((exp)) doesn't work. use `expr` instead
--
use "=" not "==" for string compares
--

and it could take much time

probably not, if it is limited to r.in.wms + its support scripts.

- what with the addons? remove all that are not POSIX-compliant? fix
them ourselves? Anyway, much work.

Not our problem. They are provided as-is by their authors.

3. Change all GRASS sh scripts to require #/bin/bash. Least work. But
what with platforms not providing bash?

Not needed as it is only one script (family) that is broken?

Anyway, bug #5499 is from r.tileset:

if [ -z "$GIS_OPT_DESTSCALE" ] ; then
  eval `g.proj -p | $GREP meters | $SED "s/\\s*:\\s*/=/"`
  DEST_SCALE=$meters;

G63> g.proj -p | grep meters | sed "s/\\s*:\\s*/=/"
units = meters
meters = 1.0

^^^ it's trying to run that string as a program....

-x debug output:

+ eval units = meters meters = 1.0
++ units = meters meters = 1.0
Too many arguments (maybe you need quotes).
Try `units --help' for more information.

r.tileset --help
..
Parameters:
  destscale Conversion factor from units to meters in source projection

I propose this fix:

Index: r.tileset

RCS file:
/home/grass/grassrepository/grass6/scripts/r.tileset/r.tileset,v
retrieving revision 1.3
diff -u -r1.3 r.tileset
--- r.tileset 19 Aug 2006 12:52:24 -0000 1.3
+++ r.tileset 19 Feb 2007 04:26:44 -0000
@@ -393,7 +393,7 @@
        message 0 "Getting projection scale:"

if [ -z "$GIS_OPT_DESTSCALE" ] ; then
- eval `g.proj -p | $GREP meters | $SED "s/\\s*:\\s*/=/"`
+ eval `g.proj -p | $GREP '^meters' | $SED -e "s/\\s*:\\s*/=/" -e "s/ //g"`
        DEST_SCALE=$meters;
else
        DEST_SCALE=$GIS_OPT_DESTSCALE

(my regex sucks, what does the "\\s*" part of "\\s*:\\s*" match?)

And adding exit code checks at vital junctures in the r.in.wms script so
it stops as soon as it breaks.

r.in.wms then goes on, but gdalwarp fails because
$GISDBASE/wms_download/wms_global_mosaic__0.geotiff is text not an image:

<?xml version='1.0' encoding="UTF-8" standalone="no" ?>
<!DOCTYPE ServiceExceptionReport SYSTEM "http://www.digitalearth.gov/wmt/xml/exception_1_1_0.dtd ">
<ServiceExceptionReport version="1.1.0">
  <ServiceException code="InvalidSRS">
    The SRS "EPSG:4230" is invalid.
  </ServiceException>
</ServiceExceptionReport>

(...proof that more sanity checks needed in the script!)

Hamish

[Glynn_Clements1]

Hamish wrote:

(my regex sucks, what does the "\\s*" part of "\\s*:\\s*" match?)

My guess would be that it's supposed to match any "whitespace"
character, but that's non-standard (it works in GNU sed 4.1.5, but
doesn't appear to be documented in the sed Info file). More portable
solutions include:

1. Just use a space.

2. Use a set containing a space or a tab, i.e. [ <tab>] where <tab> is
a literal tab character.

3. Use [:space:] or [:blank:]. The latter matches space or tab, while
the former matches a locale-dependent set; in the C/POSIX locale, it
matches space, tab, newline, carriage-return, form-feed and
vertical-tab.

--
Glynn Clements <glynn@gclements.plus.com>