Bug 452321 - Review Request: PyGreSQL - Python client library for PostgreSQL
Review Request: PyGreSQL - Python client library for PostgreSQL
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Rakesh Pandit
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-06-20 17:48 EDT by Tom Lane
Modified: 2013-07-02 23:18 EDT (History)
6 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-01-09 00:24:50 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
rpandit: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Tom Lane 2008-06-20 17:48:53 EDT
Spec URL: http://tgl.fedorapeople.org/PyGreSQL.spec
SRPM URL: http://tgl.fedorapeople.org/PyGreSQL-3.8.1-1.fc10.src.rpm
Description:
The PyGreSQL package provides a module for developers to use when writing
Python code for accessing a PostgreSQL database.

This is actually not a new package: we have been shipping it for a long time as part of the postgresql
SRPM.  However, the upstream project was split off from core postgresql quite some time ago, and
now has its own tarball name and version numbering.  To track this properly we need to split out
PyGreSQL as a separate SRPM.  I've also taken the opportunity to try to make the packaging conform to
current guidelines for python modules, which the present postgresql-python package is rather far away from.  Once this package is accepted I will remove PyGreSQL from the postgresql SRPM.
Comment 1 Devrim GUNDUZ 2009-01-01 16:42:53 EST
Tom,

PyGreSQL 4.0 was released. Would you like to update the spec file first? I'm willing to review this now.

Devrim
Comment 2 Tom Lane 2009-01-01 17:33:17 EST
The purpose of this change is to split out what we are already shipping in the postgresql RPM.  I don't want to confuse the issue by folding a major upstream version bump into the same update.
Comment 3 Devrim GUNDUZ 2009-01-01 17:48:21 EST
Ok, but this package will go to -devel. I *think* Fedora 11 *may* ship with PostgreSQL 8.4, so this won't probably hurt people -- and it may be the correct time for upgrading to newer PyGreSQL.
Comment 4 Tom Lane 2009-01-01 18:33:45 EST
You miss my point. I'm not against updating to pygresql 4.0 --- I'm against doing it as part of the package split.  Once pygresql is in as a separate SRPM I'll be glad to take a look at that.
Comment 5 Devrim GUNDUZ 2009-01-01 18:46:22 EST
Ok, fair enough. I'll review this package.
Comment 6 Jason Tibbitts 2009-07-31 22:19:00 EDT
Wow, that didn't exactly work.  Tom, is the above package still what you'd like to have reviewed?  (Probably not, over 13 months later.)
Comment 7 Tom Lane 2009-07-31 23:41:06 EDT
I haven't looked at this since it was submitted, so problems don't surprise me ... what didn't work?
Comment 8 Jason Tibbitts 2009-07-31 23:46:43 EDT
Sorry, what didn't work was Devrim's offer to review in comment #5.  I haven't even tried to build the package; just wanted to check to see if you still wanted the original version reviewed or if an updated package would be necessary before a review could start.
Comment 9 Tom Lane 2009-07-31 23:56:16 EDT
As I said in an earlier comment, I'd prefer to get this split out of the postgresql SRPM first, and then worry about version updates.  postgresql is still building, so I *think* this ought to still work, if I didn't blow it too badly in constructing the new specfile.
Comment 10 Jason Tibbitts 2009-08-01 00:08:58 EDT
Well, sure.  What I'm asking is if the version originally posted actually matches what you want reviewed.  I have no idea if the postgreql package that this is split from has actually changed in the 13 months since the PyGreSQL package was generated, but I'm assuming that it has.
Comment 11 Tom Lane 2009-08-01 00:31:02 EDT
Postgresql has changed.  And there is a new version of PyGreSQL, as noted in comment #1.  What has not changed in awhile is the version of PyGreSQL we are (improperly) embedding in the postgresql SRPM.  I would like to get that fixed before there is any discussion of version updates.
Comment 12 Jason Tibbitts 2009-08-01 00:52:30 EDT
Let me ask in a different manner.  Is this what you want reviewed:

Spec URL: http://tgl.fedorapeople.org/PyGreSQL.spec
SRPM URL: http://tgl.fedorapeople.org/PyGreSQL-3.8.1-1.fc10.src.rpm

?

That's all I've been trying to ascertain.
Comment 13 Tom Lane 2009-08-01 01:07:14 EDT
Yes, please; I don't know of a reason why that shouldn't still work.
Comment 14 Devrim Gündüz 2009-08-01 02:36:45 EDT
Tom, 4.0 was released -- maybe it is the right time to push that version instead of 3.8.1 .
Comment 15 Jason Tibbitts 2009-08-01 11:19:52 EDT
I don't know how many times he can say that he doesn't want to worry about updating things until after this package is split out.

This does still build fine; here's the rpmlint  output:

  PyGreSQL.x86_64: W: spurious-executable-perm
   /usr/share/doc/PyGreSQL-3.8.1/tutorial/basics.py
  PyGreSQL.x86_64: W: spurious-executable-perm
   /usr/share/doc/PyGreSQL-3.8.1/tutorial/advanced.py
  PyGreSQL.x86_64: W: doc-file-dependency
   /usr/share/doc/PyGreSQL-3.8.1/tutorial/basics.py /usr/bin/env
  PyGreSQL.x86_64: W: doc-file-dependency
   /usr/share/doc/PyGreSQL-3.8.1/tutorial/advanced.py /usr/bin/env
Generally documentation is not made executable.  If there's something that people need to be able to execute, it should be installed in the usual location for binaries.  If it's just there to look at, there's no reason for it to be executable.   It doesn't really hurt anything as long as it doesn't add dependencies that aren't present in the rest of the package, but in this case it does.  Of course, that dependency is in coreutils, so it's still not a huge idea, but I'd fix it.

  PyGreSQL.x86_64: E: non-executable-script
   /usr/lib64/python2.6/site-packages/pg.py 0644 /usr/bin/env
  PyGreSQL.x86_64: E: non-executable-script /usr/lib64/python2.6/site-packages
   /pgdb.py 0644 /usr/bin/env
For whatever reason, python programmers like to put #! lines even in files that aren't intended to be run.  Some people like to fix these up; I don't particularly care.

You indicate License: BSD, and docs/readme.txt says "BSD license", but setup.py says license = "Python" and the only license text I can find in the package is actually the MIT license (specifically http://fedoraproject.org/wiki/Licensing/MIT#Old_Style_with_legal_disclaimer_4).  pgdb.py says "See package documentation for further information on copyright."  pg.py has no licensing information.

I believe the situation is sufficiently confusing that upstream should be consulted.  Perhaps the license on the new version is clearer.
Comment 16 Tom Lane 2009-08-05 18:54:33 EDT
Hm.  The license = "Python" bit definitely seems wrong, and it hasn't changed in 4.0, so I'll poke upstream about it.

As for the MIT vs BSD part, I think you've opened a bit of a can of worms.  The reason this package is labeled BSD is that it split off from PostgreSQL a long time ago, and it has the same license wording as PostgreSQL, which the upstream group for that project has always considered to be BSD-without-advertising.  Not only because it came from Berkeley originally, but also because it once had an advertising clause, which UCB later allowed them (us) to remove.  I am quite confused by the wiki's attribution of this wording to MIT, and frankly I'm not sure I believe it's authoritative.  Particularly not when all their examples cite UCB not MIT.

In short: if you want me to change this, you're going to have to persuade a large number of people that what they always thought was BSD wording is MIT wording.  And a wiki page with these examples on it *ain't* going to do the trick.
Comment 17 Jason Tibbitts 2009-08-05 23:21:16 EDT
I'm not one to argue with the legal folks on the license thing, but you're welcome to.  Blocking FE-Legal so they can take a look.  Maybe that page in the wiki is incorrect; I'm no expert in that area, but I'm also not just going to ignore what it says and approve this package.
Comment 18 Tom Lane 2009-08-06 00:02:43 EDT
Hm, I think you missed the point of what I was saying: if the license label on this package is wrong, then so is the label on the postgresql package, which we've been shipping for ten years or so.  I'm not sure of the point of holding up approval of this package while that gets straightened out.
Comment 19 Tom "spot" Callaway 2009-10-06 10:47:52 EDT
Just FYI, I'm leaving this blocked against FE-Legal until the License tag is corrected (MIT is correct).
Comment 20 Tom Lane 2009-11-24 11:40:19 EST
I checked with the author and it seems that his intent is actually to dual-license under both the postgres license and Python's.  Hopefully the next upstream release will make that a bit more clear, but it seems that in the meantime the appropriate tag is "License: MIT or Python".  Is this sufficient or do I need more documentation about it?
Comment 21 Tom "spot" Callaway 2009-11-24 11:46:52 EST
That license tag looks fine to me.
Comment 22 Tom Lane 2009-11-24 16:09:33 EST
I have updated the submission here:

Spec URL: http://tgl.fedorapeople.org/PyGreSQL.spec
SRPM URL: http://tgl.fedorapeople.org/PyGreSQL-3.8.1-2.fc13.src.rpm

This changes the License tag and makes all the scripts under tutorial non-executable, as per discussion.
I did not do anything about the permissions on the scripts under site-packages/ --- AFAICS, if you don't like those you ought to file a bug against the python build system.
Comment 23 Tom "spot" Callaway 2009-11-24 16:27:29 EST
Lifting FE-Legal.
Comment 24 Rakesh Pandit 2010-01-07 07:59:22 EST
[x] - Ok, [-] Needs input, [na] - Not Applicable

[x] http://koji.fedoraproject.org/koji/taskinfo?taskID=1907408 - Builds fine
[x] rpmlint messages: fine

Rpmlint Messages:
PyGreSQL.x86_64: E: non-executable-script /usr/lib64/python2.6/site-packages/pg.py 0644 /usr/bin/env
PyGreSQL.x86_64: E: non-executable-script /usr/lib64/python2.6/site-packages/pgdb.py 0644 /usr/bin/env

Can be ignored

[x] Source original

Upstream:
[rakesh@simu review]$ sha1sum PyGreSQL-3.8.1.tgz 
341d14beb99e88f779657db3ce146b41c897f975  PyGreSQL-3.8.1.tgz

SRPM:
[rakesh@simu review]$ sha1sum PyGreSQL-3.8.1-2.fc13.src/PyGreSQL-3.8.1.tgz 
341d14beb99e88f779657db3ce146b41c897f975  PyGreSQL-3.8.1-2.fc13.src/PyGreSQL-3.8.1.tgz

[x] Package Name: fine (matches with upstream)
[x] License fine (may you just request upstream (yourself ;) to just add a bit explanation in some README file and provide license txt)
[x] Spec name fine
[x] Spec is legible and written in American English
[x] Package successfully builds on primary archs
[x] Debuginfo file seems to be fine
[x] Permissions of files file
[x] All folders which are created are owned correctly
[x] Docs are present and appropriately kept
[x] Files names in package valid UTF-8
[x] python-devel present
[x] provides and obsoletes seem to be fine
[-] In python_sitearch definition use %global and not %define (deprecated)
[x] Python eggs build from source
[x] byte compiled files included

Summary:
Just check one point with [-]. Everything seems to be fine. Before importing this may you make sure that you have removed PyGreSQL from the postgresql main package and corresponding updated to latest.

APPROVED

Thanks,
Comment 25 Tom Lane 2010-01-07 09:03:20 EST
Yes, I made the %define->%global change in my local copy yesterday.  Thanks for the review!
Comment 26 Tom Lane 2010-01-07 09:08:57 EST
New Package CVS Request
=======================
Package Name: PyGreSQL
Short Description: Python client library for PostgreSQL
Owners: tgl
Branches: F-13 F-12
InitialCC:
Comment 27 Kevin Fenzi 2010-01-08 23:35:48 EST
cvs done (with devel and F-12 branches).

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