Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1284989 (mumble)
Summary: | Review Request: mumble - Voice chat suite aimed at gamers | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | John <johnhatestrash> |
Component: | Package Review | Assignee: | Rex Dieter <rdieter> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | unspecified | ||
Version: | rawhide | CC: | audrey, clintonminton, codonell, eliadevito, package-review, projects.rg, rdieter, sergio |
Target Milestone: | --- | Flags: | rdieter:
fedora-review+
|
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2015-12-03 20:20:36 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | 1181365 | ||
Bug Blocks: | 928937 |
Description
John
2015-11-24 15:51:08 UTC
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 |