cckec / winetricks

Automatically exported from code.google.com/p/winetricks
0 stars 0 forks source link

SVN 220: Checkbashism warnings #3

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
The checkbashishms[1] can be used to test against constructs that may be better 
handled in other ways to ensure best compatibility. See report below.

You may also find following page informative to improve the script
<http://mywiki.wooledge.org/BashPitfalls>, especially item (6) which talks 
about:

   [ bar = "$foo" -a foo = "$bar" ]     # Not portable.

and suggests to use:

   [ bar = "$foo" ] && [ foo = "$bar" ]

[1] http://packages.debian.org/devscripts

--------------------------------------------------------------------
[svn 220]

possible bashism in debian/winetricks/usr/bin/winetricks line 416 (test -a/-o):
    if test "$3" -a ! "$checksum_ok"
possible bashism in debian/winetricks/usr/bin/winetricks line 1329 (echo -n):
        echo -n "zenity \
            --title '$_W_msg_title' \
            --text '$_W_msg_body' \
            --list \
            --radiolist \
            --column '' \
            --column '' \
            --column '' \
            --height $WINETRICKS_MENU_HEIGHT \
            --width $WINETRICKS_MENU_WIDTH \
            --hide-column 2 \
            FALSE games      '$_W_msg_new' \
            TRUE  main       '$_W_msg_default' \
            " \
            > "$WINETRICKS_WORKDIR"/zenity.sh
possible bashism in debian/winetricks/usr/bin/winetricks line 1337 (echo -n):
                echo -n " FALSE prefix=$p 'Modify wineprefix $p ($W_PREFIXES_ROOT/$p)' "
possible bashism in debian/winetricks/usr/bin/winetricks line 1340 (echo -n):
        echo -n " FALSE $_W_cmd_unattended '$_W_msg_unattended'" >> $WINETRICKS_WORKDIR/zenity.sh
possible bashism in debian/winetricks/usr/bin/winetricks line 1421 (echo -n):
          echo -n "zenity \
            --title '$_W_msg_title' \
            --text '$_W_msg_body' \
            --list \
            --radiolist \
            --column '' \
            --column '' \
            --column '' \
            --height $WINETRICKS_MENU_HEIGHT \
            --width $WINETRICKS_MENU_WIDTH \
            --hide-column 2 \
            FALSE dlls       '$_W_msg_dlls' \
            FALSE fonts      '$_W_msg_fonts' \
            FALSE benchmarks '$_W_msg_benchmarks' \
            FALSE apps       '$_W_msg_apps' \
            FALSE settings   '$_W_msg_settings' \
            FALSE winecfg    '$_W_msg_winecfg' \
            FALSE regedit    '$_W_msg_regedit' \
            FALSE taskmgr    '$_W_msg_taskmgr' \
            FALSE shell      '$_W_msg_shell' \
            FALSE annihilate '$_W_msg_annihilate' \
         "
possible bashism in debian/winetricks/usr/bin/winetricks line 1507 (echo -n):
            echo -n " " FALSE \
                    $code \
                    "\"$title\""
possible bashism in debian/winetricks/usr/bin/winetricks line 1516 (echo -n):
        echo -n "kdialog --geometry 600x400+100+100 --title '$_W_msg_title' --separate-output --checklist '$_W_msg_body' "
possible bashism in debian/winetricks/usr/bin/winetricks line 1627 (echo -n):
                echo -n " " $installed \
                    $code \
                    "\"$title\"" \
                    "\"$publisher\"" \
                    "\"$year\"" \
                    "\"$media\"" \
                    "\"$flags\"" \
                    "\"$size_MB\"" \
                    "\"$time_sec\"" 
possible bashism in debian/winetricks/usr/bin/winetricks line 1641 (echo -n):
        echo -n "kdialog --geometry 600x400+100+100 --title '$_W_msg_title' --separate-output --checklist '$_W_msg_body' "
possible bashism in debian/winetricks/usr/bin/winetricks line 2483 (trap with 
signal numbers):
    trap winetricks_kill_handler 0 1 2 3 6

Original issue reported on code.google.com by jari.aalto.fi@gmail.com on 5 Mar 2011 at 12:40

GoogleCodeExporter commented 8 years ago
Thanks for running that checker.  

http://www.in-ulm.de/~mascheck/various/echo+printf/ has a great chart
showing what systems don't support echo -n.  Solaris is listed.
We need to replace echo -n with printf %s carefully.

The two lines that use trap numbers and test -a also need fixing.

Original comment by daniel.r...@gmail.com on 5 Mar 2011 at 4:02

GoogleCodeExporter commented 8 years ago
On Mac OS 10.6, #!/bin/sh gets a compatibility mode where the shell-builtin 
echo does not support "echo -n".

As a result, winetricks is unusable:

% winetricks apps
/tmp/w.jeff.54607/zenity.sh: line 1: -n: command not found
/tmp/w.jeff.54607/zenity.sh: line 2: -n: command not found
/tmp/w.jeff.54607/zenity.sh: line 3: -n: command not found
/tmp/w.jeff.54607/zenity.sh: line 4: -n: command not found
[ ... ]

Simply changing the first line to #!/bin/bash makes it all better.

Original comment by jm...@cornell.edu on 21 Mar 2011 at 3:29

GoogleCodeExporter commented 8 years ago
Alternatively, using "/bin/echo" in place of "echo" would work as well.

Original comment by jm...@cornell.edu on 21 Mar 2011 at 3:50

GoogleCodeExporter commented 8 years ago
Thanks!

Original comment by daniel.r...@gmail.com on 21 Mar 2011 at 6:44

GoogleCodeExporter commented 8 years ago
Issue 8 has been merged into this issue.

Original comment by daniel.r...@gmail.com on 21 Mar 2011 at 6:45

GoogleCodeExporter commented 8 years ago
r348 uses printf %s instead of echo -n, should make things work better on mac.

Original comment by daniel.r...@gmail.com on 23 Mar 2011 at 12:13

GoogleCodeExporter commented 8 years ago
I tried the one with prinftf %s from svn ... it seems to be working fine for me 
now!

Original comment by doh...@gmail.com on 23 Mar 2011 at 1:35

GoogleCodeExporter commented 8 years ago
To explain why I chose printf:
#!/bin/bash would not help keep the script portable, could break it on some 
systems.  I prefer to stay with posix.
/bin/echo is a bit of a gamble, too, since some systems might place it in 
/usr/bin.
winetricks already uses printf, so that seemed like the right way to get rid of 
echo -n.

Original comment by daniel.r...@gmail.com on 24 Mar 2011 at 3:07

GoogleCodeExporter commented 8 years ago
Trap and test -a fixed as of r355.  "checkbashims -p -x winetricks" now clean.

Original comment by daniel.r...@gmail.com on 24 Mar 2011 at 3:58