[Bug 432259] Review Request: speech-dispatcher - Required for speech synthesis on OLPC XO

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: speech-dispatcher - Required for speech synthesis on OLPC XO


https://bugzilla.redhat.com/show_bug.cgi?id=432259





------- Additional Comments From mtasaka at ioa.s.u-tokyo.ac.jp  2008-06-06 13:48 EST -------
For -7:

* First of all this does not build.
  http://koji.fedoraproject.org/koji/taskinfo?taskID=650399
  - Again please fix %_infodir/dir issue
  - For the issue:
--------------------------------------------------------------
    File listed twice: /usr/lib/python2.5/site-packages/speechd/_test.py
--------------------------------------------------------------
    This file is actually listed twice as
--------------------------------------------------------------
/%{python_sitelib}/speechd/*
%attr(0755,root,root) /%{python_sitelib}/speechd/_test.py
--------------------------------------------------------------
    which is wrong.
    If you want to change the permission, change the permision
    by not using %attr but by using "chmod" at %install.

* init scripts patch
  - Don't create init script by patch but rather include the script
    directly in the srpm as %SOURCEx.

* Duplicate Requires(%post) etc
  http://fedoraproject.org/wiki/Packaging/SysVInitScript#Initscript_packaging
  - For example
---------------------------------------------------------------
Requires(post): /sbin/chkconfig /sbin/install-info
Requires(post): chkconfig
---------------------------------------------------------------
    Here /sbin/chkconfig is added to Requires(post), which is provided
    by chkconfig rpm. So "Requires(post): chkconfig" is a duplicate
    (redundant) Requires.

* Some script writing issue
--------------------------------------------------------------
for dir in \
	$PRESENT_DIR/config/ $PRESENT_DIR/doc/ ......
--------------------------------------------------------------
  - The preceding "$PRESENTDIR/" are all unneeded.

* Timestamps
  - To keep timestamps on installed files, please use
--------------------------------------------------------------
make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p"
--------------------------------------------------------------
    This method usually works for recent autotool-based Makefiles.

   - When using "install" or "cp" commands, add "-p" option
     to keep timestamps.

* Macros
  - Please use macros consistently.
--------------------------------------------------------------
mkdir $RPM_BUILD_ROOT%{_sysconfdir}/rc.d/
mkdir $RPM_BUILD_ROOT%{_sysconfdir}/rc.d/init.d
install  $PRESENT_DIR/speech-dispatcherd $RPM_BUILD_ROOT%{_initrddir}/
--------------------------------------------------------------
    Use "mkdir -p $RPM_BUILD_ROOT%{_initrddir}"
  - And /usr/bin must be %{_bindir}

* Forbidden commands on scriptlets
  - Calling iconv on scriptlets is forbidden. Converting encodings
    must be done before %install ends.

* Preceding slash
  - Preceding slashs like
-------------------------------------------------------------
/%{python_sitelib}/speechd/*
^
-------------------------------------------------------------
    are all not needed.

* Some rpmlint issue
-------------------------------------------------------------
speech-dispatcher.i386: W: service-default-enabled
/etc/rc.d/init.d/speech-dispatcherd
-------------------------------------------------------------
  - Installed service must not be enabled by default.
    You should change the line
-------------------------------------------------------------
     6  # chkconfig: 2345 13 87
-------------------------------------------------------------
    to
-------------------------------------------------------------
# chkconfig: - 13 87
-------------------------------------------------------------

! By the way
  - I guess the speech-dispatcherd init script is completely broken.
    For example
-------------------------------------------------------------
    19  start() {
    20  [ -x $exec ]  || exit 5
    21  [ -f $config ]  || exit 6
    22  echo -n $"Starting $prog"
    23  retval=$?
    24  echo
    25  [ $retval -eq 0 ]  && touch $lockfile
    26  return $retval
    27  }
-------------------------------------------------------------
    .... What does this do? This just
    - check if speech-dispatcher can be executed and config file exists
    - then echo some message
    - touch lockfile
    - then return
    This actually does nothing...

(In reply to comment #28)
> > * rpmlint issue
> > -----------------------------------------------------------
> > speech-dispatcher-doc.i386: W: file-not-utf8
> > /usr/share/info/speech-dispatcher-cs.info.gz
> 
> Still to be resolved. Wondering how to go about this one (:-?)
  - See above. iconv must be used before %install ends.

> > speech-dispatcher-devel.i386: W: no-documentation
> > speech-dispatcher-python.i386: W: no-documentation
> 
> Is this warning very important, as most of the documentation has been included
> in a separate doc pacakge?

  - As in comment 24, I didn't mention this issue (i.e. not important)

> > * Binary name
> >   - IMO the names of the binaries
> > -----------------------------------------------------------
> > %_bindir/long_message
> > %_bindir/run_test
> > -----------------------------------------------------------
> 
> prefixed spd_ to each file. I hope that is fine?

  Okay.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.