Bug 663181 - Review Request: ompc - MATLAB to Python syntax adapting compiler
Review Request: ompc - MATLAB to Python syntax adapting compiler
Status: ASSIGNED
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
Unspecified Unspecified
low Severity medium
: ---
: ---
Assigned To: Susi Lehtola
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2010-12-14 16:42 EST by Alon Levy
Modified: 2014-12-22 18:54 EST (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed:
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
susi.lehtola: fedora‑review?


Attachments (Terms of Use)

  None (edit)
Description Alon Levy 2010-12-14 16:42:45 EST
Spec URL: http://people.freedesktop.org/~alon/ompc.spec
SRPM URL: http://people.freedesktop.org/~alon/ompc-1.0beta-1.96c520b01abc.src.rpm
Description:
OMPC (http://ompc.juricap.com) aims to allow effortless reuse of MATLAB code 
from Python. OMPC is a code adaptation layer that translates MATLAB's m-files
into Python compatible syntax. The generated Python compatible code 
depends on OMPClib which provides MATLAB compatible array interface for Python.
The OMPClib interface is another numerical Python library but the only one with 
indexing based at 1 instead at 0, strict FORTRAN binary interface and 
copy on assignment.
Comment 1 Susi Lehtola 2010-12-15 03:14:13 EST
rpmlint output:
ompc.src: E: invalid-version 1.0beta
ompc.src: W: summary-ended-with-dot C MATLAB to Python syntax adapting compiler.
ompc.src: W: strange-permission ompc_gen_tarball.sh 0775L
ompc.src: W: patch-not-applied Patch1: ompc-1.0beta-fix_compilation_for_ompc_supported.patch
ompc.src: W: patch-not-applied Patch2: ompc-1.0beta-remove_ompc_pth_from_setup.patch
ompc.src: W: invalid-url Source0: ompc-1.0beta.tar.bz2
ompc.x86_64: E: invalid-version 1.0beta
ompc.x86_64: W: summary-ended-with-dot C MATLAB to Python syntax adapting compiler.
ompc.x86_64: E: no-binary
ompc.x86_64: W: no-documentation
ompc.x86_64: W: devel-file-in-non-devel-package /usr/lib/python2.7/site-packages/ompceg/test.c
ompc.x86_64: W: devel-file-in-non-devel-package /usr/lib/python2.7/site-packages/ompceg/test.h
ompc-debuginfo.x86_64: E: invalid-version 1.0beta
ompc-debuginfo.x86_64: E: empty-debuginfo-package
3 packages and 0 specfiles checked; 5 errors, 9 warnings.

Please drop the unnecessary commented lines that remain from the python spec file template.

Tag the package as BuildArch: noarch, as it is architecture independent.

Drop the patches if you don't need them.

Get rid of the test files.

You will need to stick to
 http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Package_Versioning
Having the git snapshot identifier in the release is IMHO quite pointless. I'd use the date, which is a lot more understandable.

Abusive use of wildcards can lead to trouble. Avoid their use, unless absolutely necessary. For instance this package has a clearly broken install. The contents of %{python_sitelib} should be

%{python_sitelib}/ompc/
%{python_sitelib}/OMPC-*.egg-info

Currently there's a big bunch of additional stuff that in no case belongs there, such as

/usr/lib/python2.7/site-packages/test.py
/usr/lib/python2.7/site-packages/test.pyc
/usr/lib/python2.7/site-packages/test.pyo
/usr/lib/python2.7/site-packages/LICENSE
/usr/lib/python2.7/site-packages/OMPC-1.0_beta-py2.7.egg-info
/usr/lib/python2.7/site-packages/README
/usr/lib/python2.7/site-packages/licenses
/usr/lib/python2.7/site-packages/licenses/ply
/usr/lib/python2.7/site-packages/licenses/ply/ANNOUNCE
/usr/lib/python2.7/site-packages/licenses/ply/CHANGES
/usr/lib/python2.7/site-packages/licenses/ply/COPYING
/usr/lib/python2.7/site-packages/licenses/ply/README
/usr/lib/python2.7/site-packages/licenses/ply/TODO
/usr/lib/python2.7/site-packages/ompc.cfg
/usr/lib/python2.7/site-packages/ompceg
/usr/lib/python2.7/site-packages/ompceg/ompceg.py
/usr/lib/python2.7/site-packages/ompceg/ompceg.pyc
/usr/lib/python2.7/site-packages/ompceg/ompceg.pyo
/usr/lib/python2.7/site-packages/ompceg/test.c
/usr/lib/python2.7/site-packages/ompceg/test.h
/usr/lib/python2.7/site-packages/ompclib
/usr/lib/python2.7/site-packages/ompclib/__init__.py
/usr/lib/python2.7/site-packages/ompclib/__init__.pyc
/usr/lib/python2.7/site-packages/ompclib/__init__.pyo
/usr/lib/python2.7/site-packages/ompclib/byteplay.py
/usr/lib/python2.7/site-packages/ompclib/byteplay.pyc
/usr/lib/python2.7/site-packages/ompclib/byteplay.pyo
/usr/lib/python2.7/site-packages/ompclib/m_compile.py
/usr/lib/python2.7/site-packages/ompclib/m_compile.pyc
/usr/lib/python2.7/site-packages/ompclib/m_compile.pyo
/usr/lib/python2.7/site-packages/ompclib/matpy.py
/usr/lib/python2.7/site-packages/ompclib/matpy.pyc
/usr/lib/python2.7/site-packages/ompclib/matpy.pyo
/usr/lib/python2.7/site-packages/ompclib/matpy_gnuplot.py
/usr/lib/python2.7/site-packages/ompclib/matpy_gnuplot.pyc
/usr/lib/python2.7/site-packages/ompclib/matpy_gnuplot.pyo
/usr/lib/python2.7/site-packages/ompclib/matpy_platform.py
/usr/lib/python2.7/site-packages/ompclib/matpy_platform.pyc
/usr/lib/python2.7/site-packages/ompclib/matpy_platform.pyo
/usr/lib/python2.7/site-packages/ompclib/ompc_narginout.py
/usr/lib/python2.7/site-packages/ompclib/ompc_narginout.pyc
/usr/lib/python2.7/site-packages/ompclib/ompc_narginout.pyo
/usr/lib/python2.7/site-packages/ompclib/ompc_supported.py
/usr/lib/python2.7/site-packages/ompclib/ompc_supported.pyc
/usr/lib/python2.7/site-packages/ompclib/ompc_supported.pyo
/usr/lib/python2.7/site-packages/ompclib/ompclib_numpy.py
/usr/lib/python2.7/site-packages/ompclib/ompclib_numpy.pyc
/usr/lib/python2.7/site-packages/ompclib/ompclib_numpy.pyo
/usr/lib/python2.7/site-packages/ompclib/ompclib_numpy_base.py
/usr/lib/python2.7/site-packages/ompclib/ompclib_numpy_base.pyc
/usr/lib/python2.7/site-packages/ompclib/ompclib_numpy_base.pyo
/usr/lib/python2.7/site-packages/ompclib/ompclib_platform.py
/usr/lib/python2.7/site-packages/ompclib/ompclib_platform.pyc
/usr/lib/python2.7/site-packages/ompclib/ompclib_platform.pyo
Comment 2 Alon Levy 2010-12-15 04:51:17 EST
Everything makes sense, just regarding the patches, I don't get the rpmlint message - I use them in %prep, and they are needed, so why doesn't rpmlint notice this? seems like an rpmlint bug. Maybe the BuildArch: noarch will "fix" that as well. I'll try.

Alon
Comment 3 Alon Levy 2010-12-15 05:04:18 EST
ok - had a missing space in the %patch lines. (-P 0, not -P0)

regarding the files you are absolutely right, strangely it seems the regular "python setup.py install" doesn't lead to that situation - so I'll have to look into that. If you install using "python setup.py install" you get a single directory /usr/lib/python2.7/site-packages/OMPC-1.0_beta-py2.7.egg/

It looks like that happens because of the egg creation and extraction that isn't happening with the default template I copied from - I'll just add that to the spec file install or build scripts.
Comment 4 Susi Lehtola 2010-12-15 05:32:30 EST
By the way, usually one uses
 %patch0 -p1
 %patch1 -p1
 %patch2 -p1
instead of
 %patch -P0 -p1
 %patch -P1 -p1
 %patch -P2 -p1
which is more compact and IMHO also clearer.
Comment 5 Susi Lehtola 2011-08-17 07:19:31 EDT
Ping, what is the status of this bug?
Comment 6 Alon Levy 2011-08-17 13:52:11 EDT
My second attempt at a rpm package at the time, the real blocker was a problem with setup.py, I don't remember what it was unfortunately, so I'll try to give it another try, I still want to get it in.

Alon
Comment 7 Susi Lehtola 2012-09-26 06:55:38 EDT
Then please do so.
Comment 8 Alon Levy 2012-10-08 12:57:46 EDT
I've given it another shot.
 - couldn't fix the warning about permissions of the generation script, it is 0755 which from what I see in rpmlint should be allowed but it's only allowing 0644 and 0664 afaict (it's a warning anyway)
 - changed the version, it is as you suggested <date>hg (hg signifies that it is from a mercurial repository, and the script provides the source)
 - revision is 1
 - changed the %files to be specific about it's wildcards.

I've added some patches that are not review related but just seemed like a good idea (no more warnings when importing in ipython 0.13 and using python 2.6). Ompc is still not supporting a wide range of recent matlab syntax but I don't claim to know how to fix it, and I hope it is useful enough with the current ompc (which the original author hasn't updated for the last two years).

The updated spec file is at the same location, the updated src.rpm is:

http://people.freedesktop.org/~alon/ompc.spec
http://people.freedesktop.org/~alon/ompc-20100604hg-1.src.rpm

Alon
Comment 9 Susi Lehtola 2013-05-06 18:09:49 EDT
Whoops, this has dropped under my radar. Are you still interested in the package?
Comment 10 Susi Lehtola 2013-07-15 07:10:09 EDT
Ping Alon?
Comment 11 Alon Levy 2013-07-15 10:12:00 EDT
(In reply to Susi Lehtola from comment #10)
> Ping Alon?

I would like to complete it - I don't use ompc any more so not really urgent, and would be happy if someone else steps up but not very likely :)
Comment 12 Susi Lehtola 2013-07-16 15:37:34 EDT
First of all,

warning: bogus date in %changelog: Tue Dec 15 2010 Alon Levy <alevy@redhat.com> - 1.0beta-2.20100604hg
warning: bogus date in %changelog: Mon Dec 14 2010 Alon Levy <alevy@redhat.com> - 1.0beta-1.96c520b01abc

Fix these.

**

Secondly, the versioning does not correspond to the guidelines. Because there has been no real version released, please set Version to 0, and Release to 1.20100604hg . Also note that you're missing %{?dist}.

**

Sources should not have a %release versioning. Fedora releases are independent of the upstream version.

**

Your patches are missing comments. Where did they come from? What do they do? Have they been sent upstream?

**

Please remove the empty %doc line. Or actually the LICENSE file should be there in %doc.

**

License is BSD, but some files in the distribution are LGPL licensed while others lack license boilerplates altogether. Please contact upstream for clarification.

**

The statement
 %{python_sitelib}/ompc/*
 %{python_sitelib}/ompceg/*
 %{python_sitelib}/ompclib/*
leaves the directories %{python_sitelib}/ompc{,eg,lib} without ownership. Just delete the trailing wildcard, and the directories will be owned as well.

**

Once you have addressed these I will perform the review.
Comment 13 Susi Lehtola 2013-09-04 10:26:43 EDT
Ping Alon, any progress?
Comment 14 Alon Levy 2013-09-15 02:13:18 EDT
(In reply to Susi Lehtola from comment #12)
> First of all,
> 
> warning: bogus date in %changelog: Tue Dec 15 2010 Alon Levy
> <alevy@redhat.com> - 1.0beta-2.20100604hg
> warning: bogus date in %changelog: Mon Dec 14 2010 Alon Levy
> <alevy@redhat.com> - 1.0beta-1.96c520b01abc
> 
> Fix these.
> 

Fixed

> **
> 
> Secondly, the versioning does not correspond to the guidelines. Because
> there has been no real version released, please set Version to 0, and
> Release to 1.20100604hg . Also note that you're missing %{?dist}.
> 

Fixed

> **
> 
> Sources should not have a %release versioning. Fedora releases are
> independent of the upstream version.
> 

Since the release corresponds to a hg revision I've changed the Source0 name to include expeclitly a name consistent with this ompc-<date>hg.tar.bz2, and set Release to be <date>hg%{?dist}

> **
> 
> Your patches are missing comments. Where did they come from? What do they
> do? Have they been sent upstream?

Upstream is basically dead. This is another reason why I'm not sure the effort to finish packaging ompc is worth it. But I just attempted contact with the author, and the package does work, so maybe it is worth it after all (and there are quite a few people who move from matlab to pyt 

> 
> **
> 
> Please remove the empty %doc line. Or actually the LICENSE file should be
> there in %doc.
> 
> **
> 
> License is BSD, but some files in the distribution are LGPL licensed while
> others lack license boilerplates altogether. Please contact upstream for
> clarification.
> 
> **
> 
> The statement
>  %{python_sitelib}/ompc/*
>  %{python_sitelib}/ompceg/*
>  %{python_sitelib}/ompclib/*
> leaves the directories %{python_sitelib}/ompc{,eg,lib} without ownership.
> Just delete the trailing wildcard, and the directories will be owned as well.
> 
> **
> 
> Once you have addressed these I will perform the review.

New spec file:
http://gitorious.org/fedpkg-alon/ompc/source/02a82b63318bb339fe55a1bdf6dc50f234afd714:ompc.spec

New srpm:
http://gitorious.org/fedpkg-alon/ompc/raw/02a82b63318bb339fe55a1bdf6dc50f234afd714:ompc-0-1.20100604hg.fc20.src.rpm
Comment 15 Alon Levy 2013-09-15 02:15:50 EDT
Regarding the license: I haven't contacted upstream yet about it, I am waiting a response for the patches anyway, when that gets back I'll ask about the LGPL files in there (lex.py, yacc.py, ply, gplot) and GPL (examples/appengine). I'm not packaging examples/appengine right now, so I could just change the license to BSD & LGPL. (would that just be License: BSD & LGPL ?)
Comment 17 Susi Lehtola 2014-12-22 18:54:51 EST
Looks like this passed below my radar.

(In reply to Alon Levy from comment #15)
> Regarding the license: I haven't contacted upstream yet about it, I am
> waiting a response for the patches anyway, when that gets back I'll ask
> about the LGPL files in there (lex.py, yacc.py, ply, gplot) and GPL
> (examples/appengine). I'm not packaging examples/appengine right now, so I
> could just change the license to BSD & LGPL. (would that just be License:
> BSD & LGPL ?)

Yes, but you need to include the LGPL version as in the guidelines.

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