Bug 847952 - Review Request: octave-nnet - A feed forward multi-layer neural network
Review Request: octave-nnet - A feed forward multi-layer neural network
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Peter Lemenkov
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-14 03:56 EDT by Ankur Sinha (FranciscoD)
Modified: 2012-08-30 21:00 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-08-30 21:00:40 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
lemenkov: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Ankur Sinha (FranciscoD) 2012-08-14 03:56:49 EDT
Spec URL: http://ankursinha.fedorapeople.org/octave-nnet/octave-nnet.spec
SRPM URL: http://ankursinha.fedorapeople.org/octave-nnet/octave-nnet-0.1.13-1.fc17.src.rpm

Description: 
A neural network package for Octave! Goal is to be as compatible as possible to
the one of MATLAB(TM)

Fedora Account System Username: ankursinha

It's my first octave package. It looks like it installs fine (I checked pkg list and it showed up). It may need some work though.
Comment 1 Peter Lemenkov 2012-08-14 04:09:38 EDT
I'll review it.
Comment 2 Peter Lemenkov 2012-08-14 05:26:40 EDT
Ok, here is a successful build for Rawhide:

* http://koji.fedoraproject.org/koji/taskinfo?taskID=4387581

These are Octave Packaging Guidelines:

* https://fedoraproject.org/wiki/Packaging:Octave

And here is my formal 
 
REVIEW:

Legend: + = PASSED, - = FAILED, 0 = Not Applicable

- rpmlint is noisy:

work ~/Desktop: rpmlint octave-nnet-0.1.13-1.fc19.*
octave-nnet.noarch: W: spelling-error Summary(en_US) multi -> mulch, mufti

^^^ false positive.
octave-nnet.noarch: W: obsolete-not-provided octave-forge

^^^ I think it's not required in this case. if someone still requires octave-forge it must be fixed. Also this is exactly how it's done in the Octave Packaging Guildelines.

octave-nnet.noarch: W: hidden-file-or-dir /usr/share/octave/packages/nnet-0.1.13/packinfo/.autoload
octave-nnet.noarch: E: zero-length /usr/share/octave/packages/nnet-0.1.13/packinfo/.autoload

^^^ I'm not familiar with Octave - could you please comment this.

octave-nnet.noarch: W: doc-file-dependency /usr/share/octave/packages/nnet-0.1.13/doc/latex/perl/analyzeOctaveSource.pm perl(File::Find)

^^^ Likewise. Please, comment this.

octave-nnet.noarch: W: dangerous-command-in-%preun rm

^^^ Invoked by Octave-related macro so I think it's ok.

octave-nnet.src: W: spelling-error Summary(en_US) multi -> mulch, mufti

^^^ false positive.

octave-nnet.src: W: strange-permission nnet-0.1.13.tar.gz 0640L

^^^ Strange, indeed, but harmless.

2 packages and 0 specfiles checked; 1 errors, 7 warnings.
work ~/Desktop:

+ The package is named according to the  Package Naming Guidelines.
+ The spec file name matches the base package %{name}, in the format %{name}.spec.
+ The package meets the Packaging Guidelines and Octave Packaging Guidelines (see link above).
+ The package is licensed with a Fedora approved license and meets the Licensing Guidelines.
+ The License field in the package spec file matches the actual license (GPLv2 or later, as stated in the DESCRIPTION file).

- The file, containing the text of the license(s) for the package, MUST be included in %doc. It currently installed into %{octpkgdir} so technically it's available within package but this is a MUST requirement so please mark COPYING as %doc in the %files section.

+ The spec file is written in American English.
+ The spec file for the package is legible.
+ The sources used to build the package, match the upstream source, as provided in the spec URL.

sulaco ~/rpmbuild/SOURCES: sha256sum nnet-0.1.13.tar.gz*
e1388ea8f56bad0c609e879f2e19432d117111487cc6980ae78bb69cef48e41d  nnet-0.1.13.tar.gz
e1388ea8f56bad0c609e879f2e19432d117111487cc6980ae78bb69cef48e41d  nnet-0.1.13.tar.gz.1
sulaco ~/rpmbuild/SOURCES: 

+ The package successfully compiles and builds into binary rpms on at least one primary architecture. See koji link above.
+ All build dependencies are listed in BuildRequires.
0 No need to handle locales.
0 No shared library files.
+ The package does NOT bundle copies of system libraries.
+ The package is not designed to be relocatable.
+ The package owns all directories that it creates.
+ The package does not list a file more than once in the spec file's %files listings.
+ Permissions on files are set properly.
+ The package consistently uses macros.
+ The package contains code, or permissible content.
0 No extremely large documentation files.
+ Anything, the package includes as %doc, does not affect the runtime of the application.
0 No header files.
0 No static libraries.
0 No pkgconfig(.pc) files.
0 The package doesn't contain library files with a suffix (e.g. libfoo.so.1.1).
0 No devel sub-package.
+ The package does NOT contain any .la libtool archives.
0 Not a GUI application.
+ The package does not own files or directories already owned by other packages.
+ At the beginning of %install, the package runs rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
+ All filenames in rpm packages are valid UTF-8.


Please, comment my concerns above and mark COPYING ad %doc and I'll finish it.
Comment 3 Ankur Sinha (FranciscoD) 2012-08-14 06:05:17 EDT
Hi Peter,

1. COPYING is included in the package, I think by the octave macros:
[ankur@ankur SRPMS]$ rpm -qpl /var/lib/mock/fedora-rawhide-x86_64/result/octave-nnet-0.1.13-2.fc19.noarch.rpm | egrep COPY
/usr/share/octave/packages/nnet-0.1.13/packinfo/COPYING

2. The .autoload file looks like it's included. I looked at other octave modules already in fedora and they all seem to include it:
[root@ankur ~]# repoquery -l octave-* | egrep autoload
/usr/share/octave/packages/audio-1.1.4/packinfo/.autoload
/usr/share/octave/packages/communications-1.1.0/packinfo/.autoload
/usr/share/octave/packages/control-2.2.4/packinfo/.autoload
/usr/share/octave/packages/gsl-1.0.8/packinfo/.autoload
/usr/share/octave/packages/image-1.0.15/packinfo/.autoload
/usr/share/octave/packages/miscellaneous-1.0.11/packinfo/.autoload
/usr/share/octave/packages/octcdf-1.1.4/packinfo/.autoload
/usr/share/octave/packages/optim-1.0.17/packinfo/.autoload
/usr/share/octave/packages/quaternion-1.0.0/packinfo/.autoload
/usr/share/octave/packages/signal-1.1.2/packinfo/.autoload
/usr/share/octave/packages/struct-1.0.9/packinfo/.autoload
/usr/share/octave/packages/symbolic-1.1.0/packinfo/.autoload
[root@ankur ~]#

3. I've removed the perl file. It looks like it's something upstream uses to generate TeX sources and isn't required for the fedora package.

4. I corrected the source tar's permissions also to get rid of the rpmlint warning:
[ankur@ankur SRPMS]$ rpmlint octave-nnet-0.1.13-2.fc17.src.rpm /var/lib/mock/fedora-rawhide-x86_64/result/*.rpm ../SPECS/octave-nnet.spec
octave-nnet.src: W: spelling-error Summary(en_US) multi -> mulch, mufti
octave-nnet.noarch: W: spelling-error Summary(en_US) multi -> mulch, mufti
octave-nnet.noarch: W: obsolete-not-provided octave-forge
octave-nnet.noarch: W: hidden-file-or-dir /usr/share/octave/packages/nnet-0.1.13/packinfo/.autoload
octave-nnet.noarch: E: zero-length /usr/share/octave/packages/nnet-0.1.13/packinfo/.autoload
octave-nnet.noarch: W: dangerous-command-in-%preun rm
octave-nnet.src: W: spelling-error Summary(en_US) multi -> mulch, mufti
3 packages and 1 specfiles checked; 1 errors, 6 warnings.
[ankur@ankur SRPMS]$

Thanks,
Ankur
Comment 4 Peter Lemenkov 2012-08-14 06:15:10 EDT
(In reply to comment #3)
> Hi Peter,
> 
> 1. COPYING is included in the package, I think by the octave macros:
> [ankur@ankur SRPMS]$ rpm -qpl
> /var/lib/mock/fedora-rawhide-x86_64/result/octave-nnet-0.1.13-2.fc19.noarch.
> rpm | egrep COPY
> /usr/share/octave/packages/nnet-0.1.13/packinfo/COPYING

I am aware of it but this is a "MUST" rule - so please add another one copy as %doc.

* https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text

"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 must be included in %doc."

> 2. The .autoload file looks like it's included. I looked at other octave
> modules already in fedora and they all seem to include it:
> [root@ankur ~]# repoquery -l octave-* | egrep autoload
> /usr/share/octave/packages/audio-1.1.4/packinfo/.autoload
> /usr/share/octave/packages/communications-1.1.0/packinfo/.autoload
> /usr/share/octave/packages/control-2.2.4/packinfo/.autoload
> /usr/share/octave/packages/gsl-1.0.8/packinfo/.autoload
> /usr/share/octave/packages/image-1.0.15/packinfo/.autoload
> /usr/share/octave/packages/miscellaneous-1.0.11/packinfo/.autoload
> /usr/share/octave/packages/octcdf-1.1.4/packinfo/.autoload
> /usr/share/octave/packages/optim-1.0.17/packinfo/.autoload
> /usr/share/octave/packages/quaternion-1.0.0/packinfo/.autoload
> /usr/share/octave/packages/signal-1.1.2/packinfo/.autoload
> /usr/share/octave/packages/struct-1.0.9/packinfo/.autoload
> /usr/share/octave/packages/symbolic-1.1.0/packinfo/.autoload
> [root@ankur ~]#

Ok, good then.

> 3. I've removed the perl file. It looks like it's something upstream uses to
> generate TeX sources and isn't required for the fedora package.

Ok.

> 4. I corrected the source tar's permissions also to get rid of the rpmlint
> warning:
> [ankur@ankur SRPMS]$ rpmlint octave-nnet-0.1.13-2.fc17.src.rpm
> /var/lib/mock/fedora-rawhide-x86_64/result/*.rpm ../SPECS/octave-nnet.spec
> octave-nnet.src: W: spelling-error Summary(en_US) multi -> mulch, mufti
> octave-nnet.noarch: W: spelling-error Summary(en_US) multi -> mulch, mufti
> octave-nnet.noarch: W: obsolete-not-provided octave-forge
> octave-nnet.noarch: W: hidden-file-or-dir
> /usr/share/octave/packages/nnet-0.1.13/packinfo/.autoload
> octave-nnet.noarch: E: zero-length
> /usr/share/octave/packages/nnet-0.1.13/packinfo/.autoload
> octave-nnet.noarch: W: dangerous-command-in-%preun rm
> octave-nnet.src: W: spelling-error Summary(en_US) multi -> mulch, mufti
> 3 packages and 1 specfiles checked; 1 errors, 6 warnings.
> [ankur@ankur SRPMS]$

Ok. good. I don't see any other issues (assuming that you will fix licensing issue shortly) so this package is


APPROVED.



PS. it would be great if you will review this one in return:

* https://bugzilla.redhat.com/show_bug.cgi?id=652682

It's rather messy and probably has a lot of room for improvement - sorry for that.
Comment 5 Ankur Sinha (FranciscoD) 2012-08-14 06:26:10 EDT
Added the COPYING file:

http://ankursinha.fedorapeople.org/octave-nnet/octave-nnet.spec

http://ankursinha.fedorapeople.org/octave-nnet/octave-nnet-0.1.13-3.fc17.src.rpm

Thanks for the review Peter. I've taken up 652682.

Ankur
Comment 6 Ankur Sinha (FranciscoD) 2012-08-14 06:29:11 EDT
New Package SCM Request
=======================
Package Name: octave-nnet
Short Description: A feed forward multi-layer neural network
Owners: ankursinha
Branches: f17 f18
InitialCC:
Comment 7 Gwyn Ciesla 2012-08-14 07:00:06 EDT
Git done (by process-git-requests).
Comment 8 Fedora Update System 2012-08-14 08:09:27 EDT
octave-nnet-0.1.13-3.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/octave-nnet-0.1.13-3.fc17
Comment 9 Fedora Update System 2012-08-14 17:59:37 EDT
Package octave-nnet-0.1.13-3.fc17:
* should fix your issue,
* was pushed to the Fedora 17 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing octave-nnet-0.1.13-3.fc17'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2012-11887/octave-nnet-0.1.13-3.fc17
then log in and leave karma (feedback).
Comment 10 Fedora Update System 2012-08-30 21:00:40 EDT
octave-nnet-0.1.13-3.fc17 has been pushed to the Fedora 17 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.