[Bug 772406] Review Request: cpulimit - CPU Usage Limiter for Linux

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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=772406

--- Comment #5 from Christos Triantafyllidis <christos.triantafyllidis@xxxxxxxxx> 2012-01-08 09:56:15 EST ---
Many thanks adev for the informal review!!


(In reply to comment #3)
> Informal package Review, I am not a sponsor.
> 
> 
> 
> [FAIL] MUST: rpmlint must be run on the source rpm and all binary rpms the
> build produces. The output should be posted in the review.
> 
> cpulimit.x86_64: W: incoherent-version-in-changelog 1.1-1 ['1.1-1.el5.centos',
> '1.1-1.centos']
> cpulimit.x86_64: W: no-documentation
> cpulimit-debuginfo.x86_64: E: debuginfo-without-sources
> 
> no-documentation -> no man pages or documentation files

No man page from UPSTREAM, there is a bug report with a man page for too long
but never included. I decided not to put it on my self based on:
https://bugzilla.redhat.com/show_bug.cgi?id=513541

> incoherent-version-in-changelog -> need to be set properly

I guess i miss a %{dist} there... is it required? changelog (at least to this
point) is not dist specific. Rpmlint on rawhide binaries doesn't through this
as an error. (trivial to fix if needed)

> cpulimit-debuginfo -> compiled without -g flag

Fixed in meanwhile :).

> 
> [PASS] MUST: The package must be named according to the Package Naming
> Guidelines .
> [PASS] MUST: The spec file name must match the base package %{name}, in the
> format %{name}.spec unless your package has an exemption.
> [FAIL] MUST: The package must meet the Packaging Guidelines .
> 
>  -> Usage of  %{optflags} for compilation flags if possible
Fixed

>  -> No debuginfo because of no -g opts
Fixed

>  -> why several src rpm ?
One per dist, this is required i think.

> 
> [PASS] MUST: The package must be licensed with a Fedora approved license and
> meet the Licensing Guidelines .
> [PASS] MUST: The License field in the package spec file must match the actual
> license. 
> [PASS] 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.
> [PASS] MUST: The spec file must be written in American English. 
> [PASS] MUST: The spec file for the package MUST be legible. 
> [PASS] MUST: The sources used to build the package must match the upstream
> source, as provided in the spec URL. Reviewers should use md5sum for this task.
> If no upstream URL can be specified for this package, please see the Source URL
> Guidelines for how to deal with this.
> 
>  -> rpmdev-md5 cpulimit-1.1-1.el5.src.rpm
>   00d6d2fabdcb2ecfa2355724e9dc3f4b  cpulimit-1.1-1.el5.src.rpm
>   f4ff6d4bfaef1258e8f5cd2041e2e2a3  cpulimit-1.1.tar.gz
>   e5a087539a57670534d019f92f19e119  cpulimit.spec
>  -> md5sum cpulimit-1.1.tar.gz 
>   f4ff6d4bfaef1258e8f5cd2041e2e2a3  cpulimit-1.1.tar.gz
>  -> md5sum cpulimit-1.1.tar.gz specs/cpulimit.spec 
>   e5a087539a57670534d019f92f19e119  specs/cpulimit.spec
> 
> 
> [PASS] MUST: The package MUST successfully compile and build into binary rpms
> on at least one primary architecture. 
> [PASS] 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. Each architecture listed in ExcludeArch MUST have a bug filed in
> bugzilla, describing the reason that the package does not compile/build/work on
> that architecture. The bug number MUST be placed in a comment, next to the
> corresponding ExcludeArch line. 
> [PASS] MUST: All build dependencies must be listed in BuildRequires, except for
> any that are listed in the exceptions section of the Packaging Guidelines ;
> inclusion of those as BuildRequires is optional. Apply common sense.
> [PASS] MUST: The spec file MUST handle locales properly. This is done by using
> the %find_lang macro. Using %{_datadir}/locale/* is strictly forbidden.
> [PASS] MUST: Every binary RPM package (or subpackage) which stores shared
> library files (not just symlinks) in any of the dynamic linker's default paths,
> must call ldconfig in %post and %postun. 
> [PASS] MUST: Packages must NOT bundle copies of system libraries.
> [PASS] MUST: If the package is designed to be relocatable, the packager must
> state this fact in the request for review, along with the rationalization for
> relocation of that specific package. Without this, use of Prefix: /usr is
> considered a blocker. 
> [PASS] 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. 
> [PASS] MUST: A Fedora package must not list a file more than once in the spec
> file's %files listings. (Notable exception: license texts in specific
> situations)
> [PASS] MUST: Permissions on files must be set properly. Executables should be
> set with executable permissions, for example. 
> [FAIL] MUST: Each package must consistently use macros. 
> 
>  -> /usr/bin paths can be replaced by %{_bindir} macro

Fixed

> 
> [PASS] MUST: The package must contain code, or permissable content. 
> [PASS] MUST: Large documentation files must go in a -doc subpackage. (The
> definition of large is left up to the packager's best judgement, but is not
> restricted to size. Large can refer to either size or quantity). 
> [PASS] MUST: If a package includes something as %doc, it must not affect the
> runtime of the application. To summarize: If it is in %doc, the program must
> run properly if it is not present. 
> [PASS] MUST: Header files must be in a -devel package. 
> [PASS] MUST: Static libraries must be in a -static package. 
> [PASS] 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. 
> [PASS] MUST: In the vast majority of cases, devel packages must require the
> base package using a fully versioned dependency: Requires: %{name}%{?_isa} =
> %{version}-%{release} 
> [PASS] MUST: Packages must NOT contain any .la libtool archives, these must be
> removed in the spec if they are built.
> [PASS] 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. If you feel that your packaged
> GUI application does not need a .desktop file, you must put a comment in the
> spec file with your explanation. 
> [PASS] MUST: Packages must not own files or directories already owned by other
> packages. The rule of thumb here is that the first package to be installed
> should own the files or directories that other packages may rely upon. This
> means, for example, that no package in Fedora should ever share ownership with
> any of the files or directories owned by the filesystem or man package. If you
> feel that you have a good reason to own a file or directory that another
> package owns, then please present that at package review time. 
> [PASS] MUST: All filenames in rpm packages must be valid UTF-8. 
> 
> 
> [FAIL] 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. 
> 
>  -> add license file in the package

I could ping upstream to add a license file but i don't think this will be done
soon based on reactions of the upstream to other reports. Is this a blocker?

> 
> [PASS] SHOULD: The description and summary sections in the package spec file
> should contain translations for supported Non-English languages, if available. 
> [PASS] SHOULD: The reviewer should test that the package builds in mock. 
> SHOULD: The package should compile and build into binary rpms on all supported
> architectures.
>  f15-candidate x86_64 -> success
>  f15-candidate i386 -> success 
> 
> [PASS] SHOULD: The reviewer should test that the package functions as
> described. A package should not segfault instead of running, for example.
> [PASS] SHOULD: If scriptlets are used, those scriptlets must be sane. This is
> vague, and left up to the reviewers judgement to determine sanity. 
> [PASS] SHOULD: Usually, subpackages other than devel should require the base
> package using a fully versioned dependency. 
> [PASS] SHOULD: The placement of pkgconfig(.pc) files depends on their usecase,
> and this is usually for development purposes, so should be placed in a -devel
> pkg. A reasonable exception is that the main pkg itself is a devel tool not
> installed in a user runtime, e.g. gcc or gdb. 
> [PASS] SHOULD: If the package has file dependencies outside of /etc, /bin,
> /sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the
> file instead of the file itself. 
> [FAIL] SHOULD: your package should contain man pages for binaries/scripts. If
> it doesn't, work with upstream to add them where they make sense.
> 
>  -> lack of documentation files ( man pages )

See comment above. A man page (created for debian shake) has been submitted as
bug to UPSTREAM but was never included. I fully agree with comments on about
possible inaccuracies and deprecation in future thus i'd prefer not to add one:
https://bugzilla.redhat.com/show_bug.cgi?id=513541

If this is a blocker i could add it though :(.

Many many thanks for the review.

Christos

-- 
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.
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review



[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]