Bug 566411 - (umit) Review Request: umit - Nmap frontend
Review Request: umit - Nmap frontend
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Kalev Lember
Fedora Extras Quality Assurance
:
Depends On:
Blocks: FE-SECLAB
  Show dependency treegraph
 
Reported: 2010-02-18 07:27 EST by Mykola Ulianytskyi
Modified: 2010-04-09 00:21 EDT (History)
4 users (show)

See Also:
Fixed In Version: umit-1.0-0.3.RC.fc13
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-03-31 21:49:16 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
kalevlember: fedora‑review+
tcallawa: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Mykola Ulianytskyi 2010-02-18 07:27:47 EST
Spec URL: http://repo.lystor.org.ua/fedora/12/SPECS/umit.spec
SRPM URL: http://repo.lystor.org.ua/fedora/12/SRPMS/umit-1.0-0.1.RC.fc12.src.rpm

Description: 
With Umit, you have all the power provided by Nmap through its regular
command line interface, and a lot more in a highly usable and portable
Graphical Interface. Some of its main features include:
    * Easily create powerful Nmap commands and save them as profiles to use
      whenever you need it
    * Edit your Profiles using the Interface Editor
    * Create Profiles with the assistance of a Wizard
    * Group and order you scan results
    * Filter hosts list by services
    * Filter services list by hosts
    * Compare two scan results in one of our three compare modes: text diff,
      graphical comparison and HTML diff
    * Search scan results
    * Use Umit interface through the Web

$ rpmlint {i386,x86_64,SRPMS}/umit*
5 packages and 0 specfiles checked; 0 errors, 0 warnings.

This package builds successfully by mock on i686/x86_64 architectures.

This is one from my first packages and I'm looking for a sponsor.
Comment 1 Mykola Ulianytskyi 2010-02-19 04:43:00 EST
Builds successfully in mock on Fedora 11 with i386/x86_64 architectures.
Comment 2 Mykola Ulianytskyi 2010-02-19 07:53:10 EST
Builds successfully in mock on Fedora 13 with i386/x86_64 architectures.
Comment 3 Kalev Lember 2010-03-12 17:40:20 EST
Taking for review.

The stated "License: GPLv2" is wrong. Some of the files appear to be licensed under GPLv2+, and others are LGPLv2+. The license tag should thus read:
License:        GPLv2+ and LGPLv2+

As per https://fedoraproject.org/wiki/Packaging/SourceURL#Sourceforge.net , you should use the following for source URL:
http://downloads.sourceforge.net/umit/umit-%{version}%{prerelease}.tar.gz

The package uses python gtk2 bindings and needs to require pygtk2 to make sure it is installed at runtime.

You've split html documentation into -doc subpackage. I think it would be better to have this in the main package. Help->Help is supposed to open the html pages, but if -doc subpackage is not installed, the user gets an error dialog instead. It'd be better user experience if everything worked out of the box.
Also, anything marked with %doc must not affect the runtime of the application (http://fedoraproject.org/wiki/Packaging/Guidelines#Documentation), so please don't mark the html documentation with %doc.


Various minor issues
--------------------

%python_sitearch macro defined at the top of the file isn't used anywhere.

> %patch0 -p1 -b .setup.py
The patch -b option is for making backups of the original files. Patch0 is a patch to remove two lines from setup.py file. When the patch command runs, it thus first creates a backup file called setup.py.setup.py which doesn't make much sense. I'd suggest to use something like "%patch0 -p1 -b .check-buildroot", so that the backup file would get named as "setup.py.check-buildroot".

I would suggest to not use %{__command} macros in new spec files, as they are
really not needed and only make the spec file harder to read. Instead of
%{__chmod}, %{__rm}, %{__sed}, %{__ln_s}, and %{__install} I'd use just plain chmod, rm, sed, ln -s, and install.
However, %{__python} might be useful and should remain as macro. Also see how %__ macros are used in the official Fedora python packaging guidelines: http://fedoraproject.org/wiki/Packaging:Python
But this is mostly just personal preference, and up to you if you want to
change that.
Comment 4 Mykola Ulianytskyi 2010-03-13 14:07:22 EST
Hi
Thank you for starting the review.

Spec diff:
--- umit.spec.orig	2010-02-18 12:29:17.000000000 +0200
+++ umit.spec	2010-03-13 20:44:54.432759235 +0200
@@ -1,19 +1,18 @@
 %if ! (0%{?fedora} > 12 || 0%{?rhel} > 5)
 %{!?python_sitelib: %global python_sitelib %(%{__python} -c "from distutils.sysconfig import get_python_lib; print get_python_lib()")}
-%{!?python_sitearch: %global python_sitearch %(%{__python} -c "from distutils.sysconfig import get_python_lib; print get_python_lib(1)")}
 %endif

 %global prerelease RC

 Name:           umit
 Version:        1.0
-Release:        0.1.%{prerelease}%{?dist}
+Release:        0.2.%{prerelease}%{?dist}
 Summary:        Nmap frontend

 Group:          Applications/Internet
-License:        GPLv2
+License:        GPLv2+ and LGPLv2+
 URL:            http://umit.sourceforge.net/
-Source0:        http://downloads.sourceforge.net/project/umit/umit/%{version}%{prerelease}/umit-%{version}%{prerelease}.tar.gz
+Source0:        http://downloads.sourceforge.net/umit/umit-%{version}%{prerelease}.tar.gz
 # http://trac.umitproject.org/ticket/378
 Source1:        umit_48x48.png
 # http://trac.umitproject.org/ticket/378
@@ -29,6 +28,7 @@
 BuildRequires:  python-sphinx

 Requires:       nmap
+Requires:       pygtk2


 %description
@@ -48,19 +48,9 @@
     * Use Umit interface through the Web


-%package doc
-Summary:        Documentation for %{name}
-Group:          Documentation
-Requires:       %{name} = %{version}-%{release}
-
-
-%description doc
-This package contains documentation files for %{name}.
-
-
 %prep
 %setup -q -n %{name}-%{version}%{prerelease}
-%patch0 -p1 -b .setup.py
+%patch0 -p1


 %build
@@ -68,30 +58,30 @@


 %install
-%{__rm} -rf %{buildroot}
+rm -rf %{buildroot}
 %{__python} setup.py install --root %{buildroot}

 # Remove useless files
-%{__rm} %{buildroot}%{_bindir}/uninstall_umit
+rm %{buildroot}%{_bindir}/uninstall_umit

 # Fix permissions
-find %{buildroot} -type f -exec %{__chmod} 644 {} \;
-%{__chmod} 755 %{buildroot}%{_bindir}/*
+find %{buildroot} -type f -exec chmod 644 {} \;
+chmod 755 %{buildroot}%{_bindir}/*

-# Remove interpreter from site-packages
+# Remove a interpreter from the site-packages
 find %{buildroot}%{python_sitelib} -type f -iname "*py" -exec \
-    %{__sed} -i 's/#!\/usr\/bin\/env python//' {} \;
+    sed -i 's/#!\/usr\/bin\/env python//' {} \;

-# Fix file end-of-line encoding
-%{__sed} -i 's/\r//' %{buildroot}%{_docdir}/%{name}/html/_sources/plugins.txt
+# Fix the file end-of-line encoding
+sed -i 's/\r//' %{buildroot}%{_docdir}/%{name}/html/_sources/plugins.txt

-# Install icons
-%{__install} -pm 0644 %{SOURCE1} %{buildroot}%{_datadir}/pixmaps/%{name}
-%{__install} -d %{buildroot}%{_datadir}/icons/hicolor/48x48/apps
-%{__ln_s} ../../../../pixmaps/%{name}/%{name}_48x48.png %{buildroot}%{_datadir}/icons/hicolor/48x48/apps/%{name}.png
+# Install the icons
+install -pm 0644 %{SOURCE1} %{buildroot}%{_datadir}/pixmaps/%{name}
+install -d %{buildroot}%{_datadir}/icons/hicolor/48x48/apps
+ln -s ../../../../pixmaps/%{name}/%{name}_48x48.png %{buildroot}%{_datadir}/icons/hicolor/48x48/apps/%{name}.png

-# Install desktop file
-%{__install} -d %{buildroot}%{_datadir}/applications
+# Install the desktop file
+install -d %{buildroot}%{_datadir}/applications
 desktop-file-install \
     --dir=%{buildroot}%{_datadir}/applications \
     %{SOURCE2}
@@ -100,7 +90,7 @@


 %clean
-%{__rm} -rf %{buildroot}
+rm -rf %{buildroot}


 %post
@@ -121,6 +111,7 @@
 %files -f %{name}.lang
 %defattr(-,root,root,-)
 %doc COPYING COPYING_HIGWIDGETS README
+%doc %{_docdir}/%{name}
 %{_bindir}/%{name}
 %{_bindir}/umit_scheduler.py
 %{_datadir}/applications/%{name}.desktop
@@ -133,12 +124,15 @@
 %{python_sitelib}/%{name}-*.egg-info


-%files doc
-%defattr(-,root,root,-)
-%doc %{_docdir}/%{name}
-
-
 %changelog
+* Sat Mar 13 2010 Nikolay Ulyanitsky <lystor AT lystor.org.ua> - 1.0-0.2.RC
+- Add the pygtk2 to the Requires
+- Fix the license
+- Fix the Source0
+- Remove the unused macro python_sitearch
+- Remove the -doc subpackage
+- Replace generally useful macros by regular commands
+
 * Fri Feb 05 2010 Nikolay Ulyanitsky <lystor AT lystor.org.ua> - 1.0-0.1.RC
 - Initial package build


Spec URL: http://repo.lystor.org.ua/fedora/12/SPECS/umit.spec
SRPM URL: http://repo.lystor.org.ua/fedora/12/SRPMS/umit-1.0-0.2.RC.fc12.src.rpm
Comment 5 Kalev Lember 2010-03-13 17:02:09 EST
The package review process needs reviewers!  If you haven't done any package
reviews recently, please consider doing one.

Fedora review umit-1.0-0.2.RC.fc12.src.rpm 2010-03-13

+ OK
! needs attention

rpmlint output:
umit.src: W: spelling-error Summary(en_US) Nmap -> Map, N map, Nap
umit.src: W: spelling-error Summary(en_US) frontend -> fronted, front end, front-end
umit.noarch: W: spelling-error Summary(en_US) Nmap -> Map, N map, Nap
umit.noarch: W: spelling-error Summary(en_US) frontend -> fronted, front end, front-end
2 packages and 1 specfiles checked; 0 errors, 4 warnings.

! rpmlint warning about "frontend" is correct and it should be changed to "front-end".
The other spelling error is just harmless noise.

+ The package is named according to the Package Naming Guidelines.
+ Spec file name matches the base package name
+ The package follows the Packaging Guidelines
+ The package is licensed with a Fedora approved license and meets the Licensing Guidelines.
+ The license field in the spec file matches the actual license
+ The package contains the license files (COPYING, COPYING_HIGWIDGETS)
+ Spec file is written in American English
+ Spec file is legible
+ Upstream sources match sources in the srpm. md5sum:
  e096ac3795017ba87be4ed569c520be8  umit-1.0RC.tar.gz
  e096ac3795017ba87be4ed569c520be8  Download/umit-1.0RC.tar.gz
+ The package builds in koji
n/a ExcludeArch bugs filed
+ BuildRequires look sane
+ The spec file handles locales properly
+ Packages does not bundle copies of system libraries
n/a Does not use Prefix: /usr
+ Package owns all directories it creates
+ No duplicate files in %files
+ Permissions are properly set and %files has %defattr
+ %clean contains rm -rf %{buildroot}
+ Consistent use of macros
+ Package contains code or permissible content
n/a Large documentation files should go in -doc subpackage
! Files marked %doc should not affect package
%{_docdir}/%{name} shouldn't be marked as %doc because the content affects package runtime (an error is displayed clicking Help->Help if the files don't exist).

n/a Header files should be in -devel
n/a Static libraries should be in -static
n/a Library files that end in .so must go in a -devel package
n/a -devel must require the fully versioned base
n/a Packages should not contain libtool .la files
+ Packages containing GUI apps must include %{name}.desktop file
+ Packages must not own files or directories owned by other packages
+ Filenames must be valid UTF-8


Please fix frontend -> front-end and remove the %doc before importing it into CVS. Those are minor issues and otherwise the package looks good.

APPROVED
Comment 6 Mykola Ulianytskyi 2010-03-15 03:55:14 EDT
> Please fix frontend -> front-end and remove the %doc before importing it into
> CVS. Those are minor issues and otherwise the package looks good.

Done

Spec URL: http://repo.lystor.org.ua/fedora/12/SPECS/umit.spec
SRPM URL: http://repo.lystor.org.ua/fedora/12/SRPMS/umit-1.0-0.3.RC.fc12.src.rpm
Comment 7 Mykola Ulianytskyi 2010-03-15 03:59:03 EDT
New Package CVS Request
=======================
Package Name: umit
Short Description: Nmap front-end
Owners: lystor
Branches: F-11 F-12 F-13
InitialCC:
Comment 8 Tom "spot" Callaway 2010-03-15 17:42:02 EDT
CVS done (by process-cvs-requests.py).
Comment 9 Fedora Update System 2010-03-16 08:20:02 EDT
umit-1.0-0.3.RC.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/umit-1.0-0.3.RC.fc13
Comment 10 Fedora Update System 2010-03-16 08:20:47 EDT
umit-1.0-0.3.RC.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/umit-1.0-0.3.RC.fc12
Comment 11 Fedora Update System 2010-03-16 08:21:37 EDT
umit-1.0-0.3.RC.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/umit-1.0-0.3.RC.fc11
Comment 12 Fedora Update System 2010-03-17 23:22:29 EDT
umit-1.0-0.3.RC.fc11 has been pushed to the Fedora 11 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 umit'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/umit-1.0-0.3.RC.fc11
Comment 13 Fedora Update System 2010-03-17 23:29:55 EDT
umit-1.0-0.3.RC.fc13 has been pushed to the Fedora 13 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 umit'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/umit-1.0-0.3.RC.fc13
Comment 14 Fedora Update System 2010-03-17 23:32:46 EDT
umit-1.0-0.3.RC.fc12 has been pushed to the Fedora 12 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 umit'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/umit-1.0-0.3.RC.fc12
Comment 15 Fedora Update System 2010-03-31 21:49:10 EDT
umit-1.0-0.3.RC.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 16 Fedora Update System 2010-03-31 21:56:45 EDT
umit-1.0-0.3.RC.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 17 Fedora Update System 2010-04-09 00:21:13 EDT
umit-1.0-0.3.RC.fc13 has been pushed to the Fedora 13 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.