Bug 452321

Tom,

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

Devrim

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.

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.

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.

Ok, fair enough. I'll review this package.

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.)

I haven't looked at this since it was submitted, so problems don't surprise me ... what didn't work?

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.

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.

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.

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.

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.

Yes, please; I don't know of a reason why that shouldn't still work.

Tom, 4.0 was released -- maybe it is the right time to push that version instead of 3.8.1 .

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.

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.

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.

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.

Just FYI, I'm leaving this blocked against FE-Legal until the License tag is corrected (MIT is correct).

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?

That license tag looks fine to me.

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.

Lifting FE-Legal.

[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,

Yes, I made the %define->%global change in my local copy yesterday.  Thanks for the review!

New Package CVS Request
=======================
Package Name: PyGreSQL
Short Description: Python client library for PostgreSQL
Owners: tgl
Branches: F-13 F-12
InitialCC:

cvs done (with devel and F-12 branches).