Bug 1128253

Successful Koji scratch build
-----------------------------

$ koji build --scratch rawhide ../SRPMS/gerrymander-1.3-1.fc20.src.rpm 
Uploading srpm: ../SRPMS/gerrymander-1.3-1.fc20.src.rpm
[====================================] 100% 00:00:01  33.37 KiB  17.33 KiB/sec
Created task: 7257733
Task info: http://koji.fedoraproject.org/koji/taskinfo?taskID=7257733
Watching tasks (this may be safely interrupted)...
7257733 build (rawhide, gerrymander-1.3-1.fc20.src.rpm): open (buildhw-04.phx2.fedoraproject.org)
  7257736 buildArch (gerrymander-1.3-1.fc20.src.rpm, noarch): open (arm02-builder03.arm.fedoraproject.org)
  7257736 buildArch (gerrymander-1.3-1.fc20.src.rpm, noarch): open (arm02-builder03.arm.fedoraproject.org) -> closed
  0 free  1 open  1 done  0 failed
7257733 build (rawhide, gerrymander-1.3-1.fc20.src.rpm): open (buildhw-04.phx2.fedoraproject.org) -> closed
  0 free  0 open  2 done  0 failed

7257733 build (rawhide, gerrymander-1.3-1.fc20.src.rpm) completed successfully
kashyap@SPECS$

Hey Kashyap,
   Let me take this for review.

First few things to fix
1) We use %global over %define. Fix this

2) If not defined macro "%{?extra_release}" then we don't even need it. You can remove this.

3) Group tag is optional but can be kept if building for EPEL

4) Replace python with python2 wherever it applies.

5) Please have correct summary matching to package name

6) use python2 macros wherever applies

7) defattr is not needed 

8) This package is not compiling anything so we don't need
CFLAGS="$RPM_OPT_FLAGS" in %build

9) If upstream provides egginfo in source we need to remove it in %prep and package built egginfo in %install. You are removing it actually in %install

10) use source url as
Source0: https://github.com/berrange/%{name}/archive/v%{version}.tar.gz

Please provide updated package for further review and check the https://fedoraproject.org/wiki/Packaging:Python page. This package spec is not following those guidelines.

Thank you Parag, for the review, and your help on IRC. I addressed all your comments (please see inline).

Updated SPEC: https://kashyapc.fedorapeople.org/spec-files/gerrymander.spec
Updated SRPM: https://kashyapc.fedorapeople.org/srpms/gerrymander-1.3-2.fc20.src.rpm

(In reply to Parag AN(पराग) from comment #3)
> First few things to fix
> 1) We use %global over %define. Fix this

Done - Replaced %define with %global

> 2) If not defined macro "%{?extra_release}" then we don't even need it. You
> can remove this.

Removed the "%{?extra_release}" macro.

> 3) Group tag is optional but can be kept if building for EPEL

I left the "Group" tag intact, in case we may have to build this for EPEL.

> 4) Replace python with python2 wherever it applies.

Done - s/python/python2

> 5) Please have correct summary matching to package name

Corrected summary for python2 package description.

> 6) use python2 macros wherever applies

Done - used python2 macros consistently wherever it applies.
 
> 7) defattr is not needed 

Removed %defattr section from %files, as it is now implied by default.

> 8) This package is not compiling anything so we don't need
> CFLAGS="$RPM_OPT_FLAGS" in %build

Done - Removed it.

> 9) If upstream provides egginfo in source we need to remove it in %prep and
> package built egginfo in %install. You are removing it actually in %install

Done - Changed egg info removal section from %install to %prep

> 
> 10) use source url as
> Source0: https://github.com/berrange/%{name}/archive/v%{version}.tar.gz

Done.

> 
> Please provide updated package for further review and check the
> https://fedoraproject.org/wiki/Packaging:Python page. This package spec is
> not following those guidelines.

Please review the updated SPEC to see if it complies the guidelines now.

Successful Koji scratch build with the updated SPEC/SRPM:

$ koji build --scratch rawhide  /home/kashyap/rpmbuild/SRPMS/gerrymander-1.3-2.fc20.src.rpm 
Uploading srpm: /home/kashyap/rpmbuild/SRPMS/gerrymander-1.3-2.fc20.src.rpm
[====================================] 100% 00:00:02  33.54 KiB  13.79 KiB/sec
Created task: 7426618
Task info: http://koji.fedoraproject.org/koji/taskinfo?taskID=7426618
Watching tasks (this may be safely interrupted)...
7426618 build (rawhide, gerrymander-1.3-2.fc20.src.rpm): open (buildvm-15.phx2.fedoraproject.org)
  7426619 buildArch (gerrymander-1.3-2.fc20.src.rpm, noarch): open (buildhw-04.phx2.fedoraproject.org)
7426618 build (rawhide, gerrymander-1.3-2.fc20.src.rpm): open (buildvm-15.phx2.fedoraproject.org) -> closed
  0 free  1 open  1 done  0 failed
  7426619 buildArch (gerrymander-1.3-2.fc20.src.rpm, noarch): open (buildhw-04.phx2.fedoraproject.org) -> closed
  0 free  0 open  2 done  0 failed

7426618 build (rawhide, gerrymander-1.3-2.fc20.src.rpm) completed successfully

1) [!]: Package must own all directories that it creates.
     Note: Directories without known owners: /usr/lib/python3.4/site-
     packages/gerrymander/__pycache__, /usr/lib/python3.4/site-packages,
     /usr/lib/python2.7/site-packages/gerrymander, /usr/lib/python3.4/site-
     packages/gerrymander, /usr/lib/python3.4

Fix this

2) Add Requires: gerrymander to both subpackages

Thanks a lot to Parag for your comments and help on IRC.

New SPEC and SRPM with some more changes with updated files section correctly and add correct "Requires".


SPEC: https://kashyapc.fedorapeople.org/spec-files/gerrymander.spec
SRPM: https://kashyapc.fedorapeople.org/srpms/gerrymander-1.3-3.fc20.src.rpm

I think we are good now. 

APPROVED.

(In reply to Parag AN(पराग) from comment #6)
> Fix this
> 
> 2) Add Requires: gerrymander to both subpackages

That makes no sense. The subpackages exist precisely so the modules can be installed on their own without pulling in the /usr/bin command line tool.

so I can use gerrymander without installing python2-gerrymander and python3-gerrymander?

I installed gerrymander only and tried to run gerrymander
$ gerrymander
Traceback (most recent call last):
  File "/usr/bin/gerrymander", line 18, in <module>
    from gerrymander.commands import CommandTool
ImportError: No module named gerrymander.commands

(In reply to Parag AN(पराग) from comment #10)
> so I can use gerrymander without installing python2-gerrymander and
> python3-gerrymander?
> 
> I installed gerrymander only and tried to run gerrymander
> $ gerrymander
> Traceback (most recent call last):
>   File "/usr/bin/gerrymander", line 18, in <module>
>     from gerrymander.commands import CommandTool
> ImportError: No module named gerrymander.commands

That says that 'gerrymander' needs a 'Requires: python3-gerrymander' dependency, *not* that 'python[3]-gerrymander' needs a 'Requires: gerrymander' dependency.

I don't remember any package that pulls its python2 and python3 subpackage for main package. Does anyone knows any such example in Fedora?

(In reply to Parag AN(पराग) from comment #12)
> I don't remember any package that pulls its python2 and python3 subpackage
> for main package. Does anyone knows any such example in Fedora?

It is totally normal packaging practice to choose to structure things so the CLI tool is separate from the library code. It means people can simply 'yum install gerrymander' without having to know/care whether it is currently using  the python2 or python3 library modules

Addressed Dan's concern,removed the needless circular dependency of "Requires: gerrymander". 


Updated:

SPEC - https://kashyapc.fedorapeople.org/spec-files/gerrymander.spec
SRPM - https://kashyapc.fedorapeople.org/srpms/gerrymander-1.3-3.fc20.src.rpm

It makes sense for the main gerrymander package to depend *either* on python-gerrymander *or* python3-gerrymander, so that the users just install "gerrymander" and don't care about the dependencies. It however wouldn't make sense to me to install *both* python-gerrymander *and* python3-gerrymander - /usr/bin/gerrymander can only import one, anyway.
The specfile seems fine, but you should take care of the situation when with_python3 is "0" - in this situation, /usr/bin/gerrymander wouldn't work.
I'd solve it like this (incomplete example):

# in the main package
%if 0%{with_python3}
Requires: python3-gerrymander
%else
Requires: python-gerrymander
%endif

# in prep
%if 0%{with_python3}
# make sure that hashbang points to /usr/bin/python3 explicitly
%else
# make sure that hashbang points to /usr/bin/python2 explicitly
%endif

Hope this makes sense.

(In reply to Bohuslav "Slavek" Kabrda from comment #15)
> The specfile seems fine, but you should take care of the situation when
> with_python3 is "0" - in this situation, /usr/bin/gerrymander wouldn't work.
> I'd solve it like this (incomplete example):

Even simpler is to just delete the 'with_python3' conditional - all current Fedora releases have python3  so might as well just rely on it existing.

(In reply to Daniel Berrange from comment #16)
> (In reply to Bohuslav "Slavek" Kabrda from comment #15)
> > The specfile seems fine, but you should take care of the situation when
> > with_python3 is "0" - in this situation, /usr/bin/gerrymander wouldn't work.
> > I'd solve it like this (incomplete example):
> 
> Even simpler is to just delete the 'with_python3' conditional - all current
> Fedora releases have python3  so might as well just rely on it existing.

Yes, assuming the maintainer doesn't want to also put this package in EPEL and keep the same specfile that he has in Fedora.

(In reply to Bohuslav "Slavek" Kabrda from comment #17)
> (In reply to Daniel Berrange from comment #16)
> > (In reply to Bohuslav "Slavek" Kabrda from comment #15)
> > > The specfile seems fine, but you should take care of the situation when
> > > with_python3 is "0" - in this situation, /usr/bin/gerrymander wouldn't work.
> > > I'd solve it like this (incomplete example):
> > 
> > Even simpler is to just delete the 'with_python3' conditional - all current
> > Fedora releases have python3  so might as well just rely on it existing.
> 
> Yes, assuming the maintainer doesn't want to also put this package in EPEL
> and keep the same specfile that he has in Fedora.

For now, I don't have any intentions to make an EPEL package, only for Fedora. If there's strong interest for EPEL, and someone wants to co-maintain, they're more than welcome.

(In reply to Bohuslav "Slavek" Kabrda from comment #17)
> (In reply to Daniel Berrange from comment #16)
> > (In reply to Bohuslav "Slavek" Kabrda from comment #15)
> > > The specfile seems fine, but you should take care of the situation when
> > > with_python3 is "0" - in this situation, /usr/bin/gerrymander wouldn't work.
> > > I'd solve it like this (incomplete example):
> > 
> > Even simpler is to just delete the 'with_python3' conditional - all current
> > Fedora releases have python3  so might as well just rely on it existing.
> 
> Yes, assuming the maintainer doesn't want to also put this package in EPEL
> and keep the same specfile that he has in Fedora.

So do we need to create python-gerrymander or skip it? The script gerrymander points to python3 hashbang. If we think on dropping with_python3 conditionals then drop python-gerrymander also and just package gerrymander and python3-gerrymander.

(In reply to Parag AN(पराग) from comment #19)
> (In reply to Bohuslav "Slavek" Kabrda from comment #17)
> > (In reply to Daniel Berrange from comment #16)
> > > (In reply to Bohuslav "Slavek" Kabrda from comment #15)
> > > > The specfile seems fine, but you should take care of the situation when
> > > > with_python3 is "0" - in this situation, /usr/bin/gerrymander wouldn't work.
> > > > I'd solve it like this (incomplete example):
> > > 
> > > Even simpler is to just delete the 'with_python3' conditional - all current
> > > Fedora releases have python3  so might as well just rely on it existing.
> > 
> > Yes, assuming the maintainer doesn't want to also put this package in EPEL
> > and keep the same specfile that he has in Fedora.
> 
> So do we need to create python-gerrymander or skip it? The script
> gerrymander points to python3 hashbang. If we think on dropping with_python3
> conditionals then drop python-gerrymander also and just package gerrymander
> and python3-gerrymander.

That is up to you. If you think people will not want to use the python-gerrymander library with Python 2 or you don't want to maintain it, you *don't have to* provide it.
Having said that, many people have apps written in Python 2 that are still not compatible with Python 3, so providing both subpackages might help them with migration and testing their apps on both Python major versions. But again, this is your decision as a packager. You can start having just Python 3 subpackage and add Python 2 support if someone requests it.

(In reply to Parag AN(पराग) from comment #19)
> So do we need to create python-gerrymander or skip it? The script
> gerrymander points to python3 hashbang. If we think on dropping with_python3
> conditionals then drop python-gerrymander also and just package gerrymander
> and python3-gerrymander.

Of course we should keep python-gerrymander. Plenty of the world still wants to use Python2 and it is normal Fedora practice to provide libraries for both 2 & 3.

Ok, I removed the with_python3 conditionals per Dan's suggestion, and did a scratch build again (this'll generate packages: gerrymander, python-gerrymander, python3-gerrymander

Updated

SPEC - https://kashyapc.fedorapeople.org/spec-files/gerrymander.spec
SRPM - https://kashyapc.fedorapeople.org/srpms/gerrymander-1.3-4.fc20.src.rpm


Successful Koji scratch build
-----------------------------

$ koji build --scratch rawhide  /home/kashyap/rpmbuild/SRPMS/gerrymander-1.3-4.fc20.src.rpm 
Uploading srpm: /home/kashyap/rpmbuild/SRPMS/gerrymander-1.3-4.fc20.src.rpm
[====================================] 100% 00:00:02  33.73 KiB  13.10 KiB/sec
Created task: 7430651
Task info: http://koji.fedoraproject.org/koji/taskinfo?taskID=7430651
Watching tasks (this may be safely interrupted)...
7430651 build (rawhide, gerrymander-1.3-4.fc20.src.rpm): open (buildvm-15.phx2.fedoraproject.org)
  7430652 buildArch (gerrymander-1.3-4.fc20.src.rpm, noarch): open (buildvm-16.phx2.fedoraproject.org)
  7430652 buildArch (gerrymander-1.3-4.fc20.src.rpm, noarch): open (buildvm-16.phx2.fedoraproject.org) -> closed
  0 free  1 open  1 done  0 failed
7430651 build (rawhide, gerrymander-1.3-4.fc20.src.rpm): open (buildvm-15.phx2.fedoraproject.org) -> closed
  0 free  0 open  2 done  0 failed

7430651 build (rawhide, gerrymander-1.3-4.fc20.src.rpm) completed successfully

New Package SCM Request
=======================
Package Name: gerrymander
Short Description: A client API and command line tool for gerrit
Upstream URL: http://github.com/berrange/gerrymander
Owners: kashyapc
Branches: f20 21
InitialCC: berrange

Git done (by process-git-requests).

is this built on koji?

(In reply to Parag AN(पराग) from comment #25)
> is this built on koji?

My bad, missed that.

Just issued one for F21:

  http://koji.fedoraproject.org/koji/taskinfo?taskID=7917014

Please ignore the above failedbuild. 

Here's a successful build in Rawhide: http://koji.fedoraproject.org/koji/buildinfo?buildID=586833 -- gerrymander-1.3-4.fc22

------------------------------------------------
$ fedpkg build
Building gerrymander-1.3-4.fc22 for rawhide
Created task: 7917617
Task info: http://koji.fedoraproject.org/koji/taskinfo?taskID=7917617
Watching tasks (this may be safely interrupted)...
7917617 build (rawhide, /gerrymander:8e72f5b3482968b5c36473b2a9e0876e1c4f8a76): open (buildvm-08.phx2.fedoraproject.org)
  7917618 buildSRPMFromSCM (/gerrymander:8e72f5b3482968b5c36473b2a9e0876e1c4f8a76): open (arm02-builder19.arm.fedoraproject.org)
  7917618 buildSRPMFromSCM (/gerrymander:8e72f5b3482968b5c36473b2a9e0876e1c4f8a76): open (arm02-builder19.arm.fedoraproject.org) -> closed
  0 free  1 open  1 done  0 failed
  7917632 buildArch (gerrymander-1.3-4.fc22.src.rpm, noarch): open (buildhw-04.phx2.fedoraproject.org)
  7917632 buildArch (gerrymander-1.3-4.fc22.src.rpm, noarch): open (buildhw-04.phx2.fedoraproject.org) -> closed
  0 free  1 open  2 done  0 failed
  7917663 tagBuild (noarch): open (buildvm-23.phx2.fedoraproject.org)
  7917663 tagBuild (noarch): open (buildvm-23.phx2.fedoraproject.org) -> closed
  0 free  1 open  3 done  0 failed
7917617 build (rawhide, /gerrymander:8e72f5b3482968b5c36473b2a9e0876e1c4f8a76): open (buildvm-08.phx2.fedoraproject.org) -> closed
  0 free  0 open  4 done  0 failed

7917617 build (rawhide, /gerrymander:8e72f5b3482968b5c36473b2a9e0876e1c4f8a76) completed successfully
------------------------------------------------

Is this package built for requested branches? can we close this?

Yes (they're both tagged for 'updates-candidate')

F20: http://koji.fedoraproject.org/koji/buildinfo?buildID=587126
F21: http://koji.fedoraproject.org/koji/buildinfo?buildID=586839

gerrymander-1.4-1.fc21 has been submitted as an update for Fedora 21.
https://admin.fedoraproject.org/updates/gerrymander-1.4-1.fc21

gerrymander-1.4-1.fc21 has been pushed to the Fedora 21 testing repository.

gerrymander-1.4-1.fc21 has been pushed to the Fedora 21 stable repository.