Bug 586777

As already discussed on IRC, it would be better to make a library here:

I think this could be usefull for other packages too, so other could simply import this and then work with it.

So you could install it in %{python3_sitelib} and set up a starter script (I'm sure upstream will incorporate it).

e.g.:

#!/usr/bin/python3
from glyphtracer import start_program

start_program()


(Currently untested, but should work)

(By the way, is this python3 or python2, you BR python3, but it starts with python2...)

Thanks Che and Tomspur for your feedback.

I have updated package now
Spec URL: http://paragn.fedorapeople.org/fedora-work/SPECS/glyphtracer.spec
SRPM URL:
http://paragn.fedorapeople.org/fedora-work/SRPMS/glyphtracer-0-0.1.r69.fc13.src.rpm

Can anyone please review this package?

The spec file should have a dependency on potrace. Also, there is an actual 1.0 release, you should probably use that rather than a bzr checkout.

jpakkane,
   Thanks I have used upstream released 1.0 release. I also have request if you can add setup.py in upstream then it will be easy for installation of this program.

Spec URL: http://paragn.fedorapeople.org/fedora-work/SPECS/glyphtracer.spec
SRPM URL:
http://paragn.fedorapeople.org/fedora-work/SRPMS/glyphtracer-1.0-1.fc13.src.rpm

Now it has a setup.py.

Thanks. I have updated again package.

Spec URL: http://paragn.fedorapeople.org/fedora-work/SPECS/glyphtracer.spec
SRPM URL: http://paragn.fedorapeople.org/fedora-work/SRPMS/glyphtracer-0-0.1.r72.fc13.src.rpm

I have kept wrapper file to be installed in /usr/bin as its easy way to generate ttf file from a given image.

Spec URL: http://paragn.fedorapeople.org/fedora-work/SPECS/glyphtracer.spec
SRPM URL: http://paragn.fedorapeople.org/fedora-work/SRPMS/glyphtracer-1.0-1.r73bzr.fc13.src.rpm

anyone to review this? I can review your package in exchange of this review.

I'll trade for python-bunch: https://bugzilla.redhat.com/show_bug.cgi?id=575185

GOOD:
* Source matches upstream
* builds in koji for f13 --> See notes in NEEDSWORK for other releases
* license is correct and approved
* spec file is readable
* No locale files to handle
* not a shared library
* No bundled libraries
* package owns all files and directories it creates and nothing else
* no file listed multiple times in %files
* Permissions are proper
* macros used consistently
* no doc files affect runtime

NEEDSWORK
* This is a post-release snapshot so you have the release almost correct but it
  should include the date like this::
    1.$DATEbzr$BZRbzr%{?dist}

  So like this::
    2.20100507bzr73

* Better method to generate the source::
    ### Source tarball is created from latest 73 revision
    ### bzr branch lp:glyphtracer -r 73
    ### cd glyphtracer
    ### python setup.py sdist
    ### tarball is in dist/glyphtracer-%{version}.tar.gz
    Source0: glyphtracer-%{version}.tar.gz

  The reason this is better is that this is the way most upstream's create
  their tarballs.  So making our tarball this way will let us see problems they
  would when they make their next release.

  (And hello jpakkane!  In case you didn't know about the setup.py sdist
  command, there it is :-)

  When making this change, you also need to change the %setup line:
    - %setup -q -n %{name}
    + %setup -q glyphtracer-%{version}

* Need to Requires: potrace

* If you are building for anything less recent than F13 you need to define
  python_sitelib at the top of your spec file like this::

    %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())")}
    %endif

* This is a GUI app so it needs a .desktop file.

RPMLINT:
glyphtracer.src: W: spelling-error %description -l en_US vectorised -> vectored, vector, verdigrised
glyphtracer.noarch: W: spelling-error %description -l en_US vectorised -> vectored, vector, verdigrised

  Both vectorize and vectorise are in common use.  No need to change this.

glyphtracer.src: W: invalid-url Source0: glyphtracer-1.0-73bzr.tar.gz

  The comments explain how to generate this so it's also fine.

glyphtracer.noarch: E: non-executable-script /usr/lib/python2.6/site-packages/glyphtracer.py 0644 /usr/bin/python
glyphtracer.noarch: E: non-executable-script /usr/lib/python2.6/site-packages/gtlib.py 0644 /usr/bin/python

  These are being generated because of the #!/usr/bin/python lines in gtlib.py
  and glyphtracer.py.  I'd remove the line from gtlib.py and submit upstream
  because there is no code there intended to be run as a script.
  glyphtracer.py is tougher -- it does have an if __name__ == '__main__'
  section that allows you to execute the program as a script.

  Several ways to resolve this:
  * Decide that glyphtracer.py isn't useful as a library.  In that case i'd add it to the setup.py as a script and install it to /usr/bin.  Renaming glyphtracer.py to glyphtracer and making this change to setup.py would do that::
    - py_modules = ['glyphtracer', 'gtlib'],
    + py_modules = ['gtlib'],
    + scripts = ['glyphtracer'],

  * Decide that in the future glyphtracer.py will be used as a library too --
    I'd remove the if __name__ == '__main__' section from glyphtracer.py and
    add the wrapper script (present in the srpm) to the upstream distribution
    in the scripts  line of setup.py.

OTHER (none of these are necessary changes):
* Note for jpakkane: usually, people include a copy of the GPL (v3 since that's
  what you are licensing under) in the tarball.  The funny thing about the GPL
  is that if the software author doesn't specify a version explicitly anywhere,
  the recipient is officially allowed to use any version of the license (even
  though the author likely meant GPLv3 or later or GPLv2 or later).  So your
  explitness in license.txt is also good to have.  Thanks for that!  I'd
  recommend keeping license.txt and adding a COPYING file with the text of the
  GPLv3.

* -O1 isn't needed with any current versions of rpm in Fedora; the python
  bytecompilation step will do -O1 for us.

* When writing python scripts I like to use::
    #!/usr/bin/python -tt

  The -tt makes it illegal to mix spaces and tabs in a single source file.
  That's nice to do since different people have different settings for width of
  tabs.

* The package may be misnamed.  Now that glyphtracer is being used as a library
  we may want to name the package python-glyphtracer.  This is somewhat vague
  though.  Reading the source, it looks like glyphtracer.py is primarily for
  supporting /usr/bin glyphtracer... it would be hard to implement a different
  front end using the classes and methods that glyphtracer.py provides.  With
  that in mind naming the package glyphtracer is fine since it's primary
  purpose is to support the glyphtracer application.  If gtlib.py (or
  glyphtracer.py with API changes) becomes used by other python programs we may
  want to rethink that at that time.

* Should: no man page for glyphtracer

Grr typo:
* This is a post-release snapshot so you have the release almost correct but it
  should include the date like this::
    1.$DATEbzr$BZR%{?dist}

  So like this::
    2.20100507bzr73%{?dist}

I don't plan on providing any sort of API stability in gtlib or glyphtracer. Thus I would not recommend importing them to anyone else.

jpakkane, Do these changes look good?

  bzr branch lp:~toshio/glyphtracer/misc-packaging-changes

I have merged your changes to trunk. Thanks.

Created attachment 412706 [details]
spec file update taking care of remaining must issues

Here's a spec file patch that works with the current snapshot to resolve all remaining blockers.  The only thing it lacks is the SHOULD to include a man page.  I don't think that glyphtracer has any non-QT commandline options, though, so I certainly won't block on that.

Thanks Toshio for your detailed review and patch for upstream.

Thanks jpakkane for committing Toshio's patch in upstream.

I have again pulled current bzr code and here is new package

SPEC URL: http://paragn.fedorapeople.org/fedora-work/SPECS/glyphtracer.spec
SRPM URL: http://paragn.fedorapeople.org/fedora-work/SRPMS/glyphtracer-1.1-1.20100510bzr76.fc13.src.rpm

Looks Mid-air collision. But anyway I still submitted my comment.

All issues taken care of.

APPROVED.

Thanks Toshio!

New Package CVS Request
=======================
Package Name: glyphtracer
Short Description: Program for creating fonts from images
Owners: pnemade
Branches: F-12 F-13 EL-5
InitialCC:

cvs done

Thanks Huzaifas!

Built for all Fedora branches.

glyphtracer-1.1-1.20100510bzr76.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/glyphtracer-1.1-1.20100510bzr76.fc13

glyphtracer-1.1-1.20100510bzr76.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/glyphtracer-1.1-1.20100510bzr76.fc12

I am unable to build this for EPEL5. Can anyone help me what changes I need to make this program working for python 2.4.3?

See failed scratch build http://koji.fedoraproject.org/koji/taskinfo?taskID=2176307

Does changing class definitions from:
    class Foo():

to:
    class Foo(object):

work?

Thanks again Toshio!

It worked fine. Also need to add --vendor to desktop-file-install and scratch  build is successful at http://koji.fedoraproject.org/koji/taskinfo?taskID=2176848

I have done the same fix in trunk.

glyphtracer-1.1-1.20100510bzr76.fc12 has been pushed to the Fedora 12 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 glyphtracer'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/glyphtracer-1.1-1.20100510bzr76.fc12

glyphtracer-1.1-1.20100510bzr76.fc13 has been pushed to the Fedora 13 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 glyphtracer'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/glyphtracer-1.1-1.20100510bzr76.fc13

glyphtracer-1.1-1.20100510bzr76.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.

glyphtracer-1.1-1.20100510bzr76.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.