Bug 630822 - Review Request: python-ansi2html - convert ansi color codes to html
Summary: Review Request: python-ansi2html - convert ansi color codes to html
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Toshio Ernie Kuratomi
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-09-07 05:36 UTC by Ralph Bean
Modified: 2015-08-24 23:54 UTC (History)
2 users (show)

Fixed In Version: python-ansi2html-0.8.3-1.fc16
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-02-28 09:57:06 UTC
Type: ---
Embargoed:
a.badger: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Ralph Bean 2010-09-07 05:36:40 UTC
Spec URL: http://www.cs.rit.edu/~rjb6296/python-ansi2html/python-ansi2html.spec
SRPM URL: http://www.cs.rit.edu/~rjb6296/python-ansi2html/python-ansi2html-0.5.1-1.fc13.src.rpm
Description: The ansi2html module can convert text with ansi color codes to HTML.

My first one(!)

Comment 1 Mark McKinstry 2010-09-15 00:01:55 UTC
Ralph,

Since this your first package, you should set FE-NEEDSPONSOR as a blocking bug. You'll want want to read the guide on how to become a package maintainer http://fedoraproject.org/wiki/PackageMaintainers/Join as well as a the package guidelines http://fedoraproject.org/wiki/Packaging/Guidelines and the ones specific to python https://fedoraproject.org/wiki/Packaging:Python

I am not a package sponsor but can help you with making your package meet the guidelines. My suggestions are:

* At the top of your spec you have macros for RHEL 3 which isn't needed for Fedora. If you're interested in making packages for RHEL, EPEL has packages for RHEL 4 and 5 but not 3. AFAIK RHEL 3 comes with python 2.2 and not python 2.3 too.

Anyways, to make this package work on Fedora <= 12 and RHEL <= 5 you can use this, which might work on RHEL 3 (https://fedoraproject.org/wiki/Packaging:Python#Macros)

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

* In the description and summary, you should capitalize ANSI in 'with ansi color codes…'

* In the install - don't test if buildroot exists, just remove it. If it exists, it will be removed - if it doesn't, no harm is done.

* In the install - you can add --skip-build and leave out --prefix=/usr

* In the files section, you need to include the LICENSE in a %doc section

* In the files section, you should change the egg section to '%if 0%{?fedora} >= 9 || 0%{?rhel} >= 6' (http://fedoraproject.org/wiki/Packaging:Python#Packaging_eggs_and_setuptools_concerns)

* In the files section you don't need to specify the directories. You can just do the following:
%{python_sitelib}/ansi2html/*.py*
%{python_sitelib}/ansi2html/templates/*.html

* You should choose one style of macros and stick with it. You switch between $RPM_BUILD_ROOT and %{buildroot}. Either one is fine as long as your consistent

Comment 2 Ralph Bean 2010-09-15 04:51:59 UTC
Mark,

That was really helpful, thanks!

I made all the changes you suggested and they make sense.

I bumped the version to 0.5.2.  Links are here:

Spec URL: http://www.cs.rit.edu/~rjb6296/python-ansi2html/python-ansi2html.spec
SRPM URL: http://www.cs.rit.edu/~rjb6296/python-ansi2html/python-ansi2html-0.5.2-1.fc13.src.rpm
Description: The ansi2html module can convert text with ansi color codes to
HTML.

Comment 3 Ralph Bean 2012-01-31 01:57:15 UTC
ansi2html has progressed from version 0.5.2 to 0.8.2 since this ticket was opened.  Here is the updated spec file and rebuilt srpm:

https://raw.github.com/ralphbean/ansi2html/master/python-ansi2html.spec
http://threebean.org/rpm/python-ansi2html-0.8.2-1.fc16.src.rpm

I just pulled off my first successful koji scratch builds for targets f16 and f17:

f16 - http://koji.fedoraproject.org/koji/taskinfo?taskID=3747600
f17 - http://koji.fedoraproject.org/koji/taskinfo?taskID=3747602

Comment 4 Toshio Kuratomi 2012-02-01 23:47:48 UTC
Good:
* Named according to the naming guidelines
* Spec file named appropriately
* License field matches upstream and is an approved open source license
* license text included
* Spec is legible
* tarball matches with upstream
* Compiles and builds in koji f16
* No locale files
* Not an ELF library
* No bundled libraries
* Not relocatable
* No duplicate files
* Permissions set appropriately
* Macros used consistently
* Code, not content
* No large doc files
* No %doc files affect the package at runtime
* Not a GUI application
* Does not own files and directories owned by other packages
* All filenames valid utf-8
* Tested that /usr/bin/ansi2html will successfully convert ansi escape
  sequences into an html document
* No scriptlets

Needswork:
* There's a testsuite so should run that in a %check section::

    BuildRequires: python-nose
    [...]
    %check
    python setup.py test

* Unowned directory: %{python_sitelib}/%{srcname}/
  You have some choices about how to fix this.  You could add::
     %dir %{python_sitelib}/%{srcname}/
  or you could let rpm recursively include things::
    %files
    %defattr(-,root,root,-)
    %doc LICENSE README.rst
    %dir %{python_sitelib}/%{srcname}*.egg-info
    %{python_sitelib}/%{srcname}*.egg-info/*
    %{python_sitelib}/%{srcname}/
    %{_bindir}/ansi2html
  or even more succinctly::
      %files
      %defattr(-,root,root,-)
      %doc LICENSE README.rst
      %{python_sitelib}/*
      %{_bindir}/ansi2html
  Up to you which style to prefer.

rpmlint output:
  - python-ansi2html.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/ansi2html/ansi2html.py
    The FSF address has changed: http://www.fsf.org/about/contact/
    Since you're upstream, you can change this in upstream's git and it will be
    reflected in our package on the next upstream release.
  - python-ansi2html.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ansi2html/style.py 0644L /usr/bin/env
    This is because the style.py file has a shebang line: #!/usr/bin/env python
    Looking at the file, it can't be executed as a script so the shebang line should be removed.
  - python-ansi2html.noarch: W: no-manual-page-for-binary ansi2html
    This is a warning only.  if you want to write a man page for this, great.
    If not, it's recommended but not required.
Of these, only the shebang removal needs to be done now.

Non-blocking:

* Not a huge deal but since you're upstream, I had to hunt to find that
  style.py was the file that made the project GPLv3+.  Everything else is
  licensed under "any version of the GPL".  You could mention that it is GPLv3+
  in the README.rst or something.
* If you're planning on building for EPEL5, you'll need to get multiprocessing reviewed
  (multiprocessing is built into python-2.6+ so EPEL6 and all non-EOL Fedora are okay).

Comment 5 Ralph Bean 2012-02-03 19:48:07 UTC
Toshio, thanks!  I've bumped this to 0.8.3 now.

spec - https://raw.github.com/ralphbean/ansi2html/master/python-ansi2html.spec
SRPM - http://threebean.org/rpm/python-ansi2html-0.8.3-1.fc16.src.rpm

Updated:

* The spec now includes a %check section
* I took the third of the three options you suggested to resolve the directory
  ownership issues.
* The license ambiguity was resolved upstream.
* Shebang was removed from style.py

Still an issue?

* I can't get rpmlint to produce the same output you listed.  It reports to
  me that everything is fine.  Can you show me how you're running it?

Other:

* No plans yet to release for EPEL5.

Comment 6 Toshio Ernie Kuratomi 2012-02-03 21:52:20 UTC
APPROVED

All issues I noted taken care of.  I've sponsored you into packager as well :-)

rpmlint will analyze several different things -- spec files, srpms, rpms, and installed packages.  The warnings and errors I reported were from running "rpmlint *rpm" and came from the noarch.rpm.  The same errors were found running "rpmlint python-ansi2html" after the package was installed.

Comment 7 Toshio Ernie Kuratomi 2012-02-03 21:59:51 UTC
This will be your next check:

http://fedoraproject.org/wiki/Package_SCM_admin_requests

Comment 8 Toshio Ernie Kuratomi 2012-02-03 22:03:57 UTC
s/check/step/

Comment 9 Ralph Bean 2012-02-03 22:17:14 UTC
New Package SCM Request
=======================
Package Name: python-ansi2html
Short Description: convert text with ansi color codes to HTML.
Owners: ralph
Branches: f16 f17
InitialCC:

Comment 10 Gwyn Ciesla 2012-02-06 13:16:01 UTC
Git done (by process-git-requests).

f17==devel for another few days.

Comment 11 Fedora Update System 2012-02-06 19:22:58 UTC
python-ansi2html-0.8.3-1.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/python-ansi2html-0.8.3-1.fc16

Comment 12 Fedora Update System 2012-02-07 07:57:54 UTC
python-ansi2html-0.8.3-1.fc16 has been pushed to the Fedora 16 testing repository.

Comment 13 Fedora Update System 2012-02-28 09:57:06 UTC
python-ansi2html-0.8.3-1.fc16 has been pushed to the Fedora 16 stable repository.


Note You need to log in before you can comment on or make changes to this bug.