Bug 648633 - Review Request: python-sphinx10 - Python documentation generator
Summary: Review Request: python-sphinx10 - Python documentation generator
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Toshio Ernie Kuratomi
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: 638152 (view as bug list)
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-11-01 19:38 UTC by Michel Alexandre Salim
Modified: 2010-11-19 18:32 UTC (History)
3 users (show)

Fixed In Version: python-sphinx10-1.0.4-3.el5
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-11-11 22:25:38 UTC
Type: ---
a.badger: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Michel Alexandre Salim 2010-11-01 19:38:44 UTC
Spec URL: http://salimma.fedorapeople.org/specs/python/python-sphinx10.spec
SRPM URL: http://salimma.fedorapeople.org/specs/python/python-sphinx10-1.0.4-1.fc14.src.rpm
Description:
Sphinx is a tool that makes it easy to create intelligent and
beautiful documentation for Python projects (or other documents
consisting of multiple reStructuredText sources), written by Georg
Brandl. It was originally created to translate the new Python
documentation, but has now been cleaned up in the hope that it will be
useful to many other projects.

This is a parallel-installable packaging of sphinx for older Fedora and RHEL releases.

Comment 1 Michel Alexandre Salim 2010-11-01 19:51:17 UTC
I notice that our main python-sphinx package does not ship LC_MESSAGES/*.po which I have not excluded from python-sphinx10; let me know if you want these to be stripped. Or if we should indeed re-add them to python-sphinx!

Comment 2 Michel Alexandre Salim 2010-11-01 20:09:17 UTC
*** Bug 638152 has been marked as a duplicate of this bug. ***

Comment 3 Toshio Ernie Kuratomi 2010-11-01 21:07:34 UTC
*.po files are a source file, *.mo files are what's actually used so we want to exclude them.  I noticed that you also included the *.js translations in the main python-sphinx package which was a good catch... but I think we're losing the marking of the files as language specific files.  Taking a look at that.

An initial look at this spec looks good except that the language stuff is going to be incorrect... but fixing it will take a bit of patching as the versions will clash if we don't.

The main sphinx package puts the language files in /usr/share/locale and /usr/share/sphinx.  %find_lang is able to find the files in /usr/share/locale and we manually find the files under /usr/share/sphinx.  This package leaves everything in the module directory so find_lang isn't marking them.  We can't move them to the same directory as the main sphinx uses as the files would clash with the main sphinx package then... probably have to patch the code to look for them under an alternate name/directory like sphinx10.

I can look at making a patch for this package after looking at the main package if you want.

Comment 4 Michel Alexandre Salim 2010-11-02 07:55:52 UTC
(In reply to comment #3)
> *.po files are a source file, *.mo files are what's actually used so we want to
> exclude them.  I noticed that you also included the *.js translations in the
> main python-sphinx package which was a good catch... but I think we're losing
> the marking of the files as language specific files.  Taking a look at that.

Is that actually the case? It looks like rpm -ql does not mark the files in the output -- I tried with another package that contains translations (gnome-vfs2) and it does not mark the translation files in the output either.

sphinx.lang from the build directory of both the main package and python-sphinx10 definitely has both the .mo and .js files marked with their language codes.


> I can look at making a patch for this package after looking at the main package
> if you want.

I can rework the patch for sphinx10 -- there was a patch in 0.6.5 (merged in 0.6.6) that adds the main locale directory, so I just need to refer to it to find which file to patch. But if you could figure out what's going on with the language marking that'd be great :)

Comment 5 Michel Alexandre Salim 2010-11-02 08:30:46 UTC
Locale files now moved to /usr/share/sphinx-1.0 (for .js files) and /usr/share/locale/*/LC_MESSAGES/sphinx-1.0.mo (for .mo files); I've patched the two places where they are referred to to use the new directories; see Sphinx-1.0.4-localedirs.patch in the SRPM

Spec URL: http://salimma.fedorapeople.org/specs/python/python-sphinx10.spec
SRPM URL:
http://salimma.fedorapeople.org/specs/python/python-sphinx10-1.0.4-2.fc14.src.rpm

Comment 6 Toshio Ernie Kuratomi 2010-11-02 09:27:27 UTC
Okay, you're doing it right :-)  I missed that you're using %dir for the directories in %files which is exactly what needs doing.  Here's the rpm invocation to check that:

rpm --qf '[%{filelangs} %{filenames}\n]' -qp python-sphinx-1.0.4-3.fc14.noarch.rpm

Comment 7 Michel Alexandre Salim 2010-11-02 10:38:06 UTC
Yeah, for a while we were actually *not* shipping the .js files at all (somehow nobody noticed).

PS the initial -2.fc14.src.rpm upload is missing some directory ownerships; I fixed it without bumping the revision just in time before your comment (05:04 EDT), so you probably have the right one, but just redownload if that's not the case.

Comment 8 Toshio Ernie Kuratomi 2010-11-02 18:20:32 UTC
Good:
* Package follows naming guidelines
* Package licensed  appropriately
* Spec file readable
* Sources match upstream
* locales handled correctly
* Not a shared library
* No bundled libraries
* Package owns all directories and files it creates and nothing else
* Permissions set properly
* Macros used consistently.
* code, not content.
* Nothing in %doc affects runtime
* Not a GUI
* All filenames are utf8

Needswork:
* Does not build in koji for EL-5:
http://koji.fedoraproject.org/koji/getfile?taskID=2572200&name=build.log

I'd just disable man page generation on EL-5 I think.  I'll check rpmlint and that it works properly after that's fixed.  Everything else looks fine.

Comment 9 Michel Alexandre Salim 2010-11-02 22:46:36 UTC
Turns out there are several more changes needed for EL-5; see spec file for details. I've tested a Koji build for EL-5 and a local build for F-14

Spec URL: http://salimma.fedorapeople.org/specs/python/python-sphinx10.spec
SRPM URL:
http://salimma.fedorapeople.org/specs/python/python-sphinx10-1.0.4-3.fc14.src.rpm
EL-5 build: http://koji.fedoraproject.org/koji/taskinfo?taskID=2572916

- manpage generation disabled
- conditionally re-add BuildRoot and %clean section
- manually create %{buildroot}%{python_sitelib} since EL-5's easy_install cannot recreate path

Comment 10 Toshio Ernie Kuratomi 2010-11-03 00:02:23 UTC
rpmlint:
python-sphinx10.src: W: spelling-error %description -l en_US reStructuredText -> restructured Text, restructured-text, restructure
python-sphinx10.src: W: spelling-error %description -l en_US indices -> induces, indies, indicts
python-sphinx10.src: W: spelling-error %description -l en_US docstrings -> doc strings, doc-strings, drawstrings

These are false positives

python-sphinx10.src: W: no-cleaning-of-buildroot %clean
python-sphinx10.src: W: no-buildroot-tag
python-sphinx10.src: W: no-%clean-section

Also false positives -- they're there just in conditionals.

python-sphinx10.noarch: E: script-without-shebang /usr/lib/python2.4/site-packages/Sphinx-1.0.4-py2.4.egg/sphinx/themes/epub/static/epub.css

Lots of these.  I think it's due to the version of setuptools in RHEL5 installing everything with execute permissions.  We probably want to run this in %install:

  find %{buildroot}%{python_sitelib} -type f -exec chmod a-x \{\} \;

python-sphinx10.noarch: W: no-manual-page-for-binary sphinx-1.0-quickstart
python-sphinx10.noarch: W: no-manual-page-for-binary sphinx-1.0-autogen
python-sphinx10.noarch: W: no-manual-page-for-binary sphinx-1.0-build

Ignorable -- we can't generate the man pages on EL5.

4 packages and 0 specfiles checked; 68 errors, 16 warnings.

You can go ahead and do the chmod when you import the package.  This review request is APPROVED.

Comment 11 Michel Alexandre Salim 2010-11-03 00:19:46 UTC
Thanks! Will fix the permissions and also make Pygments BR unconditional (not just for EL-5) when importing

New Package SCM Request
=======================
Package Name: python-sphinx10
Short Description: Python documentation generator
Owners: salimma
Branches: el5 f13
InitialCC:

Comment 12 Kevin Fenzi 2010-11-03 04:10:22 UTC
Git done (by process-git-requests).

Comment 13 Fedora Update System 2010-11-03 08:18:54 UTC
python-sphinx10-1.0.4-3.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/python-sphinx10-1.0.4-3.el5

Comment 14 Fedora Update System 2010-11-03 08:19:03 UTC
python-sphinx10-1.0.4-3.fc13 has been submitted as an update for Fedora 13.
https://admin.fedoraproject.org/updates/python-sphinx10-1.0.4-3.fc13

Comment 15 Fedora Update System 2010-11-03 16:35:55 UTC
python-sphinx10-1.0.4-3.el5 has been pushed to the Fedora EPEL 5 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-sphinx10'.  You can provide feedback for this update here: https://admin.fedoraproject.org/updates/python-sphinx10-1.0.4-3.el5

Comment 16 Fedora Update System 2010-11-11 22:25:34 UTC
python-sphinx10-1.0.4-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 17 Fedora Update System 2010-11-19 18:32:06 UTC
python-sphinx10-1.0.4-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.