[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 goyal.hemant at gmail.com  2008-02-15 15:27 EST -------
Thanks for the input :)

> * bconf
>   - Your usage of bconf conditional treatment is not right.
>     Please to the following link for example.
>     http://cvs.fedora.redhat.com/viewcvs/*checkout*/rpms/gimp/devel/gimp.spec

I tried to use this approach for conditional build but somehow was unsuccessful.
 If this approach is absolutely needed, I will spend more time and fix it. (At
this moment my attempt is commented out for your reference)

> * BuildRequires
>   - It seems dotconf is not in Fedora yet.
>     If you want to use dotconf, you have to submit another review request
>     for dotconf.

I am on it, thank you for letting me know about the issue.

> * Directory ownership issue
>   - Please make it sure that all directories which are created when installing
>     a rpm are owned by some package.
>     For example, the directory %{_sysconfdir}/peech-dispatcher itself is
>     not owned by any packages.

Am i supposed to set the Directory permissions to a particular value? If that is
the case I applied the same permissions as was done for GIMP.

> * libtool .la files
>   - must be removed.

Done. I was getting a build error, and for that reason i had to add the macro
%define _unpackaged_files_terminate_build 0.

> * EVR (Epoch-Version-Release) specific dependency
>   - The dependency between subpackages must be EVR (not only Version)
>     specific.

Added, I hope it has been done correctly.

> * /sbin/ldconfig
>   - (Usually, and actually for this package) calling 
>     /sbin/ldconfig is not needed for -devel package.

Commented out.

> * Static archive
>   - Packaging static archive is forbidden when providing shared
>     libraries, please remove them.
>     Also please check if configure accepts --disable-static option.

Removed the static libraries. The present script when run with --without
static_libs works fine, and disables static libs.

> * Info file
>   - Files under %_infodir are automatically marked as %doc.

I ve removed the %doc tag.

> * Changelog
>   - Please check
>     http://fedoraproject.org/wiki/Packaging/Guidelines#Changelogs
>   - Especially using %date macro in your way is forbidden.
>     In this way %date changes every time you rebuild this srpm,
>     which changes the old changelog entry

Fixed, I am now using the date format as mentioned on the website.

>   - One %changelog must be written in one spec file.
>     i.e. writting %changelog for every subpackage is not allowed.
>          These %changelog's must be unified.

Done.

> > (In reply to comment #5)
> > Also how will I distribute the init scripts since they are not part of the
> > original package? As a patch?
>   - You can add it as other sources like %SOURCE1.

Okay, thanks, I think it would be best to add this once you are happy with the
present state of the SPEC file.

> 
> ? symlink which seems modules
>   - BTW does this package work well if symlink .so under 
>     %_libdir/speech-dispatcher are not in main package?
>     These type of files are usually dlopen'ed and not aimed
>     for being used from other packages (i.e. not aimed for
>     being in -devel package).

I have moved .so files to main package now, there was an issue of dangling
pointers reported by rpmlint tool when the .so files were placed in the devel
package.

> %defattr
>   - We now recommend %defattr(-,root,root,-)

I ve applied 0755 as directory permissions, I am not absolutely clear what you
mean by the package owning the directory.

> * %post/%postun dependency for /sbin/install-info
>   - is missing for -doc-en, -doc-cs (please check
>     the section "Texinfo" of
>     http://fedoraproject.org/wiki/Packaging/ScriptletSnippets )

Are you referring to these?
Requires(post): /sbin/chkconfig /sbin/install-info

Requires(preun): /sbin/service /sbin/install-info



>   ? By the way do you really want to create -doc-en, -doc-cs
>     subpackages for only info files?

I ve merged them to a single documentation package at this point.
> 
> !
>   Please change release number of your spec every time you modify
>   your spec file to avoid confusion.

Sorry! I have started doing that now, and also maintaining a proper change log.

I suppose at this stage the issues that need to be resolved : 

1]Directory ownership
2]BCond
3]Dotconf packages
4]init scripts - will put the patch as advised by you
5]python packages refuse to get installed correctly. They get installed in a
"build" directory within BUILD and rpmbuild has no way to pick them and put them
in a python package.

Thanks as always for being so helpful :)

Hemant

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