Bug 630822

Summary: Review Request: python-ansi2html - convert ansi color codes to html
Product: [Fedora] Fedora Reporter: Ralph Bean <rbean>
Component: Package ReviewAssignee: Toshio Ernie Kuratomi <a.badger>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: low    
Version: rawhideCC: fedora-package-review, notting
Target Milestone: ---Flags: a.badger: fedora‑review+
limburgher: fedora‑cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: python-ansi2html-0.8.3-1.fc16 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2012-02-28 04:57:06 EST Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Description Ralph Bean 2010-09-07 01:36:40 EDT
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-14 20:01:55 EDT
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 00:51:59 EDT
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-30 20:57:15 EST
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 18:47:48 EST
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 14:48:07 EST
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 16:52:20 EST
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 16:59:51 EST
This will be your next check:

http://fedoraproject.org/wiki/Package_SCM_admin_requests
Comment 8 Toshio Ernie Kuratomi 2012-02-03 17:03:57 EST
s/check/step/
Comment 9 Ralph Bean 2012-02-03 17:17:14 EST
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 Jon Ciesla 2012-02-06 08:16:01 EST
Git done (by process-git-requests).

f17==devel for another few days.
Comment 11 Fedora Update System 2012-02-06 14:22:58 EST
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 02:57:54 EST
python-ansi2html-0.8.3-1.fc16 has been pushed to the Fedora 16 testing repository.
Comment 13 Fedora Update System 2012-02-28 04:57:06 EST
python-ansi2html-0.8.3-1.fc16 has been pushed to the Fedora 16 stable repository.