Bug 429882 (python-Levenshtein) - Review Request: python-Levenshtein - Levenshtein distance measurement library in C
Summary: Review Request: python-Levenshtein - Levenshtein distance measurement library...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: python-Levenshtein
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 430887 439140
TreeView+ depends on / blocked
 
Reported: 2008-01-23 16:28 UTC by Dwayne Bailey
Modified: 2008-11-26 06:20 UTC (History)
8 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-11-05 20:23:16 UTC
Type: ---
Embargoed:
j: fedora-review+
a.badger: fedora-cvs+


Attachments (Terms of Use)

Description Dwayne Bailey 2008-01-23 16:28:26 UTC
Spec URL: http://translate.sourceforge.net/releases/testing/fedora/python-Levenshtein.spec
SRPM URL: http://translate.sourceforge.net/releases/testing/fedora/python-Levenshtein-0.10.1-2.fc8.src.rpm
Description: This package provides a python C extension that computes Levenshtein distance measurements for comparing differences between strings.

This is my first package that I've created from scratch. So I will need a sponsor.

Comment 1 Krzysztof Kurzawski 2008-01-25 15:35:00 UTC
I can't approve your package, but I'd like to help You.

I wanted to build this SRPM, but I couldn't.
http://kurzawa.nonlogic.org/logs/python-Leveshtein-build.log

I think that genextdoc.py.patch shouldn't be in Patch0, but in Source1.

Rpmlint warns:
python-Levenshtein.src: W: summary-ended-with-dot Python extension computing
string distances and similarities.
python-Levenshtein.src: W: spelling-error-in-description becuase because




Comment 2 Dwayne Bailey 2008-01-30 09:49:40 UTC
(In reply to comment #1)
> I can't approve your package, but I'd like to help You.

Thanks, much appreciated.

> I wanted to build this SRPM, but I couldn't.
> http://kurzawa.nonlogic.org/logs/python-Leveshtein-build.log

Strange, builds fine on my machine, wonder why it can't find the module to
import.  I might have some macro's defined that are hiding this on my side but
none seem obvious.

> I think that genextdoc.py.patch shouldn't be in Patch0, but in Source1.

Yes I agree, but couldn't work our how to get a Source1 to properly deposit the
program into the directory, as Patch0 it worked.  If you can help on that that
would be great.

> Rpmlint warns:
> python-Levenshtein.src: W: summary-ended-with-dot Python extension computing
> string distances and similarities.
> python-Levenshtein.src: W: spelling-error-in-description becuase because

Will fix those thanks.  Realised I didn't run rpmlint :(

Comment 3 Dwayne Bailey 2008-01-30 17:02:14 UTC
I've created a new src.rpm which fixes the document generating bug you saw:
http://translate.sourceforge.net/releases/testing/fedora/python-Levenshtein-0.10.1-3.fc8.src.rpm

The problem was an already installed version of python-Levenshtein while of
course I should be pulling in the just created one.

rpmlint issues are fixed.

The issue of using Source1 instead of a patch is not fixed.

Comment 4 Dwayne Bailey 2008-02-15 06:25:57 UTC
Spec URL:
http://translate.sourceforge.net/releases/testing/fedora/python-Levenshtein-0.10.1-4.spec
SRPM URL:
http://translate.sourceforge.net/releases/testing/fedora/python-Levenshtein-0.10.1-4.fc8.src.rpm

New SRPM and SPEC to address the last issue of using Source1 instead of Patch.

All the issues noted have now been fixed.

Comment 5 David Fraser 2008-03-26 14:17:39 UTC
(In reply to comment #4)
> Spec URL:
>
http://translate.sourceforge.net/releases/testing/fedora/python-Levenshtein-0.10.1-4.spec
> SRPM URL:
>
http://translate.sourceforge.net/releases/testing/fedora/python-Levenshtein-0.10.1-4.fc8.src.rpm
> 
> New SRPM and SPEC to address the last issue of using Source1 instead of Patch.
> 
> All the issues noted have now been fixed.

Ah, a package I can try and review in search of Sponsorship :-)

Note: The URL pointing to the package home
(http://trific.ath.cx/python/levenshtein/) no longer exists... in fact all URLs
relating to the project now redirect to http://trific.ath.cx - it seems that
he's simply redone his website in February and ditched all this stuff. Suggest
you contact the author to resolve...

Package Review
==============

Key:
 - = N/A
 x = Check
 ! = Problem
 ? = Not evaluated

=== REQUIRED ITEMS ===
 [x] Package is named according to the Package Naming Guidelines.
 [-] Spec file name must match the base package %{name}, in the format %{name}.spec.
  (this is OK as presumably your URL download is just versioned and will be
adjusted)
 [x] Package meets the Packaging Guidelines.
 [x] Package successfully compiles and builds into binary rpms on at least one
supported architecture.
     Tested on: FC8/i386
 [!] Rpmlint output:
source RPM: python-Levenshtein.src:26: E: use-of-RPM_SOURCE_DIR
binary RPM: empty

You should rather use %SOURCE1 to refer to the location of the source file.
Also, this source file should be pointed to a URL from whence it can be
downloaded. It may be good to make a package for genextdoc, and then you can
just say BuildRequires and run it normally

 [x] Package is not relocatable.
 [x] Buildroot is correct
(%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) )
 [x] Package is licensed with an open-source compatible license and meets other
legal requirements as defined in the legal section of Packaging Guidelines.
 [x] License field in the package spec file matches the actual license.
     License type: GPLv2+
 [x] If (and only 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 is included in %doc.
 [x] Spec file is legible and written in American English.
 [?] Sources used to build the package matches the upstream source, as provided
in the spec URL.
This is hard to check because the original website has disappeared, and the spec
URL provided points to the translate package download copy. Either taking over
maintenance of the project, or getting the author to resurrect it would solve this.
I did check against versions from the web archive successfully:
     SHA1SUM of tar.bz2:d630141e003f47a43e0f8eacdcbf593bf9d15ed6
http://web.archive.org/web/20070305015113/trific.ath.cx/Ftp/python/levenshtein/python-Levenshtein-0.10.1.tar.bz2
Also as noted above, genextdoc.py should be placed on a public URL
Also, it would be good to use %{version} for the version in the source URL
 [x] Package is not known to require ExcludeArch
 [x] All build dependencies are listed in BuildRequires, except for any that are
listed in the exceptions section of Packaging Guidelines.
 [-] The spec file handles locales properly.
 [-] ldconfig called in %post and %postun if required.
 [x] Package must own all directories that it creates.
 [x] Package requires other packages for directories it uses.
 [x] Package does not contain duplicates in %files.
 [x] Permissions on files are set properly.
 [x] Package has a %clean section, which contains rm -rf %{buildroot} (or
$RPM_BUILD_ROOT).
 [x] Package consistently uses macros.
 [x] Package contains code, or permissible content.
 [-] Large documentation files are in a -doc subpackage, if required.
 [x] Package uses nothing in %doc for runtime.
 [-] Header files in -devel subpackage, if present.
 [-] Static libraries in -devel subpackage, if present.
 [-] Package requires pkgconfig, if .pc files are present.
 [-] Development .so files in -devel subpackage, if present.
 [-] Fully versioned dependency in subpackages, if present.                    
 [x] Package does not contain
any libtool archives (.la).
 [-] Package contains a properly installed %{name}.desktop file if it is a GUI
application.
 [x] Package does not own files or directories owned by other packages.
 [x] All filenames present are valid UTF-8

=== SUGGESTED ITEMS ===
 [x] Latest version is packaged.
 [x] Package does not include license text files separate from upstream.
 [-] Description and summary sections in the package spec file contains
translations for supported Non-English languages, if available.
 [?] Reviewer should test that the package builds in mock.
     Tested on: devel/x86_64
 [?] Package should compile and build into binary rpms on all supported
architectures.
     Tested on: i386
 [x] Package functions as described.
 [-] Scriptlets must be sane, if used.
 [-] The placement of pkgconfig(.pc) files is correct.
 [-] File based requires are sane.



Comment 6 manuel wolfshant 2008-03-26 15:08:33 UTC
on my system mock builds yields:

error: Installed (but unpackaged) file(s) found:
   /usr/lib64/python2.5/site-packages/python_Levenshtein-0.10.1-py2.5.egg-info
    Installed (but unpackaged) file(s) found:
   /usr/lib64/python2.5/site-packages/python_Levenshtein-0.10.1-py2.5.egg-info
RPM build errors:

It might be an error in my setup but I think that you should read
http://fedoraproject.org/wiki/Packaging/Python/Eggs


Comment 7 David Fraser 2008-03-26 15:41:00 UTC
(In reply to comment #6)
> on my system mock builds yields:
> 
> error: Installed (but unpackaged) file(s) found:
>    /usr/lib64/python2.5/site-packages/python_Levenshtein-0.10.1-py2.5.egg-info
>     Installed (but unpackaged) file(s) found:
>    /usr/lib64/python2.5/site-packages/python_Levenshtein-0.10.1-py2.5.egg-info
> RPM build errors:
> 
> It might be an error in my setup but I think that you should read
> http://fedoraproject.org/wiki/Packaging/Python/Eggs
> 

This is basically a Fedora 9 thing as that page says:
> Starting with Fedora 9, our python package provides egg-info for any distutils
and setuptools packages
and which is why I didn't pick it up (testing on Fedora 8)

The relevant section of the above page which explains how to include Eggs for
non-setuptools modules is
http://fedoraproject.org/wiki/Packaging/Python/Eggs#head-4ac98208bf2f5a13b9cd997c91e2e424f67a7e35

Comment 8 manuel wolfshant 2008-03-26 17:45:58 UTC
My point was that since the most preferred way of adding packages to the Fedora
Collection is via rawhide, the spec must be corrected.


Comment 9 Dwayne Bailey 2008-03-27 10:00:45 UTC
Thanks David,

(In reply to comment #5)
> (In reply to comment #4)

>  [!] Rpmlint output:
> source RPM: python-Levenshtein.src:26: E: use-of-RPM_SOURCE_DIR
> binary RPM: empty
> 
> You should rather use %SOURCE1 to refer to the location of the source file.
> Also, this source file should be pointed to a URL from whence it can be
> downloaded. It may be good to make a package for genextdoc, and then you can
> just say BuildRequires and run it normally

Fixed - not worth packaging genextdoc separately.

>  [?] Sources used to build the package matches the upstream source, as provided
> in the spec URL.
> This is hard to check because the original website has disappeared, and the spec
> URL provided points to the translate package download copy. Either taking over
> maintenance of the project, or getting the author to resurrect it would solve
this.
> I did check against versions from the web archive successfully:
>      SHA1SUM of tar.bz2:d630141e003f47a43e0f8eacdcbf593bf9d15ed6
>
http://web.archive.org/web/20070305015113/trific.ath.cx/Ftp/python/levenshtein/python-Levenshtein-0.10.1.tar.bz2

Have left/tried to leave comment on the new blog.

> Also as noted above, genextdoc.py should be placed on a public URL

Not that critical I think.  Its as valuable as a patch

> Also, it would be good to use %{version} for the version in the source URL

Fixed.
 
>  [?] Reviewer should test that the package builds in mock.
>      Tested on: devel/x86_64
>  [?] Package should compile and build into binary rpms on all supported
> architectures.
>      Tested on: i386

Not sure myself how to do a mock build

Will upload new SRPM and SPEC


Comment 10 Dwayne Bailey 2008-03-27 10:16:33 UTC
Spec URL:
http://translate.sourceforge.net/releases/testing/fedora/python-Levenshtein-0.10.1-5.spec
SRPM URL:
http://translate.sourceforge.net/releases/testing/fedora/python-Levenshtein-0.10.1-5.fc8.src.rpm

New SRPM and SPEC to address the last issue of using Source1 instead of Patch.

$RPM_SOURCE_DIR - fixed
%{version} - fixed

Busy checking out mock

Comment 11 Dwayne Bailey 2008-06-02 20:18:49 UTC
Spec URL:
http://translate.sourceforge.net/releases/testing/fedora/python-Levenshtein-0.10.1-5.spec

SRPM URL:
http://translate.sourceforge.net/releases/testing/fedora/python-Levenshtein-0.10.1-5.fc9.src.rpm

Nothing has changed since last release, simply rebuilt on fc9 and built using mock.

mock rebuild -r fedora-devel-i386 python-Levenshtein-0.10.1-5.fc9.src.rpm

All works correctly.

Comment 12 Jason Tibbitts 2008-10-17 18:49:04 UTC
I'm trying to work through some of these old tickets; are you still interested in getting this package into Fedora?

Some observations:

Builds fine; rpmlint for me has only an erroneous complaint about permissions on the .so file.

The page I get from visiting the URL doesn't seem to have anything at all to do with this software.  Is there an actual upstream URL?

Where goes genextdoc.py come from?  You should provide its full location in Source1: if possible.

Otherwise everything looks OK to me; just let me know if you'd like for me to do a full review.

Comment 13 Dwayne Bailey 2008-10-20 13:17:31 UTC
(In reply to comment #12)
> I'm trying to work through some of these old tickets; are you still interested
> in getting this package into Fedora?

Yes as its used by other software that we are packaging.

> Some observations:
> 
> Builds fine; rpmlint for me has only an erroneous complaint about permissions
> on the .so file.

Not sure if that need fixing?

> The page I get from visiting the URL doesn't seem to have anything at all to do
> with this software.  Is there an actual upstream URL?

No their isn't the software seems to have disappeared but is hosted in various places around the net.

> Where goes genextdoc.py come from?  You should provide its full location in
> Source1: if possible.

I'm not really sure, it simply wasn't part of the tarball that I discovered and I found it through Google codesearch and pushed it into the package.

> Otherwise everything looks OK to me; just let me know if you'd like for me to
> do a full review.

If you are willing to do a full review that would be great.  If I know its going to move forward then I can spend more time correcting issues that you pick up.  So that's a YES! :)

Comment 14 Jason Tibbitts 2008-10-21 04:09:28 UTC
The odd permissions error doesn't need fixing; it's not your problem.  I only mention it because it is produced by rpmlint and anyone who builds the package and checks the output of rpmlint can potentially see the complaint (no matter how bogus) and may expect to see it addressed in the review.

Packaging code from a dead upstream poses a particular problem.  If there's really no upstream web site to refer to then it would make sense to specify no URL tag, or to continue to refer to the old location (in the hope that it will come back), but in either case it would be beneficial to add an explanatory comment to the spec.

The same goes for the Source1: URL; if there's no authoritative source for that file then you'll need to add a comment to that effect.  I'm guessing that the Source: URL is just pointing at your project's sourceforge site, and that you're just mirroring the dead upstream site.  Or are you taking over maintenance of the code?

The real concern with a dead upstream is ongoing maintenance and specifically security issues.  I don't see much possibility for security vulnerabilities in this code, but you never know, and if upstream is dead then the burden falls entirely on you, the maintainer, to deal with issues of that nature.  Anyway, it's up to you and I'm sure you're aware; I'm just making sure that it gets said at some point before the package is in the distro.

So basically the spec just needs a little bit of commenting about the state of upstream and the lack of a download location for Source1:.  My checklist follows; since there's not really any upstream, several of my usual checklist items will be missing.

* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package.
* BuildRequires are proper.
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* debuginfo package looks complete.
* rpmlint has acceptable complaints.
* final provides and requires are sane:
   Levenshtein.so()(64bit)
   python-Levenshtein = 0.10.1-5.fc10
   python-Levenshtein(x86-64) = 0.10.1-5.fc10
  =
   libpython2.5.so.1.0()(64bit)
   python(abi) = 2.5

* %check is not present; no test suite upstream.
* no shared libraries are added to the regular linker search paths.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no generically named files
* code, not content.
* documentation is small, so no -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* no headers.
* no pkgconfig files.
* no static libraries.
* no libtool .la files.

Comment 15 Dwayne Bailey 2008-10-21 15:02:16 UTC
(In reply to comment #14)
> Packaging code from a dead upstream poses a particular problem.  If there's
> really no upstream web site to refer to then it would make sense to specify no
> URL tag, or to continue to refer to the old location (in the hope that it will
> come back), but in either case it would be beneficial to add an explanatory
> comment to the spec.

Added comment and rather pointed to the pypi entry.

> The same goes for the Source1: URL; if there's no authoritative source for that
> file then you'll need to add a comment to that effect.  I'm guessing that the
> Source: URL is just pointing at your project's sourceforge site, and that
> you're just mirroring the dead upstream site.  

Found packages using the wayback machine and added comments to that effect.

> Or are you taking over
> maintenance of the code?

Simply mirroring and not taking over the code.

> The real concern with a dead upstream is ongoing maintenance and specifically
> security issues.  I don't see much possibility for security vulnerabilities in
> this code, but you never know, and if upstream is dead then the burden falls
> entirely on you, the maintainer, to deal with issues of that nature.  Anyway,
> it's up to you and I'm sure you're aware; I'm just making sure that it gets
> said at some point before the package is in the distro.

Yes am aware of these issues.

> So basically the spec just needs a little bit of commenting about the state of
> upstream and the lack of a download location for Source1:.  My checklist
> follows; since there's not really any upstream, several of my usual checklist
> items will be missing.

Done, mostly changes are simply commenting on the lack of upstream and pointing to way back machine.  Rebuilt and installed using mock on 9 and rawhide. New files here:

SRPM: http://translate.sourceforge.net/releases/testing/fedora/python-Levenshtein-0.10.1-6.fc9.src.rpm

SPEC: http://translate.sourceforge.net/releases/testing/fedora/python-Levenshtein-0.10.1-6.spec

Comment 16 Jason Tibbitts 2008-10-21 20:04:29 UTC
OK, this looks fine. 

APPROVED

Go ahead and apply for your Fedora account, do the CLA bits and apply for packager access.  I will sponsor you and then you can make your CVS request.

Comment 17 Jason Tibbitts 2008-10-27 22:22:41 UTC
Did you ever get your account set up?  I don't see any names matching yours in the system.

I'm going to be on vacation for a bit starting later this week, so if you want to get this set up before I go, please do the setup on your side soon.  If you need help with this, just let me know.

Comment 18 Dwayne Bailey 2008-10-29 09:01:50 UTC
@Jason: thanks sorry for the delay been a bit busy here.

(In reply to comment #16)
> Go ahead and apply for your Fedora account, do the CLA bits and apply for
> packager access.  

account, CLA done.  packager group access requested.  Hope I've made it before you go on holiday.

> I will sponsor you and then you can make your CVS request.

Comment 19 Dwayne Bailey 2008-10-29 19:46:56 UTC
New Package CVS Request
=======================
Package Name: python-Levenshtein
Short Description: Python extension computing string distances and similarities
Owners: dwayne
Branches: F-9 F-10
InitialCC:

Comment 20 Kevin Fenzi 2008-10-29 21:35:42 UTC
cvs done.

Comment 21 Fedora Update System 2008-10-30 06:20:03 UTC
python-Levenshtein-0.10.1-6.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/python-Levenshtein-0.10.1-6.fc9

Comment 22 Fedora Update System 2008-10-31 10:26:18 UTC
python-Levenshtein-0.10.1-6.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 23 Dwayne Bailey 2008-11-05 13:54:11 UTC
Package Change Request
======================
Package Name: python-Levenshtein
New Branches: EL-4 EL-5 OLPC-2 OLPC-3

Comment 24 Toshio Ernie Kuratomi 2008-11-05 23:54:09 UTC
cvs done.

Comment 25 Dwayne Bailey 2008-11-07 07:06:12 UTC
(In reply to comment #23)
> Package Change Request
> ======================
> Package Name: python-Levenshtein
> New Branches: EL-4 EL-5 OLPC-2 OLPC-3

(In reply to comment #24)
> cvs done.

The branches have appeared correctly on the pkgdb:
https://admin.fedoraproject.org/pkgdb/packages/name/python-Levenshtein

But branches aren't present in CVS:
http://cvs.fedoraproject.org/viewvc/rpms/python-Levenshtein/

I believe there was a CVS outage which might relate to the problem.  Thus re-requesting the branches.

Comment 26 Toshio Ernie Kuratomi 2008-11-07 14:57:29 UTC
My bad.

cvs really done this time :-)

Comment 27 Fedora Update System 2008-11-24 07:25:15 UTC
python-Levenshtein-0.10.1-6.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/python-Levenshtein-0.10.1-6.fc10

Comment 28 Fedora Update System 2008-11-26 06:20:40 UTC
python-Levenshtein-0.10.1-6.fc10 has been pushed to the Fedora 10 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.