Bug 566405 - (nmbscan) Review Request: nmbscan - A NMB/SMB network scanner
Review Request: nmbscan - A NMB/SMB network scanner
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Martin Gieseking
Fedora Extras Quality Assurance
:
Depends On:
Blocks: FE-SECLAB
  Show dependency treegraph
 
Reported: 2010-02-18 07:25 EST by Mykola Ulianytskyi
Modified: 2014-09-24 06:04 EDT (History)
9 users (show)

See Also:
Fixed In Version: nmbscan-1.2.6-1.fc13
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-08-07 19:28:14 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
martin.gieseking: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Mykola Ulianytskyi 2010-02-18 07:25:55 EST
Spec URL: http://repo.lystor.org.ua/fedora/12/SPECS/nmbscan.spec
SRPM URL: http://repo.lystor.org.ua/fedora/12/SRPMS/nmbscan-1.2.5-1.fc12.src.rpm

Description: 
Scans a SMB shares network, using NMB and SMB protocols. Useful to acquire
information on local area network (security audit, etc.)

Matches informations such as NMB/SMB/Windows hostname, IP address, IP
hostname, ethernet MAC address, Windows username, NMB/SMB/Windows domain name
and master browser.

Can discover all NMB/SMB/Windows hosts on a local area network thanks to hosts
lists maintained by master browsers.

$ rpmlint {i386,x86_64,SRPMS}/nmbscan*
3 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:42:34 EST
Builds successfully in mock on Fedora 11 with i386/x86_64 architectures.
Comment 2 Mykola Ulianytskyi 2010-02-19 07:52:50 EST
Builds successfully in mock on Fedora 13 with i386/x86_64 architectures.
Comment 3 Mykola Ulianytskyi 2010-03-13 15:11:51 EST
* Sat Mar 13 2010 Nikolay Ulyanitsky <lystor AT lystor.org.ua> - 1.2.5-2
- Fix the license
- Fix the summary
- Replace generally useful macros by regular commands


Spec URL: http://repo.lystor.org.ua/fedora/12/SPECS/nmbscan.spec
SRPM URL: http://repo.lystor.org.ua/fedora/12/SRPMS/nmbscan-1.2.5-2.fc12.src.rpm


The srpm builds successfully by the mock on Fedora 11, 12, 13 with i386/x86_64
architectures.

The rpmlint output is clean.
Comment 4 Matthias Runge 2010-03-19 16:52:21 EDT
Since the nmbscan script relies on ping, you should add iputils to the requirements. 

arp is provided by package net-tools (missing in spec)
nslookup is provided from bind-utils (missing in spec)

I could do an inofficial review, since I'm not sponsored yet.
Comment 5 Mykola Ulianytskyi 2010-03-19 17:28:58 EDT
Fixed

Spec diff:
@@ -1,6 +1,6 @@
 Name:           nmbscan
 Version:        1.2.5
-Release:        2%{?dist}
+Release:        3%{?dist}
 Summary:        NMB/SMB network scanner

 Group:          Applications/Internet
@@ -10,6 +10,9 @@
 BuildRoot:      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
 BuildArch:      noarch

+Requires:       bind-utils
+Requires:       iputils
+Requires:       net-tools
 Requires:       samba-client


@@ -49,6 +52,9 @@


 %changelog
+* Fri Mar 19 2010 Nikolay Ulyanitsky <lystor AT lystor.org.ua> - 1.2.5-3
+- Add Requires: bind-utils, iputils, net-tools


Spec URL: http://repo.lystor.org.ua/fedora/12/SPECS/nmbscan.spec
SRPM URL: http://repo.lystor.org.ua/fedora/12/SRPMS/nmbscan-1.2.5-3.fc12.src.rpm
Comment 6 Matthias Runge 2010-03-21 02:34:26 EDT
Key:
- = N/A
x = Check
! = Problem
? = Not evaluated

=== REQUIRED ITEMS ===
[x]  Package is named according to the Package Naming Guidelines.
[x]  Spec file name must match the base package %{name}, in the format %{name}.spec.
[ ]  Package meets the Packaging Guidelines.
[x]  Package successfully compiles and builds into binary rpms on at least one supported architecture.
Tested on:
[x]  Rpmlint output:
[mrunge@sofja rpmbuild]$ rpmlint nmbscan*
nmbscan.src: W: spelling-error %description -l en_US hostname -> host name, host-name, hostage
nmbscan.src: W: spelling-error %description -l en_US ethernet -> Ethernet, ether net, ether-net
nmbscan.src: W: spelling-error %description -l en_US username -> user name, user-name, whatshername
Error checking signature of nmbscan-1.2.5-3.fc12.src.rpm: nmbscan-1.2.5-3.fc12.src.rpm: (SHA1) DSA sha1 md5 (GPG) NOT OK (MISSING KEYS:GPG#d8ab0cec) 
1 packages and 1 specfiles checked; 0 errors, 3 warnings.

I guess, thats ok.
[?]  Package is not relocatable.
[x]  Buildroot is correct (%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n))
[x]  Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines.
[x]  License field in the package spec file matches the actual license.
License type: GPLv2+
[x]  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 is included in %doc.
[x]  Spec file is legible and written in American English.
[x]  Sources used to build the package matches the upstream source, as provided in the spec URL.
MD5SUM this package    : 7cbfd9c7ea817e67525006e78fb5d32e
MD5SUM upstream package: 7cbfd9c7ea817e67525006e78fb5d32e
[x]  Package is not known to require ExcludeArch, OR:
Arches excluded:
Why:
[x]  All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines.
[-]  The spec file handles locales properly.
[-]  ldconfig called in %post and %postun if required.
[-]  Package must own all directories that it creates.
[-]  Package requires other packages for directories it uses.
[x]  Package does not contain duplicates in %files.
[x]  Permissions on files are set properly.
[x]  Package has a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
[x]  Package consistently uses macros.
[x]  Package contains code, or permissable content.
[-]  Large documentation files are in a -doc subpackage, if required.
[x]  Package uses nothing in %doc for runtime.
[-]  Header files in -devel subpackage, if present.
[-]  Static libraries in -devel subpackage, if present.
[-]  Package requires pkgconfig, if .pc files are present.
[-]  Development .so files in -devel subpackage, if present.
[-]  Fully versioned dependency in subpackages, if present.
[x]  Package does not contain any libtool archives (.la).
[-]  Package contains a properly installed %{name}.desktop file if it is a GUI application.
[x]  Package does not own files or directories owned by other packages.

=== SUGGESTED ITEMS ===
[x]  Latest version is packaged.
[?]  Package does not include license text files separate from upstream.
[-]  Description and summary sections in the package spec file contains translations for supported Non-English languages, if available.
[x]  Reviewer should test that the package builds in mock.
Tested on: fedora-12-i386
[x]  Package should compile and build into binary rpms on all supported architectures.
Tested on: noarch
[x]  Package functions as described.
[-]  Scriptlets must be sane, if used.
[-]  The placement of pkgconfig(.pc) files are correct.
[-]  File based requires are sane.


I would approve it, but since I'm not approved yet, this review is just informal.
Comment 7 Martin Gieseking 2010-04-09 13:04:40 EDT
Hi Nikolay,

according to the copyright notice included in the shell script, the license of nmbscan is GPLv2, not GPLv2+ because the addition "or (at your option) any later version" is missing.

I also suggest to use the macro %{name} in Source0 and %file:
Source0: http://nmbscan.gbarbier.org/down/%{name}-%{version}.tar.gz
%{_bindir}/%{name}
Comment 8 Mykola Ulianytskyi 2010-04-17 10:33:13 EDT
Hi Martin

Spec diff:
-Release:        3%{?dist}
+Release:        4%{?dist}
...
-License:        GPLv2+
+License:        GPLv2
...
-Source0:        http://nmbscan.gbarbier.org/down/nmbscan-%{version}.tar.gz
+Source0:        http://nmbscan.gbarbier.org/down/%{name}-%{version}.tar.gz
...
-%{_bindir}/nmbscan
+%{_bindir}/%{name}
...
 %changelog
+* Sat Apr 17 2010 Nikolay Ulyanitsky <lystor AT lystor.org.ua> - 1.2.5-4
+- Fix the license and Source0


Spec URL: http://repo.lystor.org.ua/fedora/12/SPECS/nmbscan.spec
SRPM URL:
http://repo.lystor.org.ua/fedora/12/SRPMS/nmbscan-1.2.5-4.fc12.src.rpm
Comment 9 Martin Gieseking 2010-05-12 15:56:57 EDT
Since nobody else seems to be interested in this package, I take the review. :)
Matthias already did a good job with his informal review, and there are just two cosmetic things left I would address:

- Add a comment to the %build section telling that there's nothing to build.

- As suggested by the spell checker, capitalize "Ethernet" in the %description. The other spelling errors can be ignored.


$ rpmlint /var/lib/mock/fedora-12-x86_64/result/*.rpm
nmbscan.noarch: W: spelling-error %description -l en_US hostname -> host name, host-name, hostage
nmbscan.noarch: W: spelling-error %description -l en_US ethernet -> Ethernet, ether net, ether-net
nmbscan.noarch: W: spelling-error %description -l en_US username -> user name, user-name, whatshername
nmbscan.src: W: spelling-error %description -l en_US hostname -> host name, host-name, hostage
nmbscan.src: W: spelling-error %description -l en_US ethernet -> Ethernet, ether net, ether-net
nmbscan.src: W: spelling-error %description -l en_US username -> user name, user-name, whatshername
2 packages and 0 specfiles checked; 0 errors, 6 warnings.

---------------------------------
keys used in following checklist:

[+] OK
[.] OK, not applicable
[X] needs work
---------------------------------

[+] MUST: The package must be named according to the Package Naming Guidelines.
[+] MUST: The spec file name must match the base package %{name}.
[+] MUST: The package must meet the Packaging Guidelines.
[+] MUST: The package must be licensed with a Fedora approved license.
    - GPLv2 according to text in shell script

[+] MUST: The License field in the package spec file must match the actual license.
[+] MUST: The file containing the text of the license(s) for the package must be included in %doc.
[+] MUST: The spec file must be written in American English.
[+] MUST: The spec file for the package MUST be legible.
[+] MUST: The sources used to build the package must match the upstream source.
    $ md5sum nmbscan-1.2.5.tar.gz*
    7cbfd9c7ea817e67525006e78fb5d32e  nmbscan-1.2.5.tar.gz
    7cbfd9c7ea817e67525006e78fb5d32e  nmbscan-1.2.5.tar.gz.1

[+] MUST: The package MUST successfully compile and build into binary rpms on at least one primary architecture.
    - shell script doesn't need to be built
    - package builds in koji:
      http://koji.fedoraproject.org/koji/taskinfo?taskID=2183527

[.] MUST: If the package does not successfully compile ...
[+] MUST: All build dependencies must be listed in BuildRequires.
    - none required

[.] MUST: The spec file MUST handle locales properly.
    - no locales
  
[.] MUST: Packages storing shared library files must call ldconfig in %post and %postun.
[+] MUST: Packages must NOT bundle copies of system libraries.
[.] MUST: If the package is designed to be relocatable, ...
[+] MUST: A package must own all directories that it creates. 
[+] MUST: A Fedora package must not list a file more than once in %files.
[+] MUST: Permissions on files must be set properly.
[+] MUST: Each package must consistently use macros.
[+] MUST: The package must contain code, or permissable content.
[.] MUST: Large documentation files must go in a -doc subpackage.
[+] MUST: Files in %doc must not affect the runtime of the application.
[.] MUST: Header files must be in a -devel package.
[.] MUST: Static libraries must be in a -static package.
[.] MUST: If a package contains library files with a suffix ...
[.] MUST: devel packages must require the base package 
[+] MUST: Packages must NOT contain any .la libtool archives
[.] MUST: Packages containing GUI applications ...
[+] MUST: Packages must not own files or directories already owned by other packages.
[+] MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot}.
[+] MUST: All filenames in rpm packages must be valid UTF-8.

[+] SHOULD: The reviewer should test that the package builds in mock.
    - builds in mock

[.] SHOULD: The package should compile and build into binary rpms on all supported architectures.
    - noarch package

[+] SHOULD: The reviewer should test that the package functions as described.
[.] SHOULD: If scriptlets are used, those scriptlets must be sane.
[.] SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency.
[.] SHOULD: pkgconfig(.pc) should be placed in a -devel pkg.
Comment 10 Martin Gieseking 2010-07-11 06:50:16 EDT
Nikolay, are you still interested in this package? If so, please address the small remaining issues so that we can finish the review.
Comment 11 Michal Ambroz 2010-08-02 19:41:45 EDT
Hello,
as there is long time no update from Nikolay I volunteer to finish those cosmetic changes and co-maintain the package.

Here is updated package with cosmetic changes and manual page :
http://rebus.fedorapeople.org/13/SPECS/nmbscan.spec
http://rebus.fedorapeople.org/13/SRPMS/nmbscan-1.2.5-5.fc13.src.rpm

Output from rpmlint:
$ rpmlint nmbscan-1.2.5-5.fc13.src.rpm nmbscan-1.2.5-5.fc13.noarch.rpm 
2 packages and 0 specfiles checked; 0 errors, 0 warnings.

Best regards
Michal Ambroz
Comment 12 manuel wolfshant 2010-08-03 02:41:53 EDT
If Nikolay is busy and/or cannot finish this submission process, this bug should be closed and a new one should be submitted by the new maintainer.
Comment 13 Michal Ambroz 2010-08-03 07:27:53 EDT
(In reply to comment #12)
> this bug should be closed and a new one should be submitted
> by the new maintainer.    
Hello Manuel, 
I beg to differ - I believe it is better to have it on one place and not split the effort. There is nothing wrong if package has more than one maintainer. In fact it is encouraged to be like that to ensure that package gets proper maintenance in future. 

In Package Review Process (http://fedoraproject.org/wiki/Package_Review_Process) there are defined roles, but it is not said that there can't be more than one maintainer.

In fact in the CVS Admin request you can see even in the example that there could be more package maintainers from the very beginning. 
(http://fedoraproject.org/wiki/Package_SCM_admin_requests#New_Packages)

Manuel - please could you support your claim by some link to some procedure/guideline, which says it should be closed and new reopened? 
I have not found it codified anywhete. If it is just your gut feeling, please reconsider the position.

Best regards
Michal Ambroz
Comment 14 Susi Lehtola 2010-08-03 07:36:27 EDT
Manuel is correct, that is normal practice. Even though it can be considered normal for a package to have *co*maintainers, there is always a *primary* maintainer who is considered responsible.

If Nikolay is not any more willing to tend to this review, you (Michal) must open up a new review request and mark this one as its duplicate.
Comment 15 Matthias Runge 2010-08-03 07:42:25 EDT
Michal,

I think, it's great, if you volunteer and take over this package. Since I can
not see, what information this page here provides you, which a new request can
not provide.

The bug reporter is the person maintaining the package. There is a way to take
over a package, but only, if the package is approved. If I understand it
correctly, co-maintainer get scm access (later on).

What is the problem with the solution, Manuel suggested? I'm sure, someone
(Martin, Manuel(?), or me) will review your package.
Matthias
Comment 16 Mykola Ulianytskyi 2010-08-04 07:47:06 EDT
> Nikolay, are you still interested in this package? If so, please address the
small remaining issues so that we can finish the review.

Hi
Yes. I am still interested in this package and I will finish the nmbscan review process as soon as I can.

Also I allow Michal Ambroz to co-maintain the package.
Comment 17 Mykola Ulianytskyi 2010-08-05 03:43:48 EDT
Spec diff:
--- nmbscan.spec.orig	2010-04-17 17:26:19.000000000 +0300
+++ nmbscan.spec	2010-08-05 10:34:04.730173241 +0300
@@ -1,12 +1,13 @@
 Name:           nmbscan
 Version:        1.2.5
-Release:        4%{?dist}
+Release:        5%{?dist}
 Summary:        NMB/SMB network scanner
 
 Group:          Applications/Internet
 License:        GPLv2
-URL:            http://nmbscan.gbarbier.org/
-Source0:        http://nmbscan.gbarbier.org/down/%{name}-%{version}.tar.gz
+URL:            http://nmbscan.g76r.eu/
+Source0:        http://nmbscan.g76r.eu/down/%{name}-%{version}.tar.gz
+Source1:        %{name}.1
 BuildRoot:      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
 BuildArch:      noarch
 
@@ -20,9 +21,9 @@
 Scans a SMB shares network, using NMB and SMB protocols. Useful to acquire
 an information on a local area network (security audit, etc.)
 
-Matches the information such as NMB/SMB/Windows hostname, IP address, IP
-hostname, ethernet MAC address, Windows username, NMB/SMB/Windows domain name
-and master browser.
+Matches the information such as NMB/SMB/Windows host name, IP address,
+IP host name, Ethernet MAC address, Windows user name,
+NMB/SMB/Windows domain name and master browser.
 
 Can discover all NMB/SMB/Windows hosts on a local area network thanks to 
 hosts lists maintained by master browsers.
@@ -33,12 +34,15 @@
 
 
 %build
+# Nothing to build
 
 
 %install
 rm -rf %{buildroot}
 install -d %{buildroot}%{_bindir}
-install -p -m 755 nmbscan %{buildroot}%{_bindir}/
+install -p -m 0755 nmbscan %{buildroot}%{_bindir}/
+install -d %{buildroot}%{_mandir}/man1
+install -p -m 0644 %{SOURCE1} %{buildroot}%{_mandir}/man1/
 
 
 %clean
@@ -49,9 +53,15 @@
 %defattr(-,root,root,-)
 %doc Documentation/HOWTO_contribute.txt Documentation/gplv2.txt
 %{_bindir}/%{name}
+%{_mandir}/man1/%{name}.1*
 
 
 %changelog
+* Wed Aug 04 2010 Nikolay Ulyanitsky <lystor AT lystor.org.ua> - 1.2.5-5
+- Fixed the project URL
+- Added a man page (thx to Michal Ambroz)
+- Cosmetic changes


$ rpmlint nmbscan-1.2.5-5.fc13.noarch.rpm nmbscan-1.2.5-5.fc13.src.rpm 
2 packages and 0 specfiles checked; 0 errors, 0 warnings.


Spec URL: http://repo.lystor.org.ua/fedora/13/SPECS/nmbscan.spec
SRPM URL: http://repo.lystor.org.ua/fedora/13/SRPMS/nmbscan-1.2.5-5.fc13.src.rpm
Comment 18 Martin Gieseking 2010-08-05 04:12:46 EDT
Nikolay, thanks for your feedback. The package is ready now.

----------------
Package APPROVED
----------------
Comment 19 Mykola Ulianytskyi 2010-08-05 04:40:52 EDT
New Package SCM Request
=======================
Package Name: nmbscan
Short Description: NMB/SMB network scanner
Owners: lystor rebus
Branches: f13 f14
InitialCC:
Comment 20 Kevin Fenzi 2010-08-05 12:59:53 EDT
Git done (by process-git-requests).
Comment 21 Fedora Update System 2010-08-07 05:23:34 EDT
nmbscan-1.2.5-5.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/nmbscan-1.2.5-5.fc13
Comment 22 Fedora Update System 2010-08-07 05:23:40 EDT
nmbscan-1.2.5-5.fc14 has been submitted as an update for Fedora 14.
http://admin.fedoraproject.org/updates/nmbscan-1.2.5-5.fc14
Comment 23 Fedora Update System 2010-08-07 19:28:08 EDT
nmbscan-1.2.5-5.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 24 Fedora Update System 2010-08-18 21:03:23 EDT
nmbscan-1.2.5-5.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 25 Fedora Update System 2010-09-11 21:50:24 EDT
nmbscan-1.2.6-1.fc13 has been submitted as an update for Fedora 13.
https://admin.fedoraproject.org/updates/nmbscan-1.2.6-1.fc13
Comment 26 Fedora Update System 2010-09-11 21:50:31 EDT
nmbscan-1.2.6-1.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/nmbscan-1.2.6-1.fc14
Comment 27 Fedora Update System 2010-09-14 00:55:53 EDT
nmbscan-1.2.6-1.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 28 Fedora Update System 2010-09-15 01:54:29 EDT
nmbscan-1.2.6-1.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 29 Fabian Affolter 2014-09-24 02:38:30 EDT
Package Change Request
======================
Package Name: nmbscan
New Branches: el6 epel7
Owners: fab rebus
InitialCC:
Comment 30 Gwyn Ciesla 2014-09-24 06:04:30 EDT
Git done (by process-git-requests).

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