Bug 710905 (oct-optim) - Review Request: octave-optim - Optimization for Octave
Summary: Review Request: octave-optim - Optimization for Octave
Keywords:
Status: CLOSED ERRATA
Alias: oct-optim
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Susi Lehtola
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: oct-signal
TreeView+ depends on / blocked
 
Reported: 2011-06-05 16:23 UTC by Thomas Sailer
Modified: 2011-09-10 00:30 UTC (History)
4 users (show)

Fixed In Version: octave-optim-1.0.16-3.fc15
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-09-09 17:14:18 UTC
Type: ---
Embargoed:
susi.lehtola: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

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.


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