[libvirt] Remove bashisms from libvirt-guests

[Laurent Léonard]

Le vendredi 17 décembre 2010 15:08:44, Eric Blake a écrit :
> On 12/17/2010 04:09 AM, Laurent Léonard wrote:
> > Hi,
> > 
> > The attached patch removes bashisms from libvirt-guests.
> > 
> > TEXTDOMAINDIR is not specified, so system default will be used
> > ("/usr/share/locale" on Debian, I don't know if it's the same on Fedora).
> 
> Hmm; that should be derived from one of the autoconf-provided
> installation directory names; I'll look more into the correct value to
> use (that is, it should line up with whatever ./configure --prefix=...
> you used, by using some form of @prefix@ in the .in version of the
> script).  For example, see how we set $sysconfdir.
> 
> > "xgettext -L Shell" output is the same with gettext shell functions as
> > with $"..." deprecated Bash-specific syntax.
> > 
> > Please generate po files somewhere in the source tree.
> 
> That's just a matter of modifying po/POTFILES.in to recognize that our
> init scripts have translatable strings.
> 
> > +++ b/tools/libvirt-guests.init.in
> > @@ -31,6 +31,11 @@ libvirtd=@sbindir@/libvirtd
> > 
> >  # Source function library.
> >  . "$sysconfdir"/rc.d/init.d/functions
> > 
> > +. gettext.sh
> 
> Must be specified as an absolute path, to avoid accidentally sourcing a
> trojan script.
> 
> > +
> > +TEXTDOMAIN=libvirt-guests
> > +export TEXTDOMAIN
> 
> We can assume POSIX sh here, and do this on one line instead of two.
> And the domain should be libvirt, rather than libvirt-guests (that is,
> share the same .mo files as the rest of libvirt).
> 
> >      if [ "x$ON_BOOT" != xstart ]; then
> > 
> > -        echo $"libvirt-guests is configured not to start any guests on
> > boot" +        gettext "libvirt-guests is configured not to start any
> > guests on boot"; echo
> 
> So gettext(1) doesn't output a trailing newline?  Okay.
> 
> >      name=$(guest_name $uri $guest)
> > 
> > -    label=$"Suspending $name: "
> > +    label="`eval_gettext \"Suspending \\$name: \"`"
> 
> Assume a POSIX sh; we do NOT want to use `` in this script, and the
> outer "" are not necessary in assignment context.  This is much more
> legible as:
> 
> label=$(eval_gettext "Suspending \$name: ")
> 
> >      echo -n "$label"
> >      run_virsh $uri managedsave $guest >/dev/null &
> >      virsh_pid=$!
> > 
> > @@ -187,7 +192,7 @@ suspend_guest()
> > 
> >              printf '\r%s%-12s ' "$label" "..."
> >          
> >          fi
> >      
> >      done
> > 
> > -    retval wait $virsh_pid && printf '\r%s%-12s\n' "$label" $"done"
> > +    retval wait $virsh_pid && printf '\r%s%-12s\n' "$label" "`gettext
> > \"done\"`"
> 
> Likewise: "$(gettext "done")"
> 
> But thanks for taking this on; we're getting closer to a nice solution.

I followed:
http://www.gnu.org/software/hello/manual/gettext/Preparing-Shell-Scripts.html
but totally agree with your comments.

Indeed gettext(1) doesn't output a trailing newline.

Don't forget the
-        timeout=$[timeout - 1]
+        timeout=$((timeout - 1))
I also added in the "Remove bashisms" patch, if you did'nt noticed it.

Thank you,

-- 
Laurent Léonard