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

Re: [libvirt] Remove bashisms from libvirt-guests



Le mardi 04 janvier 2011 22:42:15, Eric Blake a écrit :
> 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|g
> > ueststatus|shutdown}" +    program_name=$0
> > +    eval_gettext "Usage: \$program_name
> > {start|stop|status|restart|condrestart|try-restart|reload|force-reload|g
> > ueststatus|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.

Can you tell me what command line you are using to call xgettext ?

"xgettext -L Shell -o - tools/libvirt-guests.init.in" works fine.

Thank you,
-- 
Laurent Léonard


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