[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] Remove bashisms from libvirt-guests



On 01/04/2011 11:13 AM, Laurent Léonard wrote:
> 
> The attached patch should match with your comments.
> 
> Thank you,

Thank you for the work.

> +++ b/tools/Makefile.am
> @@ -146,6 +146,9 @@ 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		\

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).

> +++ b/tools/libvirt-guests.init.in
> @@ -32,6 +32,13 @@ libvirtd= sbindir@/libvirtd
>  test ! -r "$sysconfdir"/rc.d/init.d/functions ||
>    . "$sysconfdir"/rc.d/init.d/functions
>  
> +. @bindir@/gettext.sh

Just to be safe in case @bindir@ contains spaces, I'm changing this to:

. "@bindir@"/gettext.sh

> +
> +TEXTDOMAIN= PACKAGE@
> +export TEXTDOMAIN

POSIX requires the shorter form to work, plus more quoting for safety:

export TEXTDOMAIN="@PACKAGE@"

> @@ -173,7 +180,7 @@ suspend_guest()
>      guest=$2
>  
>      name=$(guest_name $uri $guest)
> -    label=$"Suspending $name: "
> +    label=$(eval_gettext "Suspending \$name: ")
>      echo -n "$label"

'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'.

> @@ -226,7 +233,7 @@ stop() {
>      if [ "x$ON_SHUTDOWN" = xshutdown ]; then
>          suspending=false
>          if [ $SHUTDOWN_TIMEOUT -le 0 ]; then
> -            echo $"Shutdown action requested but SHUTDOWN_TIMEOUT was not set"
> +            gettext "Shutdown action requested but SHUTDOWN_TIMEOUT was not set"; echo

I also broke up some lines where your patch made things go longer than
80 columns.

> @@ -305,7 +312,8 @@ rh_status() {
>  # usage [val]
>  # Display usage string, then exit with VAL (defaults to 2).
>  usage() {
> -    echo $"Usage: $0 {start|stop|status|restart|condrestart|try-restart|reload|force-reload|gueststatus|shutdown}"
> +    program_name=$0
> +    eval_gettext "Usage: \$program_name {start|stop|status|restart|condrestart|try-restart|reload|force-reload|gueststatus|shutdown}"; echo

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@/gettext.sh
+# Source gettext library.
+# Make sure this file is recognized as having translations: _("dummy")
+. "@bindir@"/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 redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]