Bug 1245255 - Review Request: netspy2ban - GUI Networking Tool and Fail2ban Controller
Summary: Review Request: netspy2ban - GUI Networking Tool and Fail2ban Controller
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: FE-DEADREVIEW
TreeView+ depends on / blocked
 
Reported: 2015-07-21 14:51 UTC by FT
Modified: 2018-03-16 16:50 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2018-03-16 16:50:42 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description FT 2015-07-21 14:51:29 UTC
Spec URL: https://github.com/ftsiadimos/netspy2ban/blob/master/rpms/netspy2ban.spec
SRPM URL: https://github.com/ftsiadimos/netspy2ban/blob/master/rpms/netspy2ban-1.0-1.fc22.src.rpm
Description: NetSpy2Ban is a graphic user interface program for Fedora 22 OS. The program serves three functions. The first function is to view connected network cards and their speed. The second is to allow real time monitoring of your network connections. Lastly, NetSpy2Ban includes a graphic user interface to provide user-friendly functionality for the Fail2Ban service.
Fedora Account System Username:

Comment 1 Fotios Tsiadimos 2015-07-28 17:18:49 UTC
Hi,

I fixed my spec with macros and now is ready to pass. 

[fotis@fotis SPECS] $ rpmlint -v -i netspy2ban.spec 
netspy2ban.spec: I: checking
0 packages and 1 specfiles checked; 0 errors, 0 warnings.

Please let me know if I have missed something

Comment 2 Christopher Meng 2015-07-29 09:27:15 UTC
Fedora Account System Username?

Comment 4 Jens Lody 2015-07-29 09:55:26 UTC
This is a snippet of fedora-review running on your source-rpm with rawhide as target configuration:

Issues:
=======
- Sources used to build the package match the upstream source, as provided
  in the spec URL.
  Note: Upstream MD5sum check error, diff is in
  /home/jens/netspy2ban/diff.txt
  See: http://fedoraproject.org/wiki/Packaging/SourceURL

[...]

Rpmlint
-------
Checking: netspy2ban-1.0-1.fc24.noarch.rpm
          netspy2ban-1.0-1.fc24.src.rpm
netspy2ban.noarch: W: spurious-executable-perm /usr/share/man/man1/netspy2ban.1.gz
netspy2ban.noarch: W: no-manual-page-for-binary netspy2start
netspy2ban.noarch: W: dangerous-command-in-%postun rm
2 packages and 0 specfiles checked; 0 errors, 3 warnings.




Rpmlint (installed packages)
----------------------------
netspy2ban.noarch: W: spurious-executable-perm /usr/share/man/man1/netspy2ban.1.gz
netspy2ban.noarch: W: no-manual-page-for-binary netspy2start
netspy2ban.noarch: W: dangerous-command-in-%postun rm
1 packages and 0 specfiles checked; 0 errors, 3 warnings.



Are your links uptodate ?

Comment 5 Fotios Tsiadimos 2015-07-29 14:57:35 UTC
Hello Jens,

Thank you for your feedback. I replace the link and now this issue is fixed, please let me know if I need to fix something else 


Source checksums
----------------
https://raw.githubusercontent.com/ftsiadimos/netspy2ban/master/rpms/netspy2ban-1.0.tar.gz :
  CHECKSUM(SHA256) this package     : f61a1ed73374b0def1302097ce9a51187faee5d549fd057a86d04b25e97875c4
  CHECKSUM(SHA256) upstream package : f61a1ed73374b0def1302097ce9a51187faee5d549fd057a86d04b25e97875c4

Comment 6 Jens Lody 2015-08-03 22:54:37 UTC
Informal review:

I do not use python myself, so I'm not familiar to it.

Did you read:
https://fedoraproject.org/wiki/Packaging:Python ?
If I understand correctly you should not use the generic ${python} (etc.) macros, but the specific ${python2} (etc.) version, to be sure the package works, if the default switches from python2 to 3.

Besides the python-specific stuff:
Don't use rm -rf %{buildroot} at the beginning of %install
Install the man-page with "-m 644" parameter to avoid setting the executable-bit
Remove the %clean-section
defattr in %files only needed for rpm < 4.4 (even RedHat/CentOS 5 has 4.4.x)

Your license-file is unclear about the correct license, it mentions GPLv2+ and GPL3+, the spec-file says You use GPLv2+.

In the sources you only write about GPL without any version.

Please check: https://fedoraproject.org/wiki/Packaging:LicensingGuidelines and https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing

Please test your package also with fedora-review -b 1245255 -m fedora-rawhide-x86_64 (see the man-page of fedora-review for the -m parameters if you want to test different mock-configurations).

This is just an informal review, I'm not a fedora-packager, "just" a contributor looking for a review (and a sponsor) myself.

Comment 7 FT 2015-08-08 17:05:31 UTC
Thank you a lot for your help!

I removed defattr, %clean-section, rm -rf %{buildroot}.

I fixed my GPL license to be GPLv3 and I added "-m 644" for my man page

In the requires I added the python2 but I will not change the macros to python2 because I want to keep the noarch flag. The python2 macro is not working with the noarch flag.

Comment 8 William Moreno 2016-05-06 15:48:05 UTC
Some issues I found:

1. Do not autostart and autoenble the fail2ban service, we can not do this:

%post
systemctl start fail2ban
systemctl enable fail2ban

See:
https://fedoraproject.org/wiki/Packaging:Systemd#Why_don.27t_we....

2. Missing appdata.xml info, see: https://fedoraproject.org/wiki/Packaging:AppData

3.I see you are the upstream developer, can you provide a setup.py file? This way the spec will be cleaner and you can simple use the py2_build and py2_install macros.

4. Your rpm ship a icon you MUST include:

%post
/bin/touch --no-create %{_datadir}/icons/hicolor &>/dev/null || :

%postun
if [ $1 -eq 0 ] ; then
    /bin/touch --no-create %{_datadir}/icons/hicolor &>/dev/null
    /usr/bin/gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || :
fi

%posttrans
/usr/bin/gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || :

5. You need to include this to handle the unit file:

%{?systemd_requires}
BuildRequires: systemd

[...]
%post
%systemd_post apache-httpd.service

%preun
%systemd_preun apache-httpd.service

%postun
%systemd_postun_with_restart apache-httpd.service


6. Include a %check seption, in this section you should validate the desktop file and the appdata.xml file.

7. Do not gzip the man page, rpm will gzip it for you, see
https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Manpages

This is a informal review, I see than you need a sponsor I can take your review request if you agree to do some informals review.

OK: The package must be named according to the Package Naming Guidelines .
OK: Package must be licensed with a Fedora approved license.
OK: The License field in the spec must match the actual license.
OK: License(s) for the package must be included in %license.
OK: Packages must NOT bundle copies of system libraries.
OK: The package must contain code, or permissible content.
OK: Large documentation files must go in a -doc subpackage.
OK: If a package includes something as %doc, it must not affect the runtime.
OK: Packages must NOT contain any .la libtool archives.
OK: Packages containing GUI applications must include a %{name}.desktop

Comment 9 William Moreno 2016-05-06 15:55:15 UTC
Also please consider suporting python3 and package the app with python3 so Fedora will use python3 by default and there will no a python2 enviroment by default.

Comment 10 Fotios Tsiadimos 2016-05-10 02:09:48 UTC
Hi William:

I'll fix the issues you mentioned above. And definitely - please review my program and I can also review someone's program informally. Thanks for your help.

Fotis

Comment 11 William Moreno 2017-12-16 04:28:31 UTC
Any update here?

Comment 12 William Moreno 2018-03-16 16:50:42 UTC
3 months without answer, closing ad DEADREVIEW


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