Re: [libvirt] Remove bashisms from libvirt-guests

On 01/04/2011 11:13 AM, Laurent Léonard wrote: Thank you for the work. Phooey - 'make syntax-check' doesn't like this. Changing it to $(PACKAGE) instead of @PACKAGE@ solved that, though. And in the process, I added better makefile quoting, to avoid other (unlikely) issues with spaces in $(bindir), for example. I also had to add a comment with the string _("dummy") in it in order to keep 'make syntax-check' happy on sc_po_check (otherwise it complained about adding libvirt-guests.init.in to POTFILES.in). Just to be safe in case @bindir@ contains spaces, I'm changing this to: . &quot;@bindir(a)&quot;/gettext.sh POSIX requires the shorter form to work, plus more quoting for safety: export TEXTDOMAIN="@PACKAGE@" 'echo -n' is a bash-ism (and worse, a non-portable bash-ism, since 'shopt -s xpg_echo' disables it). I've replaced all 'echo -n' with 'printf %s'. I also broke up some lines where your patch made things go longer than 80 columns. this one was already longer than 80 columns, but I broke it up as well. Here's what I'm planning on squashing in. However, I still have one nagging problem, that I haven't been able to figure out yet - even though we listed libvirt-guests.init.in in po/POTFILES.in, xgettext doesn't seem to be picking it up into po/libvirt.pot. So until I can figure that out, I'm holding off on pushing this just a bit longer. diff --git i/tools/Makefile.am w/tools/Makefile.am index de5f662..87cf9bd 100644 --- i/tools/Makefile.am +++ w/tools/Makefile.am @@ -146,12 +146,12 @@ BUILT_SOURCES += libvirt-guests.init libvirt-guests.init: libvirt-guests.init.in $(top_builddir)/config.status $(AM_V_GEN)sed \ - -e s!\@PACKAGE\@!@PACKAGE@!g \ - -e s!\@bindir\@!@bindir@!g \ - -e s!\@localedir\@!@localedir@!g \ - -e s!\@localstatedir\@!@localstatedir@!g \ - -e s!\@sbindir\@!@sbindir@!g \ - -e s!\@sysconfdir\@!@sysconfdir@!g \ + -e 's!\@PACKAGE\@!$(PACKAGE)!g' \ + -e 's!\@bindir\@!$(bindir)!g' \ + -e 's!\@localedir\@!$(localedir)!g' \ + -e 's!\@localstatedir\@!$(localstatedir)!g' \ + -e 's!\@sbindir\@!$(sbindir)!g' \ + -e 's!\@sysconfdir\@!$(sysconfdir)!g' \ < $< > $@-t && \ chmod a+x $@-t && \ mv $@-t $@ diff --git i/tools/libvirt-guests.init.in w/tools/libvirt-guests.init.in index ea404b7..8823d06 100644 --- i/tools/libvirt-guests.init.in +++ w/tools/libvirt-guests.init.in @@ -24,27 +24,27 @@ # See http://libvirt.org # -sysconfdir=@sysconfdir@ -localstatedir=@localstatedir@ -libvirtd=@sbindir@/libvirtd +sysconfdir="@sysconfdir@" +localstatedir="@localstatedir@" +libvirtd="@sbindir@"/libvirtd # Source function library. test ! -r "$sysconfdir"/rc.d/init.d/functions || - . "$sysconfdir"/rc.d/init.d/functions + . "$sysconfdir"/rc.d/init.d/functions -. @bindir(a)/gettext.sh +# Source gettext library. +# Make sure this file is recognized as having translations: _("dummy") +. &quot;@bindir(a)&quot;/gettext.sh -TEXTDOMAIN=@PACKAGE@ -export TEXTDOMAIN -TEXTDOMAINDIR=@localedir@ -export TEXTDOMAINDIR +export TEXTDOMAIN="@PACKAGE@" TEXTDOMAINDIR="@localedir@" URIS=default ON_BOOT=start ON_SHUTDOWN=suspend SHUTDOWN_TIMEOUT=0 -test -f "$sysconfdir"/sysconfig/libvirt-guests && . "$sysconfdir"/sysconfig/libvirt-guests +test -f "$sysconfdir"/sysconfig/libvirt-guests && + . "$sysconfdir"/sysconfig/libvirt-guests LISTFILE="$localstatedir"/lib/libvirt/libvirt-guests VAR_SUBSYS_LIBVIRT_GUESTS="$localstatedir"/lock/subsys/libvirt-guests @@ -136,7 +136,8 @@ start() { [ -f "$LISTFILE" ] || { started; return 0; } if [ "x$ON_BOOT" != xstart ]; then - gettext "libvirt-guests is configured not to start any guests on boot"; echo + gettext "libvirt-guests is configured not to start any guests on boot" + echo rm -f "$LISTFILE" started return 0 @@ -181,7 +182,7 @@ suspend_guest() name=$(guest_name $uri $guest) label=$(eval_gettext "Suspending \$name: ") - echo -n "$label" + printf %s "$label" run_virsh $uri managedsave $guest >/dev/null & virsh_pid=$! while true; do @@ -205,7 +206,7 @@ shutdown_guest() name=$(guest_name $uri $guest) label=$(eval_gettext "Shutting down \$name: ") - echo -n "$label" + printf %s "$label" retval run_virsh $uri shutdown $guest >/dev/null || return timeout=$SHUTDOWN_TIMEOUT while [ $timeout -gt 0 ]; do @@ -218,7 +219,8 @@ shutdown_guest() if guest_is_on $uri $guest; then if $guest_running; then - printf '\r%s%-12s\n' "$label" "$(gettext "failed to shutdown in time")" + printf '\r%s%-12s\n' "$label" \ + "$(gettext "failed to shutdown in time")" else printf '\r%s%-12s\n' "$label" "$(gettext "done")" fi @@ -233,7 +235,8 @@ stop() { if [ "x$ON_SHUTDOWN" = xshutdown ]; then suspending=false if [ $SHUTDOWN_TIMEOUT -le 0 ]; then - gettext "Shutdown action requested but SHUTDOWN_TIMEOUT was not set"; echo + gettext "Shutdown action requested but SHUTDOWN_TIMEOUT was not set" + echo RETVAL=6 return fi @@ -253,7 +256,7 @@ stop() { empty=true for uuid in $list; do $empty || printf ", " - echo -n $(guest_name $uri $uuid) + printf %s "$(guest_name $uri $uuid)" empty=false done if $empty; then @@ -313,7 +316,8 @@ rh_status() { # Display usage string, then exit with VAL (defaults to 2). usage() { program_name=$0 - eval_gettext "Usage: \$program_name {start|stop|status|restart|condrestart|try-restart|reload|force-reload|gueststatus|shutdown}"; echo + eval_gettext "Usage: \$program_name {start|stop|status|restart|"\ +"condrestart|try-restart|reload|force-reload|gueststatus|shutdown}"; echo exit ${1-2} } -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org