Bug 812561

Summary: Review Request: python-ipdb - IPython enabled Python debugger
Product: [Fedora] Fedora Reporter: Brian Lane <bcl>
Component: Package ReviewAssignee: Christopher Meng <i>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: i, karlthered, notting, package-review, tomspur
Target Milestone: ---Flags: i: fedora-review+
gwync: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2013-11-18 15:55:55 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:

Description Brian Lane 2012-04-14 21:45:36 UTC
Spec URL: http://bcl.fedorapeople.org/python-ipdb/python-ipdb.spec
SRPM URL: http://bcl.fedorapeople.org/python-ipdb/python-ipdb-0.7-1.20120414git.fc16.src.rpm
Description: IPython features (tab completion, syntax highlighting, better tracebacks, better introspection) right in pdb.

Comment 1 Thomas Spura 2012-04-17 19:47:24 UTC
Just a few comments/questions:
- Which commit exactly did you package? It would be nice to have that in the spec instead of HEAD.
  (ups... in the changelog, never mind ;))

- Is there a "sane way" of running a test, so that it still works?
  (Means, what can I do to ensure, I won't break it with a ipython update...?)
  The %check is at the moment useless and nosetests doesn't run automatically
  (requires user input)
- Requires: python3-ipython missing, when it'll be ready.
- HISTORY says: "0.7 (unreleased)"
  --> Release must be 0.1
- python3-ipython will be there as soon as all dependencies are fullfilled...
  (builds and works mostly anyway even right now, but I don't want to ship a partly broken package....)
- Could you please be more specific in %files, so you know when the egg can't be build?
e.g.
%{python_sitelib}/ipdb/
%{python_sitelib}/ipdb-%{version}*.egg-info/

Comment 2 Brian Lane 2012-04-21 16:23:58 UTC
Thanks for the review.

You're right the test is pretty useless at the moment, so I'll comment that out. I'm not sure if there's a good way to run an automated test on an interactive debugger.

Added the python3-ipython requires

HISTORY.txt comes from upstream. Should I patch it to say 0.7-2.20120414git?

Fixed the paths to ipdb and egg-info.

I also removed all the tabs that somehow snuck into the spec.

New files:
Spec URL: http://bcl.fedorapeople.org/python-ipdb/python-ipdb.spec
SRPM URL: http://bcl.fedorapeople.org/python-ipdb/python-ipdb-0.7-2.20120414git.fc16.src.rpm

Comment 3 Thomas Spura 2012-04-22 08:56:50 UTC
(In reply to comment #2)
> You're right the test is pretty useless at the moment, so I'll comment that
> out. I'm not sure if there's a good way to run an automated test on an
> interactive debugger.

I hope it'll be enough to just test the single test with nosetests, when I'll update ipython... ipython-0.13 will come in the next month and don't know yet, if I'll do a f17 update too.

> HISTORY.txt comes from upstream. Should I patch it to say 0.7-2.20120414git?

0.7 wasn't released yet as in setup.py is this:
"version = '0.7dev'"
and commit d404542e says:
"Back to development: 0.7"
and there is no tag, that says 0.7
and there are no downloads available with version 0.7

This means you are shipping an unreleased version and therefore need the proper pre-release naming convention:
http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Pre-Release_packages


Why aren't you using the released 0.6.2?
It seems run{,call,eval} and the python3 support are new since then.


It would be great to ask upstream, what license exactly is meant with GPL as "GPL" seems to be bad:
http://fedoraproject.org/wiki/Packaging:LicensingGuidelines#GPL_and_LGPL

As upstream doesn't add a LICENSE/COPYING file, it's a SHOULD anyway to ask them to add one...

Comment 4 Brian Lane 2012-04-22 15:29:30 UTC
(In reply to comment #3)
> 0.7 wasn't released yet as in setup.py is this:
> "version = '0.7dev'"
> and commit d404542e says:
> "Back to development: 0.7"
> and there is no tag, that says 0.7
> and there are no downloads available with version 0.7
> 
> This means you are shipping an unreleased version and therefore need the proper
> pre-release naming convention:
> http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Pre-Release_packages

I'm not sure I know what you mean here. Should I change the Release to be:

Release:        0.2.dev.%{checkout}%{?dist}

Having dev and the checkout info seems redundant to me, but after rereading the naming guidelines I guess that's the most correct way to do it.

> 
> 
> Why aren't you using the released 0.6.2?
> It seems run{,call,eval} and the python3 support are new since then.
> 

I wanted to use the most recent version available, 0.6.1 was released in 10/2011 (I assume you mean 0.6.1, as there is no 0.6.2).

> 
> It would be great to ask upstream, what license exactly is meant with GPL as
> "GPL" seems to be bad:
> http://fedoraproject.org/wiki/Packaging:LicensingGuidelines#GPL_and_LGPL
> 
> As upstream doesn't add a LICENSE/COPYING file, it's a SHOULD anyway to ask
> them to add one...

I'll see if upstream will add this and clarify the license.

Comment 5 Thomas Spura 2012-04-22 16:08:07 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > This means you are shipping an unreleased version and therefore need the proper
> > pre-release naming convention:
> > http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Pre-Release_packages
> 
> I'm not sure I know what you mean here. Should I change the Release to be:
> 
> Release:        0.2.dev.%{checkout}%{?dist}

A "Release: $number" refers to a proper released version of the package and
  "Release: 0.$number" means, it is a pre-release version.

So it looks from the package version, that you packaged 0.7, which is not released yet and therefore needs an "0." in the Release macro.

> Having dev and the checkout info seems redundant to me, but after rereading the
> naming guidelines I guess that's the most correct way to do it.

/me too. I'd left "dev" out of the release:
Release:    0.2.${date}git%{?dist}
or
Release:    0.2.${date}git${commitid}%{?dist}

> > 
> > 
> > Why aren't you using the released 0.6.2?
> > It seems run{,call,eval} and the python3 support are new since then.
> > 
> 
> I wanted to use the most recent version available, 0.6.1 was released in
> 10/2011 (I assume you mean 0.6.1, as there is no 0.6.2).

Yes, sure...

> > 
> > It would be great to ask upstream, what license exactly is meant with GPL as
> > "GPL" seems to be bad:
> > http://fedoraproject.org/wiki/Packaging:LicensingGuidelines#GPL_and_LGPL
> > 
> > As upstream doesn't add a LICENSE/COPYING file, it's a SHOULD anyway to ask
> > them to add one...
> 
> I'll see if upstream will add this and clarify the license.

Great. Most likely they mean GPLv2+ with it, but maybe only GPLv2...

Comment 6 Brian Lane 2012-05-21 16:22:48 UTC
I haven't heard anything back from upstream on adding a license file.

Comment 7 Brian Lane 2012-12-10 05:11:22 UTC
Sorry, this fell off my radar.

New files:
Spec URL: http://bcl.fedorapeople.org/python-ipdb/python-ipdb.spec
SRPM URL: http://bcl.fedorapeople.org/python-ipdb/python-ipdb-0.7-3.fc18.src.rpm

Comment 8 Haïkel Guémar 2013-03-27 19:42:42 UTC
* upstream provides a 0.7 tarball on PyPI, please update your package accordingly
* unless you plan to maintain EPEL5 branch, no need of defining buildroot or cleaning it
* please fix the with_python3 conditional as expressed in guidelines
https://fedoraproject.org/wiki/Packaging:Python#Example_spec_file
http://lists.fedoraproject.org/pipermail/python-devel/2012-July/000407.html
* drop %defattr macro, it's no more required for RPM >= 4.4.x (includes EPEL5)
https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#File_Permissions
* since it's noarch, drop the CFLAGS
* packages installs and works fine

Comment 9 Brian Lane 2013-11-13 14:51:25 UTC
Updated with changes from comment 8, new upstream v0.8 and switched to github for source because pypi only carried .zip archive.

Spec URL: http://bcl.fedorapeople.org/python-ipdb/python-ipdb.spec
SRPM URL: http://bcl.fedorapeople.org/python-ipdb/python-ipdb-0.8-1.fc21.src.rpm

Comment 10 Christopher Meng 2013-11-13 15:00:23 UTC
Will review tomorrow(also set fedora-review to ?)

Comment 11 Christopher Meng 2013-11-14 13:01:03 UTC
%{__python}-->%{__python2}
%{python_sitelib}-->%{python2_sitelib}

Above changes are useful and more than just a SHOULD but MUST now in order to improve python3 support with parallel installable python2 noadays.
--------
Remove %clean section

comment 8 has told you that no need to keep EL5 stuffs. And the fact is this module only supports py2.5+, so you really have no need to support EL5.

--------

Review failed with:

Executing(%check): /bin/sh -e /var/tmp/rpm-tmp.MCelDu
+ umask 022
+ cd /builddir/build/BUILD
+ cd ipdb-0.8
+ /usr/bin/python setup.py test
running test
Searching for ipython>=0.10
Reading https://pypi.python.org/simple/ipython/
Best match: ipython 1.1.0
Downloading https://pypi.python.org/packages/source/i/ipython/ipython-1.1.0.zip#md5=3b383afb9f367eb387d36e11f6e7c4cc
Processing ipython-1.1.0.zip
Writing /tmp/easy_install-K0wO7b/ipython-1.1.0/setup.cfg
Running ipython-1.1.0/setup.py -q bdist_egg --dist-dir /tmp/easy_install-K0wO7b/ipython-1.1.0/egg-dist-tmp-Qv6mZJ
Installed /builddir/build/BUILD/ipdb-0.8/ipython-1.1.0-py2.7.egg
running egg_info
writing requirements to ipdb.egg-info/requires.txt
writing ipdb.egg-info/PKG-INFO
writing top-level names to ipdb.egg-info/top_level.txt
writing dependency_links to ipdb.egg-info/dependency_links.txt
writing entry points to ipdb.egg-info/entry_points.txt
reading manifest file 'ipdb.egg-info/SOURCES.txt'
reading manifest template 'MANIFEST.in'
writing manifest file 'ipdb.egg-info/SOURCES.txt'
running build_ext
test_import (tests.test_import.ImportTest) ... ok
----------------------------------------------------------------------
Ran 1 test in 2.006s
OK
/builddir/build/BUILD/ipdb-0.8/ipython-1.1.0-py2.7.egg/IPython/core/displayhook.py:275: RuntimeWarning: Parent module 'IPython.core' not found while handling absolute import
  import gc
+ pushd /builddir/build/BUILD/python3-python-ipdb-0.8-1.fc21
~/build/BUILD/python3-python-ipdb-0.8-1.fc21 ~/build/BUILD/ipdb-0.8
+ /usr/bin/python3 setup.py test
running test
Searching for ipython>=0.10
Reading https://pypi.python.org/simple/ipython/
Download error on https://pypi.python.org/simple/ipython/: [Errno 111] Connection refused -- Some packages may not be found!
Scanning index of all packages (this may take a while)
Reading https://pypi.python.org/simple/
Couldn't find index page for 'ipython' (maybe misspelled?)
No local packages or download links found for ipython>=0.10
error: Could not find suitable distribution for Requirement.parse('ipython>=0.10')
error: Bad exit status from /var/tmp/rpm-tmp.MCelDu (%check)
RPM build errors:
    Bad exit status from /var/tmp/rpm-tmp.MCelDu (%check)
Child return code was: 1

It still downloads sources from pypi but no reading from local.

Comment 12 Brian Lane 2013-11-15 01:42:00 UTC
Thanks, updated with those changes. I think your build was a temporary failure, it works fine for me using mock --rebuild

Spec URL: http://bcl.fedorapeople.org/python-ipdb/python-ipdb.spec
SRPM URL: http://bcl.fedorapeople.org/python-ipdb/python-ipdb-0.8-2.fc21.src.rpm

Comment 13 Christopher Meng 2013-11-15 02:59:03 UTC
Hmm..

1. Do we still need conditional with_python3 line? I'm not sure.

2. python-ipdb.noarch: W: incoherent-version-in-changelog 0.8-1 ['0.8-2.fc21', '0.8-2']

3. 

Requires
--------
python-ipdb (rpmlib, GLIBC filtered):
    /usr/bin/python2
    ipython
    python(abi)
    python3-ipython

python3-ipdb (rpmlib, GLIBC filtered):
    /usr/bin/python3
    python(abi)

And I tried this:

[root@fab results]# yum localinstall python-ipdb-0.8-2.fc21.noarch.rpm

..[cut]..

Dependencies Resolved

=======================================================================================================
 Package                  Arch   Version                             Repository                   Size
=======================================================================================================
Installing:
 python-ipdb              noarch 0.8-2.fc21                          /python-ipdb-0.8-2.fc21.noarch
                                                                                                  41 k
Installing for dependencies:
 agg                      i686   2.5-19.fc20                         rawhide                     148 k
 openpgm                  i686   5.2.122-2.fc20                      rawhide                     173 k
 pyparsing                noarch 2.0.1-1.fc21                        rawhide                      94 k
 python-ipython           noarch 0.13.2-2.fc20                       rawhide                      10 k
 python-ipython-console   noarch 0.13.2-2.fc20                       rawhide                     1.2 M
 python-ipython-gui       noarch 0.13.2-2.fc20                       rawhide                     158 k
 python-ipython-notebook  noarch 0.13.2-2.fc20                       rawhide                     310 k
 python-matplotlib        i686   1.3.0-1.fc20                        rawhide                      32 M
 python-mglob             noarch 0.4-9.fc20                          rawhide                      11 k
 python-simplegeneric     noarch 0.8-7.fc20                          rawhide                      12 k
 python-tornado           noarch 2.2.1-6.fc20                        rawhide                     417 k
 python-zmq               i686   13.0.2-1.fc20                       rawhide                     422 k
 python3-PyQt4            i686   4.10.3-1.fc21                       rawhide                     2.7 M
 python3-dateutil         noarch 2.0-6.fc20                          rawhide                      83 k
 python3-ipython          noarch 0.13.2-2.fc20                       rawhide                      10 k
 python3-ipython-console  noarch 0.13.2-2.fc20                       rawhide                     1.3 M
 python3-ipython-gui      noarch 0.13.2-2.fc20                       rawhide                     164 k
 python3-ipython-notebook noarch 0.13.2-2.fc20                       rawhide                     313 k
 python3-matplotlib       i686   1.3.0-1.fc20                        rawhide                      32 M
 python3-mglob            noarch 0.4-9.fc20                          rawhide                      11 k
 python3-numpy            i686   1:1.8.0-2.fc21                      rawhide                     3.0 M
 python3-pexpect          noarch 3.0-1.fc21                          rawhide                     133 k
 python3-pyparsing        noarch 2.0.1-1.fc21                        rawhide                      99 k
 python3-pytz             noarch 2012d-5.fc20                        rawhide                     220 k
 python3-simplegeneric    noarch 0.8-7.fc20                          rawhide                      13 k
 python3-sip              i686   4.15.3-1.fc21                       rawhide                      93 k
 python3-tornado          noarch 2.2.1-6.fc20                        rawhide                     428 k
 python3-zmq              i686   13.0.2-1.fc20                       rawhide                     227 k
 stix-math-fonts          noarch 1.1.0-5.fc20                        rawhide                     286 k
 texlive-base             noarch 3:2013-3.20131021_r31961.fc21       rawhide                     1.4 M
 texlive-dvipng           noarch 3:svn29821.1.14-3.fc21              rawhide                      43 k
 texlive-dvipng-bin       i686   3:svn30845.0-3.20131021_r31961.fc21 rawhide                      59 k
 texlive-kpathsea         noarch 3:svn30947.0-3.fc21                 rawhide                     139 k
 texlive-kpathsea-bin     i686   3:svn30088.0-3.20131021_r31961.fc21 rawhide                      38 k
 texlive-kpathsea-lib     i686   3:2013-3.20131021_r31961.fc21       rawhide                      86 k
 zeromq3                  i686   3.2.4-1.fc21                        rawhide                     336 k

Transaction Summary
=======================================================================================================

Comment 14 Christopher Meng 2013-11-15 03:00:20 UTC
For the first question I made, I need to explain a bit more clearly:

Do we need to care about fedora < 12?

Comment 15 Thomas Spura 2013-11-15 09:00:45 UTC
(In reply to Christopher Meng from comment #13)
> Requires
> --------
> python-ipdb (rpmlib, GLIBC filtered):
>     /usr/bin/python2
>     ipython
>     python(abi)
>     python3-ipython
> 
> python3-ipdb (rpmlib, GLIBC filtered):
>     /usr/bin/python3
>     python(abi)

You need to Require python3-ipython within the python3-ipdb subpackage.

(In reply to Christopher Meng from comment #14)
> For the first question I made, I need to explain a bit more clearly:
> 
> Do we need to care about fedora < 12?

No. But as epel is not fedora, 0{?fedora} is 0 there and we won't have a python3 package.

Comment 16 Brian Lane 2013-11-15 17:05:36 UTC
I think we should leave with_python3 in there, we can revisit it when el7 is created, and it is better to have control over it.

Here's a new set with the requires moved. Sorry about that, I'd sworn I put them in the subpackage.

Spec URL: http://bcl.fedorapeople.org/python-ipdb/python-ipdb.spec
SRPM URL: http://bcl.fedorapeople.org/python-ipdb/python-ipdb-0.8-3.fc21.src.rpm

Comment 17 Christopher Meng 2013-11-17 08:23:05 UTC
PACKAGE APPROVED!

Comment 18 Brian Lane 2013-11-18 01:36:42 UTC
New Package SCM Request
=======================
Package Name: python-ipdb 
Short Description: IPython enabled Python debugger
Owners: bcl
Branches: f20 f19 f18
InitialCC:

Comment 19 Gwyn Ciesla 2013-11-18 13:00:07 UTC
Git done (by process-git-requests).