Bug 527059
Summary: | Review Request: earcandy - Sound level manager | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Lubomir Rintel <lkundrak> |
Component: | Package Review | Assignee: | Hans de Goede <hdegoede> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | low | ||
Version: | rawhide | CC: | fedora-package-review, hdegoede, martin.gieseking, notting, pikachu.2014 |
Target Milestone: | --- | Flags: | hdegoede:
fedora-review+
kevin: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2009-12-29 08:55:42 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: |
Description
Lubomir Rintel
2009-10-03 19:41:45 UTC
rpmlint complains about various files in python_sitelib having a shebang despite not being executable. Those files are really not meant to be executable and patching away the shebang is not worth since it does not affect functionality at all. Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1726062 A couple of comments: - the file pulseaudio/lib.py is licensed under BSD - you should replace the shebang of file ear_candy according to https://fedoraproject.org/wiki/Script_Interpreters_%28draft%29 $ rpmlint /var/lib/mock/fedora-11-x86_64/result/earcandy-0.5.1-1.fc11.* earcandy.noarch: E: explicit-lib-dependency pulseaudio-libs earcandy.noarch: E: non-executable-script /usr/lib/python2.6/site-packages/ear_candy/window/WindowWatcher.py 0644 /usr/bin/env earcandy.noarch: E: non-executable-script /usr/lib/python2.6/site-packages/ear_candy/Freedesktop.py 0644 /usr/bin/env earcandy.noarch: E: non-executable-script /usr/lib/python2.6/site-packages/ear_candy/ear_candy.py 0644 /usr/bin/env earcandy.noarch: E: non-executable-script /usr/lib/python2.6/site-packages/ear_candy/TrayIcon.py 0644 /usr/bin/env earcandy.noarch: E: non-executable-script /usr/lib/python2.6/site-packages/ear_candy/__init__.py 0644 /usr/bin/env 2 packages and 0 specfiles checked; 6 errors, 0 warnings. Even if the shebangs don't hurt, I would remove them anyway, e.g. with for f in `find $RPM_BUILD_ROOT%{python_sitelib}/ear_candy/ -name '*.py'`; do sed '/^#!\/usr\/bin\/env python/d' $f >$f.new touch -r $f $f.new mv $f.new $f done After doing so, there's still a remaining error. It can be ignored because package pulseaudio-libs is not picked up automatically. $ rpmlint ../RPMS/noarch/earcandy-0.5.1-1.fc11.noarch.rpm earcandy.noarch: E: explicit-lib-dependency pulseaudio-libs 1 packages and 0 specfiles checked; 1 errors, 0 warnings. (In reply to comment #2) > A couple of comments: Thanks for them! > - the file pulseaudio/lib.py is licensed under BSD Fixed. > - you should replace the shebang of file ear_candy according to > https://fedoraproject.org/wiki/Script_Interpreters_%28draft%29 Fixed. > Even if the shebangs don't hurt, I would remove them anyway, e.g. with Done > After doing so, there's still a remaining error. It can be ignored because > package pulseaudio-libs is not picked up automatically. I do not think so; even if we dlopen libpulse, we still do assumptions about ABI, therefore I changed this to depend on specific ABI version and the rpmlint warning went away. SPEC: http://v3.sk/~lkundrak/SPECS/earcandy.spec SRPM: http://v3.sk/~lkundrak/SRPMS/earcandy-0.5.1-2.fc12.src.rpm (In reply to comment #4) > I do not think so; even if we dlopen libpulse, we still do assumptions about > ABI, therefore I changed this to depend on specific ABI version and the rpmlint > warning went away. Yes, I think you're right. Could you please upload the SRPM again, e.g. to fedorapoeple.org? The above link doesn't work (404). SPEC: http://v3.sk/~lkundrak/SPECS/earcandy.spec SRPM: http://v3.sk/~lkundrak/SRPMS/earcandy-0.5.1-2.fc11.src.rpm Sorry, I typo-ed the disttag. OK, thanks. Two further things I noticed that should be addressed: - you can drop Requires: python because it's added automatically - the desktop file is added twice: $ rpmls earcandy-0.5.1-2.fc11.noarch.rpm | fgrep .desktop -rw-r--r-- /usr/share/applications/earcandy.desktop -rw-r--r-- /usr/share/earcandy/earcandy.desktop Thanks you for the comments, the fixes addressing both issues have been applied. New package: SPEC: http://v3.sk/~lkundrak/SPECS/earcandy.spec SRPM: http://v3.sk/~lkundrak/SRPMS/earcandy-0.5.1-3.fc12.src.rpm Reviewing this one, Martin thanks for all the useful comments so far. Full review done (tarbal matches upstream, license check, spec file readability, rpmlint, etc.). I've only one remark, which is a blocker though. MUST FIX: --------- * Requires: libpulse.so.0(PULSE_0) Is wrong for 64 bit systems, which have: libpulse.so.0(PULSE_0)(64bit) The only way I see to fix this, while allowing the package to stay noarch is using: Requires: pulseaudio-libs Hans: changed (actually requiring < 1, quetly and probably wrongly assuming that soname's number corresponds to the major version number) SPEC: http://v3.sk/~lkundrak/SPECS/earcandy.spec SRPM: http://v3.sk/~lkundrak/SRPMS/earcandy-0.5.1-4.fc12.src.rpm (In reply to comment #10) > Hans: changed (actually requiring < 1, quetly and probably wrongly assuming > that soname's number corresponds to the major version number) > Yes that assumption is most likely wrong, I would advise you to remove the < 1 in the requires before building, other then that it now looks ok: Approved! (In reply to comment #11) > (In reply to comment #10) > > Hans: changed (actually requiring < 1, quetly and probably wrongly assuming > > that soname's number corresponds to the major version number) > > > > Yes that assumption is most likely wrong, I would advise you to remove the > < 1 in the requires before building Ok, will do. New Package CVS Request ======================= Package Name: earcandy Short Description: Sound level manager Owners: lkundrak Branches: F-11 F-12 EL-5 cvs done. Imported and built. |