Bug 459800

I have to admit, it's terribly funny to "%define srcname py" and then go on to use "%{srcname}" in place of "py".  Ten keystrokes (twelve if you count the shift key) to replace two?  But that's your option as the maintainer.

0.9.2 came out the same day you opened this ticket; did you want to update the package?

With a license like that, you really need to be more specific about which parts have which license.  Otherwise the poor reviewer has to repeat the entire process you used to figure out the licensing information along with anyone who may maintain it in the future.

(In reply to comment #1)
> I have to admit, it's terribly funny to "%define srcname py" and then go on to
> use "%{srcname}" in place of "py".  Ten keystrokes (twelve if you count the
> shift key) to replace two?  But that's your option as the maintainer.

Well, there are more reasons for defining macros than solely reducing the number of keystrokes. Normally I use %{name} e.g. for the Url and Source tags,  but in case of python packages %{name} doesn't work because of the leading 'python-', that's why there is %{srcname}.

Anyway, I removed %{srcname} here.

> 0.9.2 came out the same day you opened this ticket; did you want to update the
> package?

Thanks for the hint. Updating the package indeed simplified packaging a bit, because upstream made some efforts towards a clean install using setuptools.
 
> With a license like that, you really need to be more specific about which parts
> have which license.  Otherwise the poor reviewer has to repeat the entire
> process you used to figure out the licensing information along with anyone who
> may maintain it in the future.

The specfile now has a more detailed list wrt licensing.

Spec URL: http://thm.fedorapeople.org/python-py.spec
SRPM URL: http://thm.fedorapeople.org/python-py-0.9.2-1.fc9.src.rpm

%changelog
* Sun Sep  7 2008 Thomas Moschny <thomas.moschny> - 0.9.2-1
- Update to 0.9.2.
- Upstream now uses setuptools and installs to %%{python_sitearch}.
- Remove %%{srcname} macro.
- More detailed information about licenses.

NEEDSWORK

Good:
* Named according to the package naming guidelines and spec matches
* Licensed appropriately:
  Regarding your note about test/web/post_multipart.py, you probably want to
  add a link to this: http://code.activestate.com/help/terms/.  (That would be
  Python for this code snippet)
* Spec file is legible.
* Tarball matches upstream.
* Not a shared library package
* Builds in mock on i386
* package owns all directories it creates and no others
* all filenames are utf-8
* proper %clean
* macros used consistently
* No duplicate files
* package contains code not content

To be fixed:
* rpmlint:
python-py.i386: E: non-standard-executable-perm /usr/lib/python2.5/site-packages/py/c-extension/greenlet/greenlet.so 0775
3 packages and 0 specfiles checked; 1 errors, 0 warnings.

Not sure why setuptools is making this 0755 instead of 0755 but you can correct
it in your spec file:
%install
[...]
chmod 0755 %{buildroot}%{python_sitearch}/py/c-extension/greenlet/greenlet.so

* The package has a compat directory with modules that are also in the
  stdlibrary.  It looks like they're just copies svn around the 2.4.4 release.
  These have to go.

  I'd suggest the following:

  - For the Fedora package, our goal is to be able to rm -rf py/compat.  We
    should do this in the spec file.
  - For upstream we need a patch that first tries system libraries and then
    fallsback to the py/compat modules if necessary.

  When patching the source we probably need to grep for occurrences of "compat"
  in the source.  Then change things like this::

    import py

    usage = "usage: %prog [-s [filename ...] | [-i | -c filename ...]]"
    optparser = py.compat.optparse.OptionParser(usage)

  to::

    import py
    try:
        import optparse
    except ImportError:
        from py.compat import optparse

    usage = "usage: %prog [-s [filename ...] | [-i | -c filename ...]]"
    optparser = py.compat.optparse.OptionParser(usage)

This second problem is important enough to require fixing before approval.

Oops, that should be::
    optparser = optparse.OptionParser(usage)

ping

The py.magic.greenlet code does not work on ppc/ppc64, and the corresponding test segfaults. So far, upstream has not been able to reproduce that, and thus there's no fix yet, that's why we are currently stalled here.

Spec URL: http://thm.fedorapeople.org/python-py.spec
SRPM URL: http://thm.fedorapeople.org/python-py-0.9.2-4.fc9.src.rpm

%changelog
* Fri Nov 21 2008 Thomas Moschny <..> - 0.9.2-4
- Use dummy_greenlet on ppc and ppc64.

* Tue Oct  7 2008 Thomas Moschny <..> - 0.9.2-3
- Replace compat modules by stubs using the system modules instead.
- Add patch from trunk fixing a timing issue in the tests.

* Tue Sep 30 2008 Thomas Moschny <..> - 0.9.2-2
- Update license information.
- Fix the tests.

Still four failing tests, though, see
http://koji.fedoraproject.org/koji/taskinfo?taskID=944709
http://koji.fedoraproject.org/koji/taskinfo?taskID=944685

Problems reported upstream, see
  https://codespeak.net/issue/py-dev/issue66
and
  https://codespeak.net/issue/py-dev/issue67

Spec URL: http://thm.fedorapeople.org/python-py.spec
SRPM URL: http://thm.fedorapeople.org/python-py-0.9.2-5.fc10.src.rpm

%changelog
* Fri Dec 12 2008 Thomas Moschny <..> - 0.9.2-5
- Add patch from trunk fixing a subversion 1.5 problem (pylib
  issue66).
- Don't replace doctest compat module (pylib issue67).

The doctest failure in apigen.txt seems to be a genuine failure in py that the compat-doctest module is somehow mishandling.  If you run the commands in apigen.txt in a python interactive shell, you get the traceback that doctest from python-2.5 yields.  I've been looking at this more but haven't gotten to the bottom of it yet.

The example fails in a python-2.5 and python-2.4.3 shell.

Here's a session you can feed back to the upstream bug report.  If the opportunity arises, you can also mention that this is a reason not to include copies of system libraries in something they ship ;-)

Python 2.4.3 (#1, Jan 14 2008, 18:32:40) 
[GCC 4.1.2 20070626 (Red Hat 4.1.2-14)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import py
>>> p = py.path.local('.')
>>> p.check(dir=True)
True
>>> from py.__.apigen.tracer.docstorage import DocStorage, DocStorageAccessor
>>> from py.__.apigen.tracer.tracer import Tracer
>>> toregister = {'py.path.local': py.path.local, 'py.path.svnwc': py.path.svnwc}
>>> ds = DocStorage().from_dict(toregister)
>>> t = Tracer(ds)
>>> t.start_tracing()
>>> p = py.path.local('.')
>>> p.check(dir=True)
Traceback (most recent call last):
  File "<stdin>", line 1, in ?
  File "/home/fedora/toshio/py-0.9.2/py/path/common.py", line 95, in check
    def check(self, **kw):
  File "/home/fedora/toshio/py-0.9.2/py/apigen/tracer/tracer.py", line 38, in _tracer
    self.docstorage.consider_call(frame, None, self.frame)
  File "/home/fedora/toshio/py-0.9.2/py/apigen/tracer/docstorage.py", line 80, in consider_call
    desc.consider_call_site(caller_frame, upward_cut_frame)
  File "/home/fedora/toshio/py-0.9.2/py/apigen/tracer/description.py", line 170, in consider_call_site
    cs = cut_stack(stack, frame, cut_frame)
  File "/home/fedora/toshio/py-0.9.2/py/apigen/tracer/description.py", line 81, in cut_stack
    lst = [py.code.Frame(i) for i in stack[stack.index(frame):\
ValueError: list.index(x): x not in list

Good:
* rpmlint is now silent
* optparse, textwrap, and subprocess have been switched to system libraries.

Still Needswork:
* doctest is still using the compat module due to:
    https://codespeak.net/issue/py-dev/issue67

Note that this is either a problem in the tracer module or the way we're being told to use the tracer module in apigen.txt, not in the py.test or doctest module as the current upstream bug seems to think.

Where does that leave us?

I think removing the doctest module in favor of system libs is the first step.  Then it would be very nice to run the tests with just the apigen.txt test disabled until upstream fixes the bug.  (This could be done by creative moving of the apigen.txt doc file before and after the test run although that's not pretty.  If there's a command line option to py.test that excludes the one failing test, that would be better.)

Once that's done, I can approve this package.

Thanks for looking into this. I've updated the upstream bug accordingly.

Spec URL: http://thm.fedorapeople.org/python-py.spec
SRPM URL: http://thm.fedorapeople.org/python-py-0.9.2-6.fc10.src.rpm

%changelog
* Wed Jan 14 2009 Thomas Moschny <..> - 0.9.2-6
- Use system doctest module again, as this wasn't the real cause of
  the test failure. Instead, remove the failing test for now.

I'd still ship the apigen documentation.  Maybe something like this::

# see pylib issue67
cp -p py/doc/apigen.txt apigen.txt.bak
cp -p py/doc/index.txt index.txt.bak
rm py/doc/apigen.txt
sed -i '/apigen/d' py/doc/index.txt

PYTHONPATH=$(pwd)/py %{__python} py/bin/py.test py
cp -p apigen.txt py/doc/apigen.txt.bak
cp -p index.txt.bak py/doc/index.txt

You can do this after import, however.  This is APPROVED.

(In reply to comment #15)
> I'd still ship the apigen documentation.  Maybe something like this::

The testsuite is run in BUILD, not BUILDROOT, (and after %install) so deleting some files there doesn't have an effect on the final package, and thus I don't think it is necessary to make backups.

> You can do this after import, however.  This is APPROVED.

Thanks for the review, Toshio!


New Package CVS Request
=======================
Package Name: python-py
Short Description: Innovative python library containing py.test, greenlets and other niceties
Owners: thm
Branches: F-9 F-10
InitialCC: none
Cvsextras Commits: yes

cvs done.

python-py-0.9.2-6.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update python-py'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-0968

python-py-0.9.2-6.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

Package Change Request
======================
Package Name: python-py
New Branches: el6

Misformatted request.

Package Change Request
======================
Package Name: python-py
New Branches: el6
Owners: thm

Git done (by process-git-requests).