Bug 710905 (oct-optim)

Summary: Review Request: octave-optim - Optimization for Octave
Product: [Fedora] Fedora Reporter: Thomas Sailer <fedora>
Component: Package ReviewAssignee: Susi Lehtola <susi.lehtola>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, jamatos, notting, susi.lehtola
Target Milestone: ---Flags: susi.lehtola: fedora-review+
gwync: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: octave-optim-1.0.16-3.fc15 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2011-09-09 17:14:18 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 710906    

Description Thomas Sailer 2011-06-05 16:23:39 UTC
Spec URL: http://sailer.fedorapeople.org/octave-optim.spec
SRPM URL: http://sailer.fedorapeople.org/octave-optim-1.0.16-1.fc15.src.rpm
Description: Non-linear optimization toolkit.

Comment 1 José Matos 2011-06-13 15:24:23 UTC
I will take the review.

Comment 3 Susi Lehtola 2011-08-12 17:45:55 UTC
Ping José? Are you going to do the review or not?

Comment 4 Susi Lehtola 2011-08-16 07:04:31 UTC
Looks like José is MIA. Taking over review.

Comment 5 Susi Lehtola 2011-08-16 07:30:09 UTC
rpmlint output:
octave-optim.src:11: W: macro-in-comment %40unreal
octave-optim.x86_64: W: obsolete-not-provided octave-forge
octave-optim.x86_64: W: hidden-file-or-dir /usr/share/octave/packages/optim-1.0.16/packinfo/.autoload
octave-optim.x86_64: E: zero-length /usr/share/octave/packages/optim-1.0.16/packinfo/.autoload
octave-optim.x86_64: W: dangerous-command-in-%preun cp
3 packages and 0 specfiles checked; 1 errors, 4 warnings.

These are expected.

**

You are using the noarch template for arch dependent packages. This is why I ran recently into trouble with two of your existing packages. Replace
 Requires:       octave
with
 Requires:       octave(api) = %{octave_api}

**

The command

for i in %{octpkgdir}/doc/figures/2D_slice-3.eps2 %{octpkgdir}/doc/figures/optim_tutorial_slice.eps; do
  iconv -f iso8859-1 -t utf-8 %{buildroot}/$i > %{buildroot}/$i.conv && mv -f %{buildroot}/$i.conv %{buildroot}/$i
done;

looks *really* odd. Did you get an rpmlint warning about text in the wrong encoding? I'm not a PostScript expert, but I'd leave these files as-is.

**

Your summary is sloppy. Please change it to something of the lines of "A non-linear optimization tool kit for Octave".

Also the description could be better, along the lines of

This package contains a non-linear optimization tool kit for Octave, containing functions for curve fitting and the following minimization algorithms:
* Nead-Miller simplex
* Conjugate Gradients
* Memory limited BFGS
* Simulated Annealing

**

MUST: The package does not yet exist in Fedora. The Review Request is not a duplicate. OK
MUST: The spec file for the package is legible and macros are used consistently. OK
MUST: The package must be named according to the Package Naming Guidelines. OK
MUST: The spec file name must match the base package %{name}. OK
MUST: The package must be licensed with a Fedora approved license and meet the  Licensing Guidelines. OK

MUST: The License field in the package spec file must match the actual license. NEEDSWORK
- As far as I can see, this is plain GPLv2+.

MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. OK
$ md5sum optim-1.0.16.tar.gz ../SOURCES/optim-1.0.16.tar.gz 
a0ed1c8bbd7d9ddafc6f5fab08aba1c5  optim-1.0.16.tar.gz
a0ed1c8bbd7d9ddafc6f5fab08aba1c5  ../SOURCES/optim-1.0.16.tar.gz

MUST: The package MUST successfully compile and build into binary rpms. OK
MUST: The spec file MUST handle locales properly. N/A

MUST: Optflags are used and time stamps preserved. ??
- The optimization flags aren't shown in the build process. Everything seems to be alright, though.

MUST: Packages containing shared library files must call ldconfig. N/A
MUST: A package must own all directories that it creates or require the package that owns the directory. OK
MUST: Files only listed once in %files listings. OK
MUST: Debuginfo package is complete. OK
MUST: Permissions on files must be set properly. OK
MUST: Large documentation files must go in a -doc subpackage. N/A
MUST: All relevant items are included in %doc. Items in %doc do not affect runtime of application. OK
MUST: Header files must be in a -devel package. N/A
MUST: Static libraries must be in a -static package. N/A
MUST: If a package contains library files with a suffix then library files ending in .so must go in a -devel package. N/A
MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned, architecture dependent dependency. N/A
MUST: Packages does not contain any .la libtool archives. N/A
MUST: Desktop files are installed properly. N/A
MUST: No file conflicts with other packages and no general names. OK
SHOULD: %{?dist} tag is used in release. OK
SHOULD: If the package does not include license text(s) as separate files from upstream, the packager should query upstream to include it. OK
SHOULD: The package builds in mock. OK
EPEL: Clean section exists. OK
EPEL: Buildroot cleaned before install. OK
EPEL: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'. N/A

Comment 6 Susi Lehtola 2011-08-16 07:46:45 UTC
Please add a '-v' flag in the Makefile, so that the commands used in the build are displayed.

I also wouldn't mind a comment about Patch0, e.g. "Fix the address of FSF."

Comment 7 Susi Lehtola 2011-08-16 07:51:59 UTC
And why are you using

%octave_cmd pkg build '-verbose' '-nodeps' %{_tmppath}/%{name}-%{version}-%{release}.%{_arch} %{_builddir}/%{buildsubdir}

instead of

%octave_pkg_build ?

Comment 8 Thomas Sailer 2011-08-24 16:46:50 UTC
(In reply to comment #5)

Thanks for taking the review!

http://sailer.fedorapeople.org/octave-optim-1.0.16-2.fc15.src.rpm

I've appled most of your suggestions, with the exception of the following:

> looks *really* odd. Did you get an rpmlint warning about text in the wrong
> encoding? I'm not a PostScript expert, but I'd leave these files as-is.

You are right, it is wrong.

But now I get the following lint warnings:

octave-optim.x86_64: W: file-not-utf8 /usr/share/octave/packages/optim-1.0.16/doc/figures/2D_slice-3.eps2
octave-optim.x86_64: W: file-not-utf8 /usr/share/octave/packages/optim-1.0.16/doc/figures/optim_tutorial_slice.eps

I think these warnings are bogus, they are coming from a binary portion of the eps file.

> MUST: The License field in the package spec file must match the actual license.
> NEEDSWORK

Well, the DESCRIPTION file contained in the source archive lists GFDL as well as GPLv2+, so that's why it is listed in the RPM license field as well.

> Please add a '-v' flag in the Makefile, so that the commands used in the build
> are displayed.

Sorry where do you want -v exactly?

> And why are you using
>
> %octave_cmd pkg build '-verbose' '-nodeps'
> %{_tmppath}/%{name}-%{version}-%{release}.%{_arch} %{_builddir}/%{buildsubdir}
>
> instead of
>
> %octave_pkg_build ?

If I use octave_pkg_build, I get a build error:
error: the following dependencies where unsatisfied:
   optim needs miscellaneous >= 1.0.11
 optim needs struct >= 1.0.9

even though octave-miscellaneous and octave-struct are installed. Seems there is a problem with the dependency detection.

Comment 9 Susi Lehtola 2011-08-24 17:14:19 UTC
(In reply to comment #8)
> > looks *really* odd. Did you get an rpmlint warning about text in the wrong
> > encoding? I'm not a PostScript expert, but I'd leave these files as-is.
> 
> You are right, it is wrong.
> 
> But now I get the following lint warnings:
> 
> octave-optim.x86_64: W: file-not-utf8
> /usr/share/octave/packages/optim-1.0.16/doc/figures/2D_slice-3.eps2
> octave-optim.x86_64: W: file-not-utf8
> /usr/share/octave/packages/optim-1.0.16/doc/figures/optim_tutorial_slice.eps
> 
> I think these warnings are bogus, they are coming from a binary portion of the
> eps file.

Yes. rpmlint often causes false alarms.

> > MUST: The License field in the package spec file must match the actual license.
> > NEEDSWORK
> 
> Well, the DESCRIPTION file contained in the source archive lists GFDL as well
> as GPLv2+, so that's why it is listed in the RPM license field as well.

OK. Now I see that optim-mini-howto-2.lyx specifies that it is under the GFDL. But I also now notice that many files that were not examined by licensecheck specify GPLv3+. So the correct tag is
 License: GPLv2+ and GPLv3+ and GFDL.

I don't see the point in shipping a LyX file (or the Makefile!), so you should compile the documentation by adding
 BuildRequires: tex(latex)
and running
 make -C doc DVIPDF=dvipdf optim-mini-howto-2.pdf
in %build.

Then, at the end of %install run
 rm -rf  %{octpkgdir}/doc
to get rid of the unnecessary stuff, and add

%doc doc/optim-mini-howto-2.pdf doc/development/interfaces.txt

> > Please add a '-v' flag in the Makefile, so that the commands used in the build
> > are displayed.
> 
> Sorry where do you want -v exactly?

Even though the make process clearly compiles something, the compiler flags are not shown. According to the man page, you should add -v flag to the mkoctfile commands:

       -v, --verbose
               Echo commands as they are executed.

> If I use octave_pkg_build, I get a build error:
> error: the following dependencies where unsatisfied:
>    optim needs miscellaneous >= 1.0.11
>  optim needs struct >= 1.0.9
> 
> even though octave-miscellaneous and octave-struct are installed. Seems there
> is a problem with the dependency detection.

Then please file a bug against octave. Maybe the -nodeps flag needs to be added to the octave_pkg_build macro.

Comment 10 Thomas Sailer 2011-08-26 09:47:21 UTC
http://sailer.fedorapeople.org/octave-optim-1.0.16-3.fc15.src.rpm

Shipping the docs as pdf (besides actually making sense) also gets rid of the eps rpmlint warnings

Comment 11 Susi Lehtola 2011-08-26 12:34:18 UTC
Build flags are now shown, and they are correct. Other issues have been adressed as well, so this package is 

APPROVED

Since the license is a bit complicated, it wouldn't hurt having a comment such as
#C++ files are GPLv2+, .m files are GPLv2+ and GPLv3+, documentation is GFDL

Before you build, please make sure that octave-3.4.2-2.fc1{5,6} is present in the buildroot. The update is still in updates-testing, and for some reason the override drops out every once in a while.

Comment 12 Thomas Sailer 2011-08-26 14:04:54 UTC
Thank you!

New Package SCM Request
=======================
Package Name: octave-optim
Short Description: A non-linear optimization tool kit for Octave
Owners: sailer
Branches: f15 f16
InitialCC:

Comment 13 Gwyn Ciesla 2011-08-26 14:25:20 UTC
Git done (by process-git-requests).

Comment 14 Fedora Update System 2011-09-01 08:35:45 UTC
octave-optim-1.0.16-3.fc16.1 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/octave-optim-1.0.16-3.fc16.1

Comment 15 Fedora Update System 2011-09-01 08:35:54 UTC
octave-optim-1.0.16-3.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/octave-optim-1.0.16-3.fc15

Comment 16 Fedora Update System 2011-09-01 19:02:07 UTC
Package octave-optim-1.0.16-3.fc16.1:
* should fix your issue,
* was pushed to the Fedora 16 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing octave-optim-1.0.16-3.fc16.1'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/octave-optim-1.0.16-3.fc16.1
then log in and leave karma (feedback).

Comment 17 Fedora Update System 2011-09-09 17:14:12 UTC
octave-optim-1.0.16-3.fc16.1 has been pushed to the Fedora 16 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 18 Fedora Update System 2011-09-10 00:30:22 UTC
octave-optim-1.0.16-3.fc15 has been pushed to the Fedora 15 stable repository.  If problems still persist, please make note of it in this bug report.