Bug 812561
Summary: | Review Request: python-ipdb - IPython enabled Python debugger | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Brian Lane <bcl> |
Component: | Package Review | Assignee: | Christopher Meng <i> |
Status: | CLOSED RAWHIDE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | 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
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/ 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 (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... (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. (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... I haven't heard anything back from upstream on adding a license file. 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 * 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 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 Will review tomorrow(also set fedora-review to ?) %{__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. 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 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 ======================================================================================================= For the first question I made, I need to explain a bit more clearly: Do we need to care about fedora < 12? (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. 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 PACKAGE APPROVED! New Package SCM Request ======================= Package Name: python-ipdb Short Description: IPython enabled Python debugger Owners: bcl Branches: f20 f19 f18 InitialCC: Git done (by process-git-requests). |