[Bug 165] Review Request: cmus - Ncurses-Based Music Player
http://bugzilla.rpmfusion.org/show_bug.cgi?id=165 Conrad Meyer kon...@tylerc.org changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution||FIXED --- Comment #22 from Conrad Meyer kon...@tylerc.org 2009-02-16 07:44:32 --- Built in devel, closing. -- Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug.
[Bug 165] Review Request: cmus - Ncurses-Based Music Player
http://bugzilla.rpmfusion.org/show_bug.cgi?id=165
--- Comment #10 from Kevin Kofler kevin.kof...@chello.at 2008-12-17 09:44:00
---
I'd even say abuse of %{name} can be outright harmful, for example using it in
the URL tag means you can't just paste the URL into a browser. And it usually
makes no sense at all, how often is a package's name going to change? And if it
is, usually it's for a compat package or something similar, where in most
places you still want to use the original name, not the modified one.
--
Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
You are the assignee for the bug.
[Bug 165] Review Request: cmus - Ncurses-Based Music Player
http://bugzilla.rpmfusion.org/show_bug.cgi?id=165 --- Comment #12 from Conrad Meyer kon...@tylerc.org 2008-12-17 11:23:14 --- http://konradm.fedorapeople.org/fedora/SPECS/cmus.spec http://konradm.fedorapeople.org/fedora/SRPMS/cmus-2.2.0-3.fc9.src.rpm Sorry it took so long. -- Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. You are the assignee for the bug.
[Bug 165] Review Request: cmus - Ncurses-Based Music Player
http://bugzilla.rpmfusion.org/show_bug.cgi?id=165 Andrea Musuruane musur...@gmail.com changed: What|Removed |Added CC||musur...@gmail.com Blocks|2 |3 Status|NEW |ASSIGNED -- Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. You are the assignee for the bug.
[Bug 165] Review Request: cmus - Ncurses-Based Music Player
http://bugzilla.rpmfusion.org/show_bug.cgi?id=165 --- Comment #14 from Conrad Meyer kon...@tylerc.org 2008-12-17 11:44:05 --- Actually I didn't have to patch the makefile at all. Just passed V=2. -- Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. You are the assignee for the bug.
[Bug 165] Review Request: cmus - Ncurses-Based Music Player
http://bugzilla.rpmfusion.org/show_bug.cgi?id=165
--- Comment #11 from Andrea Musuruane musur...@gmail.com 2008-12-17 11:12:04
---
(In reply to comment #9)
Using %{name} and %{version} macros is not mandated. I use %{version} macros,
but prefer not to use %{name} especially when %{name} is longer than cmus
(what it evaluates to).
You are right. I should have used the world should and I agree that it is a
matter of style.
I will fix the quiet makefile issue and fix what you
described w.r.t. the Patch0, but I don't think %{name} is needed.
OK. Waiting for the fixes.
--
Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
You are the assignee for the bug.
[Bug 165] Review Request: cmus - Ncurses-Based Music Player
http://bugzilla.rpmfusion.org/show_bug.cgi?id=165
Andrea Musuruane musur...@gmail.com changed:
What|Removed |Added
Blocks|3 |4
--- Comment #15 from Andrea Musuruane musur...@gmail.com 2008-12-17 16:18:22
---
Here is the review:
+:ok, =:needs attention, -:needs fixing, N:not applicable.
MUST Items:
[+] MUST: rpmlint must be run on every package.
Output is clean
[+] MUST: The package must be named according to the Package Naming Guidelines.
[+] MUST: The spec file name must match the base package %{name}
[+] MUST: The package must meet the Packaging Guidelines.
[=] MUST: The package must be licensed with a Fedora approved license and meet
the Licensing Guidelines.
No problem because this is RPM Fusion.
[+] MUST: The License field in the package spec file must match the actual
license.
[+] MUST: If (and only if) the source package includes the text of the
license(s) in its own file, then that file, containing the text of the
license(s) for the package must be included in %doc.
[+] MUST: The spec file must be written in American English.
[+] MUST: The spec file for the package MUST be legible.
[+] MUST: The sources used to build the package must match the upstream source,
as provided in the spec URL.
7a9895ecfc10cd16577c73051436962f cmus-2.2.0.tar.bz2
[+] MUST: The package must successfully compile and build into binary rpms on
at least one supported architecture.
Tested on F9/i386
[+] MUST: If the package does not successfully compile, build or work on an
architecture, then those architectures should be listed in the spec in
ExcludeArch.
[+] MUST: All build dependencies must be listed in BuildRequires
[N] MUST: The spec file MUST handle locales properly. This is done by using the
%find_lang macro.
[N] MUST: Every binary RPM package which stores shared library files (not just
symlinks) in any of the dynamic linker's default paths, must call ldconfig in
%post and %postun.
[N] MUST: If the package is designed to be relocatable, the packager must state
this fact in the request for review
[+] MUST: A package must own all directories that it creates. If it does not
create a directory that it uses, then it should require a package which does
create that directory.
[+] MUST: A package must not contain any duplicate files in the %files listing.
[+] MUST: Permissions on files must be set properly. Executables should be set
with executable permissions, for example. Every %files section must include a
%defattr(...) line.
[+] MUST: Each package must have a %clean section, which contains rm -rf
%{buildroot} (or $RPM_BUILD_ROOT).
[+] MUST: Each package must consistently use macros, as described in the macros
section of Packaging Guidelines.
[=] MUST: The package must contain code, or permissible content. This is
described in detail in the code vs. content section of Packaging Guidelines.
This is not a problem since we are in RPM Fusion.
[N] MUST: Large documentation files should go in a doc subpackage.
[+] MUST: If a package includes something as %doc, it must not affect the
runtime of the application.
[N] MUST: Header files must be in a -devel package.
[N] MUST: Static libraries must be in a -static package.
[N] MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'
(for directory ownership and usability).
[N] MUST: If a package contains library files with a suffix (e.g.
libfoo.so.1.1), then library files that end in .so (without suffix) must go in
a -devel package.
[N] MUST: In the vast majority of cases, devel packages must require the base
package using a fully versioned dependency: Requires: %{name} =
%{version}-%{release}
[+] MUST: Packages must NOT contain any .la libtool archives, these should be
removed in the spec.
[N] MUST: Packages containing GUI applications must include a %{name}.desktop
file, and that file must be properly installed with desktop-file-install in the
%install section.
[+] MUST: Packages must not own files or directories already owned by other
packages.
[+] MUST: At the beginning of %install, each package MUST run rm -rf
%{buildroot} (or $RPM_BUILD_ROOT).
[+] MUST: All filenames in rpm packages must be valid UTF-8.
SHOULD Items:
[N] SHOULD: If the source package does not include license text(s) as a
separate file from upstream, the packager SHOULD query upstream to include it.
[N] SHOULD: The description and summary sections in the package spec file
should contain translations for supported Non-English languages, if available.
[N] SHOULD: The reviewer should test that the package builds in mock.
[N] SHOULD: The package should compile and build into binary rpms on all
supported architectures.
[+] SHOULD: The reviewer should test that the package functions as described.
[+] SHOULD: If scriptlets are used, those scriptlets must be sane.
[N] SHOULD: Usually, subpackages other than devel should require the
[Bug 165] Review Request: cmus - Ncurses-Based Music Player
http://bugzilla.rpmfusion.org/show_bug.cgi?id=165 Andrea Musuruane musur...@gmail.com changed: What|Removed |Added AssignedTo|rpmfusion-package- |musur...@gmail.com |rev...@rpmfusion.org| Status|ASSIGNED|NEW -- Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. You are the assignee for the bug.
[Bug 165] Review Request: cmus - Ncurses-Based Music Player
http://bugzilla.rpmfusion.org/show_bug.cgi?id=165 --- Comment #16 from Andrea Musuruane musur...@gmail.com 2008-12-17 16:34:28 --- Can you return the favour and do a review? I have #476357 in Fedora ;) -- Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug.
[Bug 165] Review Request: cmus - Ncurses-Based Music Player
http://bugzilla.rpmfusion.org/show_bug.cgi?id=165 Andrea Musuruane musur...@gmail.com changed: What|Removed |Added Status|NEW |ASSIGNED -- Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug.
[Bug 165] Review Request: cmus - Ncurses-Based Music Player
http://bugzilla.rpmfusion.org/show_bug.cgi?id=165
--- Comment #13 from Andrea Musuruane musur...@gmail.com 2008-12-17 11:35:00
---
(In reply to comment #8)
* There is no compile output. Makefile must be patched to see it. This is a
blocking issue. A reviewer must see the output of what is happening when
building (for example, to see the CFLAGS).
I don't see how this is a blocker nor why it warrants patching the makefile.
Probably we have some kind of misunderstanding Kevin. The makefile suppress the
output when calling many commands including the compiler. In this way neither
the submitter nor the reviewer can see how the files are complied, linked and
so on.
Using %{optflags} is mandatory and with the unpatched upstream makefile there
is no way to check them:
https://fedoraproject.org/wiki/Packaging/Guidelines#Compiler_flags
--
Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
You are the assignee for the bug.
[Bug 165] Review Request: cmus - Ncurses-Based Music Player
http://bugzilla.rpmfusion.org/show_bug.cgi?id=165 --- Comment #17 from Kevin Kofler kevin.kof...@chello.at 2008-12-17 23:29:39 --- Probably we have some kind of misunderstanding Kevin. The makefile suppress the output when calling many commands including the compiler. In this way neither the submitter nor the reviewer can see how the files are complied, linked and so on. I understood this pretty well, still I think looking at what CFLAGS are being passed in the specfile is usually sufficient. (Yes, there can be typos like CFALGS, but normally you notice them when reading the specfile. :-) ) But anyway, it turns out getting the output was trivial: Actually I didn't have to patch the makefile at all. Just passed V=2. so it's all set now. -- Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug.
[Bug 165] Review Request: cmus - Ncurses-Based Music Player
http://bugzilla.rpmfusion.org/show_bug.cgi?id=165 --- Comment #5 from Andrea Musuruane musur...@gmail.com 2008-12-16 12:12:35 --- (In reply to comment #4) Ah, that's a good idea :). I tried the above and got this in %doc, though: error: magic_file(ms, /var/tmp/cmus-2.2.0-2.fc9-root-konrad/usr/share/doc/cmus-2.2.0/examples) failed: mode 040755 cannot open `/var/tmp/cmus-2.2.0-2.fc9-root-konrad/usr/share/doc/cmus-2.2.0/examples' (No such file or directory) rpmbuild: rpmfc.c:1386: rpmfcClassify: Assertion `ftype != ((void *)0)' failed. Aborted There is a way, but now I'm not so sure that the way I advised you is better than yours. https://www.redhat.com/archives/rpm-list/2001-November/msg00204.html * Why don't you use CONFIG_FFMPEG=y to have .wma support? Because as of ffmpeg 0.4.9-0.48.20080908.fc9 this feature doesn't compile. There are patches for these already on the net. For example, this one from Gentoo works: http://ftp.riken.go.jp/Linux/gentoo/media-sound/cmus/files/cmus-2.2.0-new-ffmpeg.patch * Why do you use both libmodplug and libmikmod since both are used to have .mod and .x3m support? I'm not familiar with .mod/.x3m, can you help me here? Mod and x3m are music file formats that originated on the Amiga. Both libraries provides the same functionalities. Therefore you only need to require one of the two. Configure is made to default use libmodplug, so I suggest you not to use libmikmod. -- Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. You are the assignee for the bug.
[Bug 165] Review Request: cmus - Ncurses-Based Music Player
http://bugzilla.rpmfusion.org/show_bug.cgi?id=165
--- Comment #7 from Andrea Musuruane musur...@gmail.com 2008-12-16 14:51:23
---
(In reply to comment #6)
OK, I have incorporated your suggestions into this set of spec/srpm:
http://konradm.fedorapeople.org/fedora/SPECS/cmus.spec
http://konradm.fedorapeople.org/fedora/SRPMS/cmus-2.2.0-2.fc9.src.rpm
You still have to resolve this problem:
* There is no compile output. Makefile must be patched to see it. This is a
blocking issue. A reviewer must see the output of what is happening when
building (for example, to see the CFLAGS).
Then:
* use %{name} and %{version} macros here:
Source0:http://mirror.greaterscope.net/cmus/cmus-%{version}.tar.bz2
Patch0:
http://ftp.riken.go.jp/Linux/gentoo/media-sound/cmus/files/cmus-2.2.0-new-ffmpeg.patch
Don't use the %{version} macro for the patch though. So you can reuse the same
patch in next releases, if it still applies.
* Do not use full URL for Patch0. This is just a mirror and it is not
guaranteed to work in the future. Just place a note that the patch comes from
Gentoo Bugzilla #218105.
--
Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
You are the assignee for the bug.
[Bug 165] Review Request: cmus - Ncurses-Based Music Player
http://bugzilla.rpmfusion.org/show_bug.cgi?id=165 --- Comment #6 from Conrad Meyer kon...@tylerc.org 2008-12-16 12:41:31 --- OK, I have incorporated your suggestions into this set of spec/srpm: http://konradm.fedorapeople.org/fedora/SPECS/cmus.spec http://konradm.fedorapeople.org/fedora/SRPMS/cmus-2.2.0-2.fc9.src.rpm -- Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. You are the assignee for the bug.
[Bug 165] Review Request: cmus - Ncurses-Based Music Player
http://bugzilla.rpmfusion.org/show_bug.cgi?id=165 --- Comment #8 from Kevin Kofler kevin.kof...@chello.at 2008-12-17 02:38:28 --- * There is no compile output. Makefile must be patched to see it. This is a blocking issue. A reviewer must see the output of what is happening when building (for example, to see the CFLAGS). I don't see how this is a blocker nor why it warrants patching the makefile. -- Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. You are the assignee for the bug.
[Bug 165] Review Request: cmus - Ncurses-Based Music Player
http://bugzilla.rpmfusion.org/show_bug.cgi?id=165
--- Comment #9 from Conrad Meyer kon...@tylerc.org 2008-12-17 02:49:54 ---
Using %{name} and %{version} macros is not mandated. I use %{version} macros,
but prefer not to use %{name} especially when %{name} is longer than cmus
(what it evaluates to). I will fix the quiet makefile issue and fix what you
described w.r.t. the Patch0, but I don't think %{name} is needed.
--
Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
You are the assignee for the bug.
[Bug 165] Review Request: cmus - Ncurses-Based Music Player
http://bugzilla.rpmfusion.org/show_bug.cgi?id=165 Andrea Musuruane musur...@gmail.com changed: What|Removed |Added Summary|cmus - Ncurses-Based Music |Review Request: cmus - |Player |Ncurses-Based Music Player -- Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. You are the assignee for the bug.
[Bug 165] Review Request: cmus - Ncurses-Based Music Player
http://bugzilla.rpmfusion.org/show_bug.cgi?id=165
--- Comment #3 from Andrea Musuruane musur...@gmail.com 2008-12-15 16:41:33
---
Just some questions for now:
* You should really clean up the script. If you don't use patches than do not
include them.
* Why do you do
./configure [..] exampledir=%{_datadir}/%{name}/examples [..]
and then move examples directory somewhere else?
Just set exampledir as %{_defaultdocdir}/%{name}-%{version}/examples and
declare this dir in the files section.
* Why don't you use CONFIG_FFMPEG=y to have .wma support?
* Why do you use both libmodplug and libmikmod since both are used to have .mod
and .x3m support?
* I wonder if libraries should be moved to different packages. Thus the final
user can choose what to install based on his needs.
* There is no compile output. Makefile must be patched to see it.
* Missing doc files: AUTHORS
--
Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
You are the assignee for the bug.
[Bug 165] Review Request: cmus - Ncurses-Based Music Player
http://bugzilla.rpmfusion.org/show_bug.cgi?id=165
--- Comment #4 from Conrad Meyer kon...@tylerc.org 2008-12-15 23:40:15 ---
(In reply to comment #3:)
Just some questions for now:
* You should really clean up the script. If you don't use patches than do
not include them.
OK.
* Why do you do
./configure [..] exampledir=%{_datadir}/%{name}/examples [..]
and then move examples directory somewhere else?
Just set exampledir as %{_defaultdocdir}/%{name}-%{version}/examples and
declare this dir in the files section.
Ah, that's a good idea :).
I tried the above and got this in %doc, though:
error: magic_file(ms,
/var/tmp/cmus-2.2.0-2.fc9-root-konrad/usr/share/doc/cmus-2.2.0/examples)
failed: mode 040755 cannot open
`/var/tmp/cmus-2.2.0-2.fc9-root-konrad/usr/share/doc/cmus-2.2.0/examples' (No
such file or directory)
rpmbuild: rpmfc.c:1386: rpmfcClassify: Assertion `ftype != ((void *)0)' failed.
Aborted
* Why don't you use CONFIG_FFMPEG=y to have .wma support?
Because as of ffmpeg 0.4.9-0.48.20080908.fc9 this feature doesn't compile.
* Why do you use both libmodplug and libmikmod since both are used to have
.mod and .x3m support?
I'm not familiar with .mod/.x3m, can you help me here?
* I wonder if libraries should be moved to different packages. Thus the final
user can choose what to install based on his needs.
Each one is only a few kilobytes (5-15 kiB on x86_64 systems) so I'm inclined
to say just leave them in the main package.
* There is no compile output. Makefile must be patched to see it.
OK.
* Missing doc files: AUTHORS
OK.
--
Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
You are the assignee for the bug.
