[Bug 497441] Review Request: mumble - Voice chat application

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


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





--- Comment #60 from Igor Jurišković <juriskovic.igor at gmail.com>  2009-05-16 09:21:53 EDT ---
Tnx for reply Mamoru.

(In reply to comment #59)
> Well, I have not fully read previous comments, however
> for -9:
> 
> * BR (BuildRequires)
>   - Would you explain why non-devel packages like:
> -------------------------------------------------------------------
> dbus-qt
> qt-sqlite
> -------------------------------------------------------------------
>     have to be written as BRs explicitly?
> 
There is a problem. qt-sqlite is not available on F10. Because of that(i think
cause of that) mumble fails to build at koji. What is the replacement for
qt-sqlite on F10? I was searching but found nothing.

> * Requires
>   - Proper Requires(pre) or so are missing:
>     https://fedoraproject.org/wiki/Packaging/UsersAndGroups
>
Done.

> https://fedoraproject.org/wiki/Packaging/SysVInitScript#InitscriptScriptlets
>   - Would you explain why murmur subpackage does not depend
>     on mumble main package?
You dont need mumble to run murmur. Murmur is for servers and mumble is for
clients.

>   - The directory %{_datadir}/kde4/services/ is owned by kde-filesystem
>     so it is better that -protocol subpackage should have
>     "Requires: kde-filesystem".
> 
Done.

> * Parallel make
>   - Support parallel make if possible. If not possible
>     write some comments about this:
>     https://fedoraproject.org/wiki/Packaging/Guidelines#Parallel_make
> 
Done.

> * Installation directory
> ------------------------------------------------------------------
> install -pD -m0755 release/murmurd %{buildroot}%{_sbindir}/murmurd
> ln -s ../sbin/murmurd %{buildroot}%{_bindir}/%{name}-server
> ------------------------------------------------------------------
>   - Why is the same command has to be in two different directories
>     in the path with different names?
Less confusion. After years of mumble nobody told me murmur - everyone says
mumble-server.

> 
>   - By the way
> ------------------------------------------------------------------
> %attr(-,mumble-server,mumble-server) %{_bindir}/mumble-server
> ------------------------------------------------------------------
>     Here %{_bindir}/mumble-server is a symlink and
>     %attr(.....) on symlink is useless (see man symlink(2))
>     ! By the way using %{name} macro is preferred because
>       you already use it.
> 
Fixed.

> ------------------------------------------------------------------
> #man pages
> mkdir -p %{buildroot}%{_mandir}/
> install -pD -m0664 man/murmurd.1 %{buildroot}%{_mandir}/
> install -pD -m0664 man/mumble* %{buildroot}%{_mandir}/
> ------------------------------------------------------------------
>   - These man pages must be installed under %_mandir/man1,
>     not under %_mandir (and also see below)
> 
Done.

> * Permissions
> ------------------------------------------------------------------
> install -pD -m0664 icons/%{name}.16x16.png
> %{buildroot}%{_datadir}/icons/hicolor/16x16/apps/%{name}.png
> ------------------------------------------------------------------
>   - Usually these files should have 0644 (not 0664) permission
> 
Done.

> ------------------------------------------------------------------
> install -pD scripts/%{name}.protocol
> %{buildroot}%{_datadir}/kde4/services/%{name}.protocol
> install -pD scripts/murmur.conf
> %{buildroot}%{_sysconfdir}/dbus-1/system.d/murmur.conf
> ------------------------------------------------------------------
>   - Due to this these files have executable permission however it
>     seems these should be with 0644 permision.
> 
Done.

> * Inconsistent commands
> ------------------------------------------------------------------
> install -d %{buildroot}%{_libdir}/%{name}/
> mkdir -p %{buildroot}%{_mandir}/
> ------------------------------------------------------------------
>   - Why do you use both "install -d" and "mkdir -p"?
> 
Fixed.

> * Scriptlets
>   - Scriptlets for GTK cache updating does not match current Fedora
>     guideline. Please fix this:
>     https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#Icon_Cache
>     (especially check %postun)
> 
Done.

>   - Please specify the home directory of mumble-server user
>     (see the -d option:
>      https://fedoraproject.org/wiki/Packaging/UsersAndGroups )
>     Currently this is set as "/home/mumble-server" automatically
>     (on Fedora)
> 
What should set as home directory? It says directory with data but there is
none.

> ------------------------------------------------------------------
> %preun -n murmur
> if [$1 = 0]; then
> ------------------------------------------------------------------
>   - This is wrong.
> ------------------------------------------------------------------
> [tasaka1 at localhost ~]$ num=0
> [tasaka1 at localhost ~]$ if [$num = 0] ; then echo "yes" ; fi
> -bash: [0: command not found
> ------------------------------------------------------------------
>     "if [ $1 = 0 ] ; then" is correct
>     ! Note: here "[" is a built-in "command".
> 
Fixed.

> ------------------------------------------------------------------
> %post -n murmur
> /sbin/chkconfig --add murmur --level a|| :
> ------------------------------------------------------------------
>   - "--level a" is not needed:
> 
> https://fedoraproject.org/wiki/Packaging/SysVInitScript#InitscriptScriptlets
Fixed.

> 
> * Duplicate %files entry
> ------------------------------------------------------------------
> -%files plugins
> %defattr(-,root,root,-)
> %{_libdir}/%{name}
> %{_libdir}/%{name}/*.so
> -----------------------------------------------------------------
>   - The %files entry "%{_libdir}/%{name}" means the directory %_libdir/%name
>     itself and all files/directories/etc under %_libdir/%name.
>     build.log shows:
> -----------------------------------------------------------------
>    857  Processing files: mumble-plugins-1.1.8-9.fc11.i586
>    858  warning: File listed twice: /usr/lib/mumble/liblink.so
> -----------------------------------------------------------------
Fixed.

> 
> * initscript behavior
> -----------------------------------------------------------------
> [root at localhost ~]# service murmur start
> Starting murmur:                                           [  OK  ]
> [root at localhost ~]# service murmur stop
> Shutting down murmur:                                      [  OK  ]
> [root at localhost ~]#                                        [  OK  ]
> -----------------------------------------------------------------
>   - killproc() (in %_initrcdir/functions) internally calls
>     success() or failure so there are duplicate messages when
>     shutting down murmur daemon.  
Fixed.


As stated above building at koji fails after fixing above problems. Its got to
be something related to dependency or sqlite but cant find it. btw on my PC
everything builds fine - works as expected.

SPEC: http://78.46.84.75/fedora/mumble.spec
SRPM: http://78.46.84.75/fedora/mumble-1.1.8-10.fc10.src.rpm

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