Bug 607342 - Review Request: python-markupsafe - A safe escaping library for markup languages like HTML
Summary: Review Request: python-markupsafe - A safe escaping library for markup langua...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 607302 608155
TreeView+ depends on / blocked
 
Reported: 2010-06-23 20:51 UTC by Kyle VanderBeek
Modified: 2010-07-21 19:58 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-06-28 01:33:00 UTC
Type: ---
Embargoed:
j: fedora-review+
j: fedora-cvs+


Attachments (Terms of Use)

Description Kyle VanderBeek 2010-06-23 20:51:34 UTC
Spec URL: http://www.kylev.com/tmp/python-markupsafe.spec
SRPM URL: http://www.kylev.com/tmp/python-markupsafe-0.9.2-1.fc13.src.rpm
Description: This is actually a new dependency for python-mako as of version 0.3.4
(which is a security fix release).

Comment 1 Jason Tibbitts 2010-06-24 05:06:01 UTC
A couple of quick comments; I'll do a full review once the package builds.

The spec looks as clean as can be for something that supports python 2 and 3 as well as F12 and EPEL.  However, some additional conditionals are needed for the first two lines as python3 isn't available on F12 or EPEL and F13 doesn't need the python_site* define.  (F13 also doesn't need %clean, the BuildRoot line or the buildroot cleaning in %install, but it's pointless to conditionalize those.)

So:

%if 0%{?fedora} > 12 || 0%{?rhel} > 6
%global with_python3 1
%else
%{!?python_sitelib: %global python_sitelib %(%{__python} -c "from distutils.sysconfig import get_python_lib; print (get_python_lib())")}
%endif

(probably horribly wordwrapped) and things are good.  Also, I suspect this will have problems on F12 anyway as its arch-specific so you probably want the python_sitearch define there instead of sitelib, especially since sitearch is used in %files.

Unfortunately this fails to build for me in rawhide; setyp.py won't run because setuptools is not installed.  A scratch build is at http://koji.fedoraproject.org/koji/taskinfo?taskID=2269296

Comment 2 Kyle VanderBeek 2010-06-24 20:07:36 UTC
Updated with fixed with_python3 and sitearch rather than sitelib.

Spec in same place, new SRPM
http://www.kylev.com/tmp/python-markupsafe-0.9.2-2.fc13.src.rpm

Comment 3 Jason Tibbitts 2010-06-24 20:23:01 UTC
Still fails to build on rawhide, same error as before.

Comment 4 Jason Tibbitts 2010-06-24 20:42:06 UTC
It does build OK if I add a build dependency on python-setuptools-devel and a conditionalized dep on python3-setuptools.

Comment 5 Kyle VanderBeek 2010-06-24 21:17:45 UTC
I totally blew it and missed your second paragraph.

Fixed BuildRequires and have done scratch builds for rawhide, F-13, F-12 and EPEL-5 (all the targets that Mako is currently on).

http://www.kylev.com/tmp/python-markupsafe-0.9.2-3.fc13.src.rpm

Comment 6 Jason Tibbitts 2010-06-25 05:30:17 UTC
Yep, that builds fine for me.  rpmlint just says:

python-markupsafe.x86_64: W: private-shared-object-provides
   /usr/lib64/python2.6/site-packages/markupsafe/_speedups.so _speedups.so()
  (64bit)
which as far as I know you can't do anything about because we have no dependency filtering for arch-specific packages.

The only issue I see: perhaps I'm missing something, but does the URL actually have anything at all to do with this package?  I can't find any mention at all of this module at http://dev.pocoo.org/.

It's what pypi says, so I'm no going to fret over it, but it seems a bit odd.  Honestly I'd probably list the pypi page here instead because it at least has useful information related to the module.

* source files match upstream.  sha256sum:
   d2a1980ea5bae80a999dad6aafe1ad2f0440522630caeecb84ffe061c8054342
   MarkupSafe-0.9.2.tar.gz
* 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.
* latest version is being packaged.
* 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:
  python3-markupsafe-0.9.2-3.fc14.x86_64.rpm
   _speedups.so()(64bit)  
   python3-markupsafe = 0.9.2-3.fc14
   python3-markupsafe(x86-64) = 0.9.2-3.fc14
  =
   libpython3.1.so.1.0()(64bit)  
   python(abi) = 3.1

  python-markupsafe-0.9.2-3.fc14.x86_64.rpm
   _speedups.so()(64bit)  
   python-markupsafe = 0.9.2-3.fc14
   python-markupsafe(x86-64) = 0.9.2-3.fc14
  =
   libpython2.6.so.1.0()(64bit)  
   python(abi) = 2.6

* %check is present and all tests pass:
   Ran 2 tests in 0.270s
   Ran 2 tests in 0.253s

* 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.

APPROVED

Comment 7 Kyle VanderBeek 2010-06-25 05:46:08 UTC
New Package CVS Request
=======================
Package Name: python-markupsafe
Short Description: XML/HTML/XHTML Markup safe string for Python
Owners: kylev lmacken
Branches: F-12 F-13 EL-5
InitialCC:

Comment 8 Jason Tibbitts 2010-06-26 08:06:04 UTC
CVS done (by process-cvs-requests.py).

Comment 9 Fedora Update System 2010-06-27 08:44:41 UTC
python-markupsafe-0.9.2-3.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/python-markupsafe-0.9.2-3.fc13

Comment 10 Fedora Update System 2010-06-27 08:50:43 UTC
python-markupsafe-0.9.2-3.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/python-markupsafe-0.9.2-3.fc12

Comment 11 Fedora Update System 2010-06-27 08:51:42 UTC
python-markupsafe-0.9.2-3.el5 has been submitted as an update for Fedora EPEL 5.
http://admin.fedoraproject.org/updates/python-markupsafe-0.9.2-3.el5

Comment 12 Fedora Update System 2010-06-28 17:14:16 UTC
python-markupsafe-0.9.2-3.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 13 Fedora Update System 2010-06-28 17:21:45 UTC
python-markupsafe-0.9.2-3.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 14 Fedora Update System 2010-07-21 19:58:16 UTC
python-markupsafe-0.9.2-3.el5 has been pushed to the Fedora EPEL 5 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.