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

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

 



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 goyal.hemant@xxxxxxxxx  2008-06-07 13:19 EST -------
Hi :-)

SPEC File for revision 8 -
http://www.nsitonline.in/hemant/stuff/speechd-rpm/speech-dispatcher.spec

SRPM -
http://www.nsitonline.in/hemant/stuff/speechd-rpm/speech-dispatcher-0.6.6-8.fc7.src.rpm


(In reply to comment #31)
> * First of all this does not build.
>   http://koji.fedoraproject.org/koji/taskinfo?taskID=650399
>   - Again please fix %_infodir/dir issue

Hmmm, I have included the file, but rpmlint reports an error on doc package

=================================================================
speech-dispatcher-doc.i386: E: info-dir-file /usr/share/info/dir
=================================================================

> --------------------------------------------------------------
>     File listed twice: /usr/lib/python2.5/site-packages/speechd/_test.py
> --------------------------------------------------------------

Resolved Now. Thank you for the solution.

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

I have added it as Source1 now.

> * Duplicate Requires(%post) etc

Resolved. I noticed it by the time you had already written the review. Sorry
about such a mistake.


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

Okay, somehow it was not working before. It seems to work without $PRESENTDIR/
now however :P.

> * Timestamps

I have added time stamping to all files EXCEPT the python files. I cannot
somehow find a way to add such an option to the command for installing the
python module.

Should I write a patch to modify the python script that installs the module??


> * 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}

Sorry again :-/. Fixed now.

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

Right, iconv was not even working back then, the UTF-8 encoding error stands
resolved now. Convert the speech-dispatcher-cs.info file to UTF-8 in the prep
section.
 
> * Preceding slash

Okay removed now.

> 
> * 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
> -------------------------------------------------------------

Hmmm, okay I had explicitly added that to start the service by default. Thank
you for the solution, have fixed it now.


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

Okay I have fixed it now, thank you for explaining what was actually happening
in the script. I have made the necessary modifications to make the script work.

One error however:
The script works perfectly when I start it, however after that I consistently
receive 

==========================================
speech-dispatcherd dead but subsys locked
==========================================

I must manually remove the lockfile to resolve this error. Cannot figure out
what is wrong here :-?

> > > speech-dispatcher-doc.i386: W: file-not-utf8
> > > /usr/share/info/speech-dispatcher-cs.info.gz

Fixed.

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

Cool :-)


@Kyle VanderBeek - Thanks for the tweaks :-). I have incorporated them!

Thanks,
Hemant
(prays there are no more errors :P)

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

_______________________________________________
Fedora-package-review mailing list
Fedora-package-review@xxxxxxxxxx
http://www.redhat.com/mailman/listinfo/fedora-package-review

[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]