[Bug 823163] Review Request: dpm-contrib-admintools - DPM administration toolkit (contrib from GridPP)

Tue Jun 5 12:42:42 UTC 2012

https://bugzilla.redhat.com/show_bug.cgi?id=823163

--- Comment #4 from Ricardo Rocha <rocha.porto at gmail.com> ---
Hi.

Thanks for the review, spec and srpm updated:
https://rocha.web.cern.ch/rocha/fedora/dpm-contrib-admintools.spec
https://rocha.web.cern.ch/rocha/fedora/dpm-contrib-admintools-0.2.0-2.src.rpm

Koji builds (success):
http://koji.fedoraproject.org/koji/taskinfo?taskID=4128602 (rawhide)
http://koji.fedoraproject.org/koji/taskinfo?taskID=4128606 (5E)

Details inline below.

> [OK] MUST: rpmlint must be run on the source rpm and all binary rpms the
> build produces. The output should be posted in the review.
> 
> dpm-contrib-admintools.i686: W: no-manual-page-for-binary
> dpm-sql-spacetoken-list-files
> dpm-contrib-admintools.i686: W: no-manual-page-for-binary dpm-disk-to-dpns
> dpm-contrib-admintools.i686: W: no-manual-page-for-binary
> dpns-usage-by-vouser
> dpm-contrib-admintools.i686: W: no-manual-page-for-binary dpm-delreplica
> dpm-contrib-admintools.i686: W: no-manual-page-for-binary
> dpm-sql-dpns-by-replication
> dpm-contrib-admintools.i686: W: no-manual-page-for-binary
> dpm-sql-spacetoken-replicate-hotfiles
> dpm-contrib-admintools.i686: W: no-manual-page-for-binary
> dpm-sql-spacetoken-usage
> dpm-contrib-admintools.i686: W: no-manual-page-for-binary
> dpm-sql-diskfs-to-dpns-chk
> dpm-contrib-admintools.i686: W: no-manual-page-for-binary dpns-du
> dpm-contrib-admintools.i686: W: no-manual-page-for-binary dpm-list-disk
> dpm-contrib-admintools.i686: W: no-manual-page-for-binary
> dpm-sql-usage-by-vo-user
> dpm-contrib-admintools.i686: W: no-manual-page-for-binary
> dpm-sql-list-hotfiles
> dpm-contrib-admintools.i686: W: no-manual-page-for-binary adler32sum
> dpm-contrib-admintools.i686: W: no-manual-page-for-binary dpns-find
> dpm-contrib-admintools.i686: W: no-manual-page-for-binary dpm-sql-pfn-to-dpns
> dpm-contrib-admintools.i686: W: no-manual-page-for-binary
> dpm-sql-files-by-vo-user
> dpm-contrib-admintools.i686: W: no-manual-page-for-binary
> dpm-sql-dpns-to-diskfs-chk
> dpm-contrib-admintools.src: W: invalid-url Source0:
> dpm-contrib-admintools-0.2.0.tar.gz
> 3 packages and 0 specfiles checked; 0 errors, 18 warnings.
> 
> 		-> minors warnings
> 
> 
> [OK] MUST: The package must be named according to the Package Naming
> Guidelines .
> [OK] 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 .
> 
> 	-> Missing %{?_isa} macro on python-dpm and MySQL-python
>                   
> http://fedoraproject.org/wiki/Packaging:Guidelines#Requires
> 
>         -> make is not executed properly, compilation is done  by  "make
> install" in the install section

Both fixed.

> 	-> -DCMAKE_INSTALL_PREFIX=/  is an override of the %cmake macro, this is
> not recommended.

The build is requiring this, there are many other components following this. If
it's ok with you i'll leave it like this for now.

> 
> [OK] MUST: The package must be licensed with a Fedora approved license and
> meet the Licensing Guidelines .
> [OK] MUST: The License field in the package spec file must match the actual
> license. 
> [N/A] 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.
> [OK] MUST: The spec file must be written in American English. 
> [OK] MUST: The spec file for the package MUST be legible. 
> [OK] 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.
> [OK] MUST: The package MUST successfully compile and build into binary rpms
> on at least one primary architecture. 
> [N/A] 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. 
> [OK] 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.
> [OK] MUST: The spec file MUST handle locales properly. This is done by using
> the %find_lang macro. Using %{_datadir}/locale/* is strictly forbidden.
> [N/A] 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. 
> [OK] MUST: Packages must NOT bundle copies of system libraries.
> [OK] 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. 
> [OK] 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. 
> [OK] 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)
> [OK] MUST: Permissions on files must be set properly. Executables should be
> set with executable permissions, for example. 
> [OK] MUST: Each package must consistently use macros. 
> [OK] MUST: The package must contain code, or permissable content. 
> [OK] 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). 
> [OK] 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. 
> [N/A] MUST: Header files must be in a -devel package. 
> [N/A] MUST: Static libraries must be in a -static package. 
> [N/A] 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/A] MUST: In the vast majority of cases, devel packages must require the
> base package using a fully versioned dependency: Requires: %{name}%{?_isa} =
> %{version}-%{release} 
> [N/A] MUST: Packages must NOT contain any .la libtool archives, these must
> be removed in the spec if they are built.
> [N/A] 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. 
> [OK] 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. 
> [OK] MUST: All filenames in rpm packages must be valid UTF-8. 
> 
> 
> [OK] 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. 
> [OK] SHOULD: The description and summary sections in the package spec file
> should contain translations for supported Non-English languages, if
> available. 
> [OK] SHOULD: The reviewer should test that the package builds in mock. 
> [OK] SHOULD: The package should compile and build into binary rpms on all
> supported architectures. 
> [OK] SHOULD: The reviewer should test that the package functions as
> described. A package should not segfault instead of running, for example.
> [N/A] SHOULD: If scriptlets are used, those scriptlets must be sane. This is
> vague, and left up to the reviewers judgement to determine sanity. 
> [N/A] SHOULD: Usually, subpackages other than devel should require the base
> package using a fully versioned dependency. 
> [N/A] 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. 
> [OK] 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.
> 
> 
>    -> Create and include man pages if possible

As mentioned above i've added the request upstream:
https://svnweb.cern.ch/trac/lcgdm/ticket/524

and it will come in a later release.

Thanks again,
  Ricardo(In reply to comment #3)

-- 
You are receiving this mail because:
You are on the CC list for the bug.