[Bug 749132] Review Request: dpm-dsi - Disk Pool Manager (DPM) plugin to GridFTP

[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=749132

--- Comment #2 from Steve Traylen <steve.traylen@xxxxxxx> 2011-10-29 09:19:46 EDT ---
Review of dpm-dsi, Sat 29th October 2011
https://bugzilla.redhat.com/show_bug.cgi?id=749132

[yes] specfiles match: dpm-dsi is the SVN module name.
[no] source files match upstream: 
This is built from SVN.  Your comments for createing the 
.spec file mention how to export the source but not how 
to create the tar ball. I realize it's obvious but please add it. 
See:
http://fedoraproject.org/wiki/Packaging:SourceURL#Using_Revision_Control
Please expand on your instructions.

[yes] package meets naming and versioning guidelines.
expect where detailed elsewhere.
[no] spec is properly named, cleanly written, and uses macros consistently.
You use '/usr' during the installation which should be %{prefix} or similar.


[yes] dist tag is present.
[yes] build root is correct. 
Though can be dropped unless EPEL5 is being targeted.

[no] license field matches the actual license.
.spec file says ASL2.0 but src/globus_gridftp_server_dpm.c 
talks about a globus license.

On a similar note what is src/gssapi_openssl.h for instance, is
not just a duplication of 

[yes] license is open source-compatible.
ASL2.0 is but see above.
[yes] license text included in package but see above.
[notchecked] latest version is being packaged.
[yes] BuildRequires are proper.

[no] compiler flags are appropriate.
cc -g -Wall -fPIC -D_LARGEFILE64_SOURCE
You just have a plain ./configure and not a %configure
See: http://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags 

[yes] %clean is present. 
But not needed  unless EPEL5

[yes] package builds in mock.
[no] package installs properly.
See 'requires' below.

[yes] rpmlint is silent or justified.
$ rpmlint  dpm-dsi.spec 
dpm-dsi.spec: W: invalid-url Source0: dpm-dsi-1.8.2.tar.gz
0 packages and 1 specfiles checked; 0 errors, 1 warnings.

dpm-dsi.x86_64: 
W: incoherent-init-script-name dpm-gsiftp ('dpm-dsi', 'dpm-dsid')

This is expected

[no] final provides and requires are sane
dpm-dsi-1.8.2-1.fc15.x86_64.rpm provides: 
   libglobus_gridftp_server_dpm.so.1()(64bit)  
   dpm-dsi(x86-64) = 1.8.2-1.fc15
dpm-dsi-devel-1.8.2-1.fc15.x86_64.rpm provides:
   dpm-dsi--devel = 1.8.2-1.fc15
   dpm-dsi-devel(x86-64) = 1.8.2-1.fc15

dpm-dsi-1.8.2-1.fc15.x86_64.rpm requires
   config(dpm-dsi) = 1.8.2-1.fc15
   globus-gridftp-server-progs(x86-64)  
   initscripts  
   libdl.so.2()(64bit)  
   libdpm.so.1()(64bit)  
   libglobus_ftp_control.so.1()(64bit)  
   libvomsapi.so.1()(64bit)  
   voms(x86-64)  

So the explicit requirement 'voms(x86-64)' is not needed and should be
removed.

dpm-dsi-devel-1.8.2-1.fc15.x86_64.rpm requires
    dpm-dsi-libs(x86-64) = 1.8.2-1.fc15
    libglobus_gridftp_server_dpm.so.1()(64bit)  

dpm-dsi-libs ? This is presumably just meant to be dpm-dsi unless
you meant to create a seperate libs package in the first place?


[none] %check is present and all tests pass:
[none] no shared libraries are added to the regular linker search paths.
[yes] owns the directories it creates. 
[yes] doesn't own any directories it shouldn't.
[yes] no duplicates in %files.
[yes] file permissions are appropriate.
[no] scriptlets match those on ScriptletSnippets page.
Check when ldconfig should be run, in all %post and %postun
and not just on package removal.


[yes] code, not content.
[yes] documentation is small, so no -docs subpackage is necessary.
[yes] %docs are not necessary for the proper functioning of the package.
[question] headers are devel file.
There are no header files in the -devel package?
[yes] no pkgconfig files.
[yes] no libtool .la droppings.
[yes] desktop files valid and installed properly, no guis.


More comments:
Can you confirm you are trargeting EPEL 5 (or even 4) otherwise a lot
of "junk" can be dropped like BuildRoot.

You should add a '-p' to your install lines to preserve the timestamp
on the files between releases.

The URL http://svnweb.cern.ch/trac/lcgdm/wiki/Dpm redirects
to https and this causes an rpmlint warning. Is there any reason
not to just use https in the first place.

The main one for now is dpm-dsi-libs or not since then I can actually
install to check.

What is 'dsi' the package is called dpm-dsi but dsi is not mentined
in the description at all?

-- 
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]