Spec URL: http://fedpop.github.io/fedora/mumble/mumble.spec SRPM URL: http://fedpop.github.io/fedora/mumble/mumble-1.2.10-1.fc23.src.rpm Description: Hi I am attempting to become a maintainer of mumble and unretire the package. I am not a current maintainer and am looking for a sponsor. Mumble is a voip client server package. It has been out of the repos for about a year since it became abandoned. In this build I disabled ice rpc support in order to make things easier on myself and speed up this long overdue process. The majority of Fedora users will not need ice. If this is successful I plan to add unretire ice next. I've had many users on my copr of this rpm for about 2 months now with positive feedback. I would like to get it into the official repo. The dead review request is here: https://bugzilla.redhat.com/show_bug.cgi?id=1181366 Thanks Fedora Account System Username: fedpop
Hi, I can help out here, I'm glad you submitted this, I too have seen and heard many users asking for mumble.
*** Bug 1181366 has been marked as a duplicate of this bug. ***
Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=11969289 Some initial comments: Naming: ok License: ok Scriptlets: ok Sources: mostly ok (see item 3 below) 228fff5975a61872b83052be97e2eabf 1.2.10.tar.gz 1. SHOULD use %{qmake_qt4} macro in favor of %{_qt4_qmake}, the former sets flags automatically for you, so you can avoid manually setting stuff like: QMAKE_CFLAGS_RELEASE="%{optflags}" QMAKE_CXXFLAGS_RELEASE="%{optflags}" 2. MUST document why you're not using %{?_smp_mflags} 3. SHOULD follow https://fedoraproject.org/wiki/Packaging:SourceURL?rd=Packaging/SourceURL#Git_Tags for better-named source tarballs, in particular, please consider using: Source0: https://github.com/mumble-voip/mumble/archive/%{version}.tar.gz#/%{name}-%{version}.tar.gz 4. SHOULD use unambiguous build deps, in particular use: BuildRequires: qt4-devel or BuildRequires: pkgconfig(QtGui) in favor of BuildRequires: qt-devel 5. SHOULD drop some deprecated rpm tags, including: Group: 6. SHOULD consider dropping -protocol subpkg, it's there only for one file, and arguably for something a bit deprecated (no recent fedora release ships a kde4 desktop, nor is a single .protocol file by itself particularly useful) 7. MUST document the need for these explicit runtime dependencies: Requires: celt071, speex, opus (at least some of these should already be handled via implicit rpm library autodepends)
Why did you add a depends on bug #1181365 ? the package as-is currently does not need it. I mean it *can* use it, but it's not currently a dependency, so I would argue it is not strictly a review-blocker.
Ok thanks Rex Dieter, I addressed all of your suggestions I hope. 1. done 2. added. old versions did not support parallel make. 3. done 4. done 5. done 6. deleted 7. Made a note about celt071, deleted others.
8. Remember to always bump Release: tag, and add appropriate %changelog entries, even for review modifications 9. since you addressed removing the -protocol subpkg, may be worth adding an upgrade path for folks you may have that -subpkg installed: Obsoletes: mumble-protocol < 1.2.10-2 10. Since mumble is being built without ice support at the moment, you can drop this from the .spec: # Due to missing ice on ppc64 ExcludeArch: ppc64 11. SHOULD use %license tag for license files, ie replace instances of: %doc LICENSE with %license LICENSE 12. SHOULD address rpmlint interesting warning: mumble.x86_64: W: crypto-policy-non-compliance-openssl /usr/bin/mumble SSL_CTX_set_cipher_list per looking at the non-trivial code in src/SSL.cpp and recommendations on: https://fedoraproject.org/wiki/Packaging:CryptoPolicies It would appear that patching src/SSL.cpp would do it, something like changing: QString MumbleSSL::defaultOpenSSLCipherString() { return QLatin1String("EECDH+AESGCM:AES256-SHA:AES128-SHA"); } to QString MumbleSSL::defaultOpenSSLCipherString() { return QLatin1String("PROFILE=SYSTEM"); } Though I'm definitely not sure about this, may be worth consulting with your upstream on this one, to see if they think this is appropriate (or not).
The wiki about Crypto, also recommended sending queries to: https://lists.fedoraproject.org/mailman/listinfo/security
IMHO #11 from comment #6 is MUST as fedora-review tool enforces %license. https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text
Could the celt codec be removed as well since it was merged into Opus and is no longer being updated? If so we'd need to make a change in the default server config to make opusthreshold=0 a default. And a note that clients with version <= 1.2.3 will not be able to chat.
about the openssl change: I did a diff of the ciphers and found all to be included on F23 for the default security level. see https://gist.github.com/fedpop/9c890cdd5a17332354fc I am feeling ok about this change. I will message the security mailing list and see what they think. About removing Celt: Celt is kind of a pain point because mumble forked celt because it was unstable format and in development at the time they integrated it. Celt 0.7.1 is the fallback for all Mumble clients so if people are still running old stuff then it will be bad. I would prefer to use the mumble celt so no problems can occur but due to bundling rules it's currently using the system celt071. I'm not sure the statistics on the ecosystem. I would be ok with Opus only if we could figure that out.
Spec URL: http://fedpop.github.io/fedora/mumble/mumble.spec SRPM URL: http://fedpop.github.io/fedora/mumble/mumble-1.2.10-3.fc23.src.rpm #8 ok sorry wasn't sure when it was appropriate, I received a good answer on IRC. #9 I'm not entirely sure how this works. I put Obsoletes: mumble-protocol < 1.2.10-2 in the main package but I get a rpmlint warning. mumble.x86_64: W: obsolete-not-provided mumble-protocol #10 added the license tag #11 Discovered that the ciphers can be changed in the config. modified the murmur.ini sslCiphers= config and added it to the sources.
I think the config murmur.ini file change should probably be a patch actually. I'll fix that up.
Spec URL: http://fedpop.github.io/fedora/mumble/mumble.spec SRPM URL: http://fedpop.github.io/fedora/mumble/mumble-1.2.10-4.fc23.src.rpm Hardened murmur.service and changed murmur.ini cipher setting to be applied as Patch3
Thanks! Looks good to me, approved (and sponsored). Welcome to fedora.
fyi, adjusted summary of review here to match Summary: tag in mumble.spec
mumble-1.2.10-4.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2015-934a0702cf
mumble-1.2.10-4.fc22 has been submitted as an update to Fedora 22. https://bodhi.fedoraproject.org/updates/FEDORA-2015-789c21d8a6
mumble-1.2.10-4.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with $ su -c 'dnf --enablerepo=updates-testing update mumble' You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-934a0702cf
mumble-1.2.10-4.fc22 has been pushed to the Fedora 22 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with $ su -c 'dnf --enablerepo=updates-testing update mumble' You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-789c21d8a6
mumble-1.2.10-4.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report.
mumble-1.2.11-1.fc22 has been submitted as an update to Fedora 22. https://bodhi.fedoraproject.org/updates/FEDORA-2015-764292844d