[Fedora-i18n-bugs] [Bug 626666] Review Request: groonga - An Embeddable Fulltext Search Engine
bugzilla at redhat.com
bugzilla at redhat.com
Fri Oct 1 18:54:28 UTC 2010
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=626666
--- Comment #2 from Mamoru Tasaka <mtasaka at ioa.s.u-tokyo.ac.jp> 2010-10-01 14:54:28 EDT ---
Some notes (checked 1.0.2-2)
* BR
- Looking into bindings/ directory, it seems that some languages bindings
are available for this package (python, php, and also perhaps ruby?).
Would you try to enable these bindings?
* License
-------------------------------------------------------------------
doc/ja/README says LGPLv2
BSD
doc/ja/html/_static/
MIT or GPLv2
doc/ja/html/_static/jquery.js
resource/admin_html/css/ui-lightness/jquery-ui-1.8.1.custom.css
resource/admin_html/js/jquery-1.4.2.min.js
------------------------------------------------------------------
- So -libs license should be "LGPLv2 and (MIT or GPLv2)" and -doc license
should be "LGPLv2 and BSD".
* Timestamps
- Please consider to use
-------------------------------------------------------------------
make install DESTDIR=$RPM_BUILD_ROOT INSTALL~="install -p"
-------------------------------------------------------------------
to keep timestamps on installed files as much as possible.
This method usually works for Makefiles generated by recent autotool
scripts.
* Macros
- Please use macros for standard directories. /var should be %_var or
%_localstatedir, /usr/sbin should be %_sbin, and etc
https://fedoraproject.org/wiki/Packaging/RPMMacros
* SysV script related issues
- Please use %{_initddir} (/etc/rc.d/init.d) instead of %_sysconfdir/init.d
https://fedoraproject.org/wiki/Packaging/SysVInitScript#Initscripts_on_the_filesystem
- For registering groonga user or group, please follow
https://fedoraproject.org/wiki/Packaging/UsersAndGroups
! Note
The current way of yours is anyway confusing because doing
'echo "Unexpected error adding group \"groonga\". Aborting
installation."'
does not actually abort installation (because $ echo foo will perhaps
exit
with status 0).
-------------------------------------------------------------------
/usr/sbin/useradd -r -s /sbin/nologin -c 'groonga' \
-d %{_localstatedir}/lib/groonga --create-home \
-------------------------------------------------------------------
- $ env LANG=C man useradd says "--create-home" will copy files in skeleton
directory into the created home directory. Is it needed?
- And anyway %_localstatedir/lib/groonga must also be listed in %files
entry, so I don't think --create-home option is needed here.
- The lines
-------------------------------------------------------------------
%post
/bin/mkdir -p /var/run/groonga
/bin/chown -R groonga:groonga /var/run/groonga
-------------------------------------------------------------------
is wrong. In this case, the directory %_localstatedir/run/%name must be
registered in %files entry with proper %attr added.
-------------------------------------------------------------------
%post munin-plugins
/usr/sbin/munin-node-configure --shell --remove-also | grep -e 'groonga_' | sh
[ -f /var/lock/subsys/munin-node ] && \
/sbin/service munin-node restart > /dev/null 2>&1
-------------------------------------------------------------------
- Some Requires(post) is required here (Requires(post): munin-node)
- Doesn't munin-node service support condrestart?
- We don't usually call userdel / groupdel on scriptlets:
https://fedoraproject.org/wiki/Packaging/UsersAndGroups
- Would you explain why this is needed?
--------------------------------------------------------------------
%postun munin-plugins
if [ $1 -eq 0 ]; then
rm %{_sysconfdir}/munin/plugins/groongar_* > /dev/null 2>&1
--------------------------------------------------------------------
- By the way calling /sbin/service or /sbin/chkconfig or getent or
so requires writing Proper "Requires(foo): bar"s. Please
refer to:
https://fedoraproject.org/wiki/Packaging/UsersAndGroups
https://fedoraproject.org/wiki/Packaging/SysVInitScript#Initscripts_in_spec_file_scriptlets
* %files
- We now prefer to use %defattr(-,root,root,-)
- Files under %_mandir (note: please use %_mandir instead of %_datadir/man)
are
automatically marked as %doc, so there is no need to write explicit %doc
attribute
for %_mandir/foo.
- Please take care of directory ownership issue.
https://fedoraproject.org/wiki/Packaging/Guidelines#File_and_Directory_Ownership
https://fedoraproject.org/wiki/Packaging/UnownedDirectories
! The point is that when installing a (binary) rpm creates some new
directories,
the created directories should also be owned by the (just installed) rpm.
For example, when installing groonga-libs rpm, installing
%_libdir/groonga/modules/suggest/suggest.so (on i686)
newly creates the following directories:
--------------------------------------------------------------
%_libdir/%name
%_libdir/%name/modules
%_libdir/%name/modules/suggest
--------------------------------------------------------------
so -libs subpackage should also own these directories. Also please refer
to
https://fedoraproject.org/wiki/Packaging/UnownedDirectories#Common_Mistakes
! Instead if the needed directories for a rpm are already created by the
packages
which the rpm depends on, the rpm (to be newly installed) should not own
the
directory.
For example the directory %_sysconfdir/munin/plugin-conf.d is already
owned by
munin-node, so -munin-plugins subpackage should not own this directory
itself.
--
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.
More information about the i18n-bugs
mailing list