Bug 878612

The provides and obsoletes need to be versioned: http://fedoraproject.org/wiki/Packaging:Guidelines#Renaming.2FReplacing_Existing_Packages

(In reply to comment #1)
> The provides and obsoletes need to be versioned:
> http://fedoraproject.org/wiki/Packaging:Guidelines#Renaming.
> 2FReplacing_Existing_Packages

Thanks. I've now fixed this.

By the way, should I bump the release number when I make a change to the spec like this one, or should I just upload new files with the same release number?

You should bump the release number.

Review based on the current version:
ab24557ef52c1bc6ff79f81408d9357888dfa55a4d90657576822c24c36f077e  pexpect-u-2.5.1.tar.gz

Good:
* Source matches upstream and source url is canonical
* Package named according to the naming guidelines
* spec file matches package name
* License is MIT in the spec file and files distributed in the tarball
* LICENSE file included
* Spec is legible
* No locale files shipped
* Not an ELF shared library
* Macros used consistently
* Contains code, not conent
* No large doc
* Not a GUI app, no .desktop file

Cosmetic:
* Can change python-devel to python2-devel
* No need for -O1 in the build and install lines (brp-python-bytecompile will (re)do that)

Needswork:
* Change source http://pypi.python.org/packages/source/p/pexpect-u/pexpect-u-[..]
  to Source0: http://pypi.python.org/packages/source/p/pexpect-u/pexpect-u-[..]
  for consistency with other packages and so that spectool -g works  (notice the
  correction of missing ":" after source)
* %if (0%{?fedora} > 15 || 0%{?rhel} > 5) => should remove the || 0%{?rhel} > 5 portion because
  rhel6 doesn't have python3 and we've been told that we're not sure what the
  story for rhel7 is going to be.
* Tests are shipped with the package.  They should be run.  This can be done by
  BuildRequiring python-nose (python3-nose for the pythn3 subbpackage) and
  running nosetests (nosetests-3.2 for the python3 subpackage)
* Description section in the python3 subpackage should say this is for python3.
  Easiest way is just to tack on a one sentence paragraph that says:
  "This package contains the python3 version of this module"
* The comment in the %install section about doing the python3 install first is
  misleading -- there aren't any scripts in %{_bindir} to worry about.  Unless
  there is a reason that order matters here my suggestion is simply to remove
  the comment.

To Check still:
* rpmlint
* builds in koji
* Permissions set properly
* Package owns all files and directories that it creates and nothing more
* All filenames valid utf-8

(In reply to comment #3)
> You should bump the release number.

This is now done, and the new source RPM is at: http://aml.cs.byu.edu/~amcnabb/python-pexpect-2.5.1-2.fc17.src.rpm

Provides is correct in the new package.  Obsoletes: needs to be a little higher.

The latest version that I see in koji is: pexpect-2.3-8.fc18

So the Obsoletes should be:

Obsoletes: pexpect <= 2.3-9

(In reply to comment #4)
> Review based on the current version:
> ab24557ef52c1bc6ff79f81408d9357888dfa55a4d90657576822c24c36f077e 
> pexpect-u-2.5.1.tar.gz
> 
> Cosmetic:
> * Can change python-devel to python2-devel

Done.

> * No need for -O1 in the build and install lines (brp-python-bytecompile
> will (re)do that)

Done.

> Needswork:
> * Change source
> http://pypi.python.org/packages/source/p/pexpect-u/pexpect-u-[..]
>   to Source0:
> http://pypi.python.org/packages/source/p/pexpect-u/pexpect-u-[..]
>   for consistency with other packages and so that spectool -g works  (notice
> the
>   correction of missing ":" after source)

Done.

> * %if (0%{?fedora} > 15 || 0%{?rhel} > 5) => should remove the || 0%{?rhel}
> > 5 portion because
>   rhel6 doesn't have python3 and we've been told that we're not sure what the
>   story for rhel7 is going to be.

Done.

> * Tests are shipped with the package.  They should be run.  This can be done
> by
>   BuildRequiring python-nose (python3-nose for the pythn3 subbpackage) and
>   running nosetests (nosetests-3.2 for the python3 subpackage)

Done.

> * Description section in the python3 subpackage should say this is for
> python3.
>   Easiest way is just to tack on a one sentence paragraph that says:
>   "This package contains the python3 version of this module"

Done.

> * The comment in the %install section about doing the python3 install first
> is
>   misleading -- there aren't any scripts in %{_bindir} to worry about. 
> Unless
>   there is a reason that order matters here my suggestion is simply to remove
>   the comment.

Done.

(In reply to comment #6)
> So the Obsoletes should be:
>
> Obsoletes: pexpect <= 2.3-9

Done.

The new source RPM is at:
http://aml.cs.byu.edu/~amcnabb/python-pexpect-2.5.1-3.fc17.src.rpm

Good:
* previous comments are fixed

Needswork:
* running unittests shold be moved to a %check section:
%check

PYTHONSTARTUP="" nosetests

%if 0%{?with_python3}
pushd %{py3dir}
pushd build/lib
PYTHONSTARTUP="" nosetests-%{python3_version}

popd
popd
%endif # with_python3

* Note that I also found that the nosetests for python3 has to be invoked using that macro to determine which version of python3 is on the system.  F17 has python3-3.2; F19 and F18 have python3-3.3.

* I tried building in koji with the above %check section but there's one failing unittest (on python2).  You can see it here:
  http://kojipkgs.fedoraproject.org//work/tasks/761/4710761/build.log

  This also fails locally when I try to build it on F17 so it looks like a valid  bug.

* I reversed the order of the nosetest runs and found a different test failing on python3:
http://kojipkgs.fedoraproject.org//work/tasks/877/4710877/build.log

Looks like a BuildRequires: ed is needed.

Still to check:
* rpmlint
* Permissions set properly
* Package owns all files and directories that it creates and nothing more
* All filenames valid utf-8

(In reply to comment #8)
> 
> Needswork:
> * running unittests shold be moved to a %check section:
> %check

Done.

> * Note that I also found that the nosetests for python3 has to be invoked
> using that macro to determine which version of python3 is on the system. 
> F17 has python3-3.2; F19 and F18 have python3-3.3.

Done.


> * I tried building in koji with the above %check section but there's one
> failing unittest (on python2).  You can see it here:
>   http://kojipkgs.fedoraproject.org//work/tasks/761/4710761/build.log
> 
>   This also fails locally when I try to build it on F17 so it looks like a
> valid  bug.
>
> * I reversed the order of the nosetest runs and found a different test
> failing on python3:
> http://kojipkgs.fedoraproject.org//work/tasks/877/4710877/build.log
> 
> Looks like a BuildRequires: ed is needed.

It looks like the problem is ed in both cases.

Added ed to BuildRequires.

The new source RPM is at:
http://aml.cs.byu.edu/~amcnabb/python-pexpect-2.5.1-4.fc17.src.rpm

The RPM is now successfully building in koji without any errors:

http://koji.fedoraproject.org/koji/taskinfo?taskID=4711309
http://koji.fedoraproject.org/koji/taskinfo?taskID=4711308

Good:
* Former problems taken care of
* Builds in koji
* Permissions set properly
* Package owns all files and directories that it creates and nothing more
* All filenames valid utf-8

rpmlint

* python3-pexpect.noarch: W: spelling-error %description -l en_US containts -> contains, con taints, con-taints

  Valid -- change to contains

* python3-pexpect.noarch: W: spelling-error %description -l en_US passwd -> passed, password

  False positive.  We're talking about /usr/bin/passwd here

python3-pexpect.noarch: W: spelling-error %description -l en_US pty -> pry, pt, pity

  False positive.

python3-pexpect.noarch: E: non-executable-script /usr/lib/python3.3/site-packages/pexpect/FSM.py 0644L /usr/bin/env

  This one is a python module that happens to have a shebang line and some code that can be executed if the module were run like a script.  However, that output is just an example of how to write code against the module's API -- it's not meant to actually be used on the system.  So this can be ignored.

* python-pexpect.noarch: E: zero-length /usr/lib/python2.7/site-packages/pexpect/tests/forwhich.sh
  python-pexpect.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/pexpect/tests/test_timeout_pattern.py 0644L /usr/bin/env
  [other non-executable scripts in the tests subdir]

  The tests aren't actually imported and used by the module.  Makes sense to just remove these from the built rpm like this:

  %files
  [..]
  %exclude %{python_sitelib}/pexpect/tests/

  %files -n python3-pexpect
  [..]
  %exclude %{python3_sitelib}/pexpect/tests/

4 packages and 1 specfiles checked; 29 errors, 9 warnings.

You can fix the one spelling error and add the %exclude's for the tests before you import this into git.

Package is APPROVED.

(In reply to comment #11)
> * python3-pexpect.noarch: W: spelling-error %description -l en_US containts
> -> contains, con taints, con-taints
> 
>   Valid -- change to contains

Done.

> python3-pexpect.noarch: E: non-executable-script
> /usr/lib/python3.3/site-packages/pexpect/FSM.py 0644L /usr/bin/env
> 
>   This one is a python module that happens to have a shebang line and some
> code that can be executed if the module were run like a script.  However,
> that output is just an example of how to write code against the module's API
> -- it's not meant to actually be used on the system.  So this can be ignored.

Agreed.

> * python-pexpect.noarch: E: zero-length
> /usr/lib/python2.7/site-packages/pexpect/tests/forwhich.sh
>   python-pexpect.noarch: E: non-executable-script
> /usr/lib/python2.7/site-packages/pexpect/tests/test_timeout_pattern.py 0644L
> /usr/bin/env
>   [other non-executable scripts in the tests subdir]
> 
>   The tests aren't actually imported and used by the module.  Makes sense to
> just remove these from the built rpm like this:
> 
>   %files
>   [..]
>   %exclude %{python_sitelib}/pexpect/tests/
> 
>   %files -n python3-pexpect
>   [..]
>   %exclude %{python3_sitelib}/pexpect/tests/

Done.

> You can fix the one spelling error and add the %exclude's for the tests
> before you import this into git.
> 
> Package is APPROVED.

Thanks for the helpful (and extremely fast) review.

New Package SCM Request
=======================
Package Name: python-pexpect
Short Description: Unicode-aware Pure Python Expect-like module
Owners: amcnabb
Branches: f18
InitialCC:

Git done (by process-git-requests).

python-pexpect-2.5.1-5.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/python-pexpect-2.5.1-5.fc18

python-pexpect-2.5.1-5.fc18 has been pushed to the Fedora 18 stable repository.

Needed for my epel7 cinnamon port.

Package Change Request
======================
Package Name: python-pexpect
New Branches: epel7
Owners: leigh123linux

No epel7 branches yet.

Needed for my epel7 cinnamon port.

Package Change Request
======================
Package Name: python-pexpect
New Branches: epel7
Owners: leigh123linux

Any comment from the Fedora maintainers?

(In reply to Jon Ciesla from comment #20)
> Any comment from the Fedora maintainers?

Nope--everything looks good.

By the way, if the EPEL maintainers want to be added as Fedora maintainers, too, that would be great.

Git done (by process-git-requests).