Bug 592772 - Review Request: drobo-utils - Utilities for managing Drobo storage systems
Summary: Review Request: drobo-utils - Utilities for managing Drobo storage systems
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-05-16 20:36 UTC by Jameson
Modified: 2011-02-04 19:52 UTC (History)
4 users (show)

Fixed In Version: drobo-utils-0.6.2.2-9.fc14
Clone Of:
Environment:
Last Closed: 2011-02-04 19:52:16 UTC
Type: ---
Embargoed:
j: fedora-review+
petersen: fedora-cvs+


Attachments (Terms of Use)

Description Jameson 2010-05-16 20:36:47 UTC
Spec URL: http://imntreal.fedorapeople.org/drobo-utils.spec
SRPM URL: http://imntreal.fedorapeople.org/drobo-utils-0.6.2.2-4.fc12.src.rpm
Description: Drobo-utils is a set of Linux tools to query and manage Data Robotics Drobo storage systems (Drobos).  This package provides a graphical utility.

Comment 1 Jameson 2010-05-16 22:18:55 UTC
Updated the SPEC, and SRPM:
http://imntreal.fedorapeople.org/drobo-utils-0.6.2.2-5.fc12.src.rpm

Comment 2 Jameson 2010-05-21 17:10:40 UTC
rpmlint is quiet except for a spelling warning.

Comment 3 Jason Tibbitts 2010-11-23 18:10:07 UTC
I haven't the hardware to test this, and I suspect that no other review has any either, which would explain why you've had no comments on this ticket.  But I'm just trying to clear out the old ones, and this package seems clean and simple enough.  Here are some comments.

You can remove the 0%{?fedora} > 12 conditional; all supported Fedora releases will meet it.  You can also remove mention of %python_sitearch, since you don't use it.  (You could remove the whole thing at the top if you only intend to target Fedora and RHEL6.  You could then also remove BuildRoot, %clean and the buildroot cleaning in %install.)

I don't understand why you test for pam_stack in %prep.  You control what's on the buildsystem based on the build dependencies you list, and you should know what each Fedora release supports.  You can't test what's on the end-user system at package build time.

Your %post and %postun scripts do not conform to those given in http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#desktop-database

I don't understand why you make a point of passing CFLAGS in %build, given that this is a noarch package and the C compiler is not being called.

In ssome places you reference %{buildroot} while in others, $RPM_BUILD_ROOT.  You need to be consistent.  (Personally I prefer the one with fewer characters.)

Comment 4 Jameson 2010-11-26 20:48:54 UTC
Without the CFLAGS line, I get:
running install_scripts
error: cannot copy tree 'build/scripts-2.7': not a directory

Could this actually be compiling something?  The author said that it no longer does.  This is my first Python package, so some of it is a bit strange.  I think I have everything else cleaned up.

New SRPM:  http://imntreal.fedorapeople.org/drobo-utils-0.6.2.2-6.fc14.src.rpm
New SPEC:  http://imntreal.fedorapeople.org/drobo-utils.spec

Comment 5 Jason Tibbitts 2010-12-01 03:02:45 UTC
Well, you still need to have the python call; you just don't need to set CFLAGS:
  %{__python} -c 'import setuptools; execfile("setup.py")' build
The compiler is not called during the build.

Also, rpmlint has a complaint I've not seen before:

drobo-utils-gui.noarch: E: use-old-pam-stack /etc/pam.d/droboview-root (line 4)

The implication is that pam files are supposed to use "include" instead of the pam_stack module.  Honestly I can't tell you how to do that.

Comment 6 Paul Howarth 2010-12-01 10:03:45 UTC
(In reply to comment #5)
> Also, rpmlint has a complaint I've not seen before:
> 
> drobo-utils-gui.noarch: E: use-old-pam-stack /etc/pam.d/droboview-root (line 4)
> 
> The implication is that pam files are supposed to use "include" instead of the
> pam_stack module.  Honestly I can't tell you how to do that.

Removing these lines from the spec should do the trick:

# Detect whether the system is using pam_stack
perl -pi -e's,include(\s*)(.*),required\1pam_stack.so service=\2,' droboview.pam
touch -r %{SOURCE3} droboview.pam

Comment 7 Jameson 2010-12-13 16:22:40 UTC
Removing the CFLAGS line gives me:
error: cannot copy tree 'build/scripts-2.7': not a directory

Here's an update without the old pam lines:
http://imntreal.fedorapeople.org/drobo-utils-0.6.2.2-7.fc14.src.rpm
http://imntreal.fedorapeople.org/drobo-utils.spec

Current rpmlint output:
drobo-utils.spec: W: no-cleaning-of-buildroot %install
drobo-utils.spec: W: no-cleaning-of-buildroot %clean
drobo-utils.spec: W: no-buildroot-tag
drobo-utils.spec: W: no-%clean-section

drobo-utils-gui.noarch: W: conffile-without-noreplace-flag /etc/security/console.apps/droboview-root
drobo-utils-gui.noarch: W: conffile-without-noreplace-flag /etc/pam.d/droboview-root
drobo-utils-gui.noarch: W: no-manual-page-for-binary droboview-root

Excuse my confusion.  This is the first package I've worked on that needed GUI sudo, and my first python package.  I can testify that it currently functions properly on my system, but it would be nice if there were more people out there with the hardware to test it.

Comment 8 Jason Tibbitts 2010-12-13 18:49:49 UTC
I believe I already addressed the CFLAGS bit, didn't I?  You don't remove the whole line, you just remove the setting of CFLAGS from the line (since it's completely pointless as nothing is calling a C compiler).  Obviously if you don't call python at all then the package doesn't build.

Maybe it's better if I just show what the %build section would look like:

%build
%{__python} -c 'import setuptools; execfile("setup.py")' build
sed -i -e "s/\#\!\/.*//g" DroboGUI.py

That builds fine for me.

Comment 9 Jameson 2010-12-13 19:14:56 UTC
Sorry, I got'cha, now.

http://imntreal.fedorapeople.org/drobo-utils-0.6.2.2-8.fc14.src.rpm

SPEC is in the same place.  Everything seems ok.  Still have the same rpmlint warnings, but I don't think anything can be done about those.

Comment 10 Jason Tibbitts 2010-12-14 15:26:22 UTC
Actually if you use the latest rpmlint you get no complaints from the srpm and just these from the built rpm:
  drobo-utils-gui.noarch: W: conffile-without-noreplace-flag
   /etc/security/console.apps/droboview-root
  drobo-utils-gui.noarch: W: conffile-without-noreplace-flag
   /etc/pam.d/droboview-root
  drobo-utils-gui.noarch: W: no-manual-page-for-binary droboview-root
which are all OK.

I'm having trouble figuring out the license.  The code itself doesn't have the proper license blocks and just refers to the COPYING file.  The COPYING file just has the generic GPLv3 text, which says (see section 14) that unless the code specifically indicates a version, you can use any version.  The README files and upstream web site just say "GPL".  The About dialog for the GUI says:
  license: General Public License (GPL) v3
and you have "GPLv3+" in the spec.

At this point I'd go with the About dialog and use "GPLv3", but you should ask upstream to clarify and if possible put proper GPL license blocks in their code.  The COPYING file itself indicates how to do this, down at the bottom under "How to Apply These Terms to Your New Programs".

Other than the license issue I think this is fine.

* source files match upstream.  sha256sum:
  00f2bc162c0050da9630d3134ef4317c042fec22c558f2933c7f936bfea4ebf6
   drobo-utils-0.6.2.2.tgz
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summaries are OK.
* descriptions are OK.
* dist tag is present.
X license field does not match the actual license.
* license is open source-compatible.
* license text included in package.
* latest version is being packaged.
* BuildRequires are proper.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* rpmlint has acceptable complaints.
* final provides and requires are sane:
  drobo-utils-0.6.2.2-8.fc15.noarch.rpm
   drobo-utils = 0.6.2.2-8.fc15
  =
   /usr/bin/python  
   parted  
   python(abi) = 2.7
   python-ctypes  

  drobo-utils-gui-0.6.2.2-8.fc15.noarch.rpm
   config(drobo-utils-gui) = 0.6.2.2-8.fc15
   drobo-utils-gui = 0.6.2.2-8.fc15
  =
   /bin/sh  
   PyQt4  
   config(drobo-utils-gui) = 0.6.2.2-8.fc15
   drobo-utils  
   usermode  

* no bundled libraries.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no generically named files
* code, not content.
* documentation is small, so no -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* desktop files valid and installed properly.

Comment 11 Jameson 2011-01-03 20:18:22 UTC
I've e-mailed upstream.  While I'm waiting to hear back, I've updated the license tag to GPLv3, and put a new SRPM out there:

http://imntreal.fedorapeople.org/drobo-utils-0.6.2.2-9.fc14.src.rpm

Comment 12 Jason Tibbitts 2011-01-05 00:39:58 UTC
Thanks; following the about dialog is the only thing we can do without clarification from upstream.  If you get that clarification, please include the email in the package as documentation.

APPROVED

Comment 13 Jameson 2011-01-15 19:59:45 UTC
New Package SCM Request
=======================
Package Name: drobo-utils
Short Description: Utilities for managing Drobo storage systems
Owners: imntreal
Branches: f14
InitialCC: imntreal

Comment 14 Jens Petersen 2011-01-18 06:54:29 UTC
Git done (by process-git-requests).

Comment 15 Fedora Update System 2011-01-18 19:48:06 UTC
drobo-utils-0.6.2.2-9.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/drobo-utils-0.6.2.2-9.fc14

Comment 16 Fedora Update System 2011-01-19 21:02:29 UTC
drobo-utils-0.6.2.2-9.fc14 has been pushed to the Fedora 14 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update drobo-utils'.  You can provide feedback for this update here: https://admin.fedoraproject.org/updates/drobo-utils-0.6.2.2-9.fc14

Comment 17 Fedora Update System 2011-02-04 19:52:10 UTC
drobo-utils-0.6.2.2-9.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.