Bug 591730

*** Bug 591731 has been marked as a duplicate of this bug. ***

Broadly-speaking, this looks like a sane packaging of pyjamas.

A few nits:

> # NOTE: we only byte-compile the library files. 
> # This is because: 
> # DO NOT install the pyjamas libraries into /usr/share/python-support,
> # /usr/lib/python2.N/site-packages or ANY location where the "standard"
> # python interpreter could possibly pick them up.  The pyjamas libraries
> # have nothing to do with python libraries when used by the compiler,
> # and contain replacements for sys.py, time.py and other modules that
> # are critical to pyjamas.   It goes without saying that if you overwrite
> # the standard python modules with the pyjamas equivalents, you will run into
> # massive problems.  Don't do it.

I found the above specfile comment a little unclear; it looks like it was copied from upstream instructions, but it immediately raises questions: do the pyjamas libraries get installed, and if so where?


%description desktop
Typo: s/he /The /


In the %files for the core package, you use:
  %{python_sitelib}/*
Is it possible to be a bit more explicit on this line?  Especially with the comment above, you've got me feeling paranoid about exactly what the payload of the package is.

Pyjamas provides reimplementations of various parts of the standard library, in a mix of pure python and JS  (see pyjs/src/pyjs/lib/)

"License" may be more complex that the specfile indicates; there's lots of info in the "copyright" file in the tarball, with a variety of licenses.

builder/boilerplate/pygwt.js has the comment "// this is almost directly taken from Google's GWT which is now open source":  may want to clarify that this is properly licensed.

Somewhat ironically, contrib/copyright_check.py ("debian copyright file checker") doesn't seem to have a license

(still reading through the tarball)

> I found the above specfile comment a little unclear; it looks like it was
> copied from upstream instructions, but it immediately raises questions: do the
> pyjamas libraries get installed, and if so where?

yeah, I copied this from upstream. Reworded it now. 
New wording: 

# NOTE: we only byte-compile the library files installed in the normal
# fedora python_sitelib. All other .py files are left alone. 
# This is due to those files replacing/overriding core modules. 
# They should never be run from anything but pyjamas itself, nor 
# should they be byte compiled. 

Fixed description typo. 

Changed sitelib to not use a wildcard. 

On licensing: 

- I think it does boil down to ASL2
- The pygwt.js should be ASL2 (at least google web toolkit is ASL2.0). 
- The contrib/copyright_check.py doesn't run, and I can't see how it would be useful here, but it seems a pain to just do a special tar for this. Will ask upstream I guess. It looks like they may have just dropped this post 0.7. 

New spec at: http://www.scrye.com/~kevin/fedora/pyjamas/pyjamas.spec

Should we ask spot to look over the licensing here? I could have missed something...

Thanks - this is looking good.

There are a couple of issues I think need fixing:
(i) Please fix the python-bytecode-inconsistent-mtime" from pylint as this will make "import pyjd" slower than it could be
(ii) Please fix the "License" metadata (see below)

Details:
NEEDSWORK: rpmlint must be run on every package. The output should be posted in the review.[1] 

$ rpmlint ~/rpmbuild/RPMS/noarch/pyjamas-*0.7-3*
pyjamas.noarch: E: non-executable-script /usr/lib/python2.6/site-packages/pyjs/pyjampiler.py 0644L /usr/bin/python
pyjamas.noarch: E: non-executable-script /usr/share/pyjamas/pyjs/tests/test-coverage.py 0644L /usr/bin/env
pyjamas.noarch: E: non-executable-script /usr/share/pyjamas/pyjs/src/pyjs/lib/base64.py 0644L /usr/bin/env
pyjamas.noarch: E: zero-length /usr/share/pyjamas/library/jsonrpc/json.py
pyjamas-desktop.noarch: W: no-documentation
pyjamas-desktop.noarch: E: python-bytecode-inconsistent-mtime /usr/lib/python2.6/site-packages/pyjd/__init__.pyc 2010-05-27T18:07:12 /usr/lib/python2.6/site-packages/pyjd/__init__.py 2010-05-27T18:07:14
pyjamas-desktop.noarch: E: non-executable-script /usr/lib/python2.6/site-packages/pyjd/idlparser.py 0644L /usr/bin/env
pyjamas-desktop.noarch: E: non-executable-script /usr/lib/python2.6/site-packages/pyjd/modcompile.py 0644L /usr/bin/env
pyjamas-desktop.noarch: E: non-executable-script /usr/lib/python2.6/site-packages/pyjd/pywebkitgtk.py 0644L /usr/bin/env
pyjamas-doc.noarch: W: no-documentation
pyjamas-examples.noarch: W: no-documentation
pyjamas-examples.noarch: E: zero-length /usr/share/pyjamas/examples/uitest/public/testLabelAdd.removelabel.txt
pyjamas-examples.noarch: E: zero-length /usr/share/pyjamas/examples/maparea/Hello.py
pyjamas-examples.noarch: E: zero-length /usr/share/pyjamas/examples/uitest/public/testButtonClick.removeButton.txt
pyjamas-examples.noarch: E: non-executable-script /usr/share/pyjamas/examples/jsonrpc/public/PythonCGIServer.py 0644L /usr/bin/env
pyjamas-examples.noarch: E: zero-length /usr/share/pyjamas/examples/maparea/Map.py
pyjamas-examples.noarch: E: script-without-shebang /usr/share/pyjamas/examples/flowpanel/FlowPanel.py
pyjamas-examples.noarch: E: zero-length /usr/share/pyjamas/examples/uitest/public/testDivText.removeDiv.txt
pyjamas-examples.noarch: E: zero-length /usr/share/pyjamas/examples/uitest/public/testDivAdd.removeDiv.txt
pyjamas-examples.noarch: E: zero-length /usr/share/pyjamas/examples/maparea/public/Hello.html
pyjamas-examples.noarch: E: zero-length /usr/share/pyjamas/examples/uitest/public/testDivHTML.removeDiv.txt
pyjamas-ui.noarch: W: no-documentation
5 packages and 0 specfiles checked; 18 errors, 4 warnings.

Please can you fix the "python-bytecode-inconsistent-mtime" error: this will force Python to recompile pyjd/__init__.py every time you import it, if run as non-root.

Everything else looks waivable to me.

OK: The package must be named according to the Package Naming Guidelines
OK: The spec file name must match the base package %{name}, in the format %{name}.spec unless your package has an exemption. [2] . 
OK: The package must meet the Packaging Guidelines .

OK: The package must be licensed with a Fedora approved license and meet the Licensing Guidelines .
NEEDSWORK: The License field in the package spec file must match the actual license. [3]
The file "copyright" in the source tree handily lists the licences.  Most files are indeed under Apache, but there's a mixture of others: some GPL-2, Other, ZPL-2.1, MIT, PSF, LGPL-2.1.  All appear to be Fedora-approved licenses.

Note that each subpackage can have its own "License" tag; see https://fedoraproject.org/wiki/Packaging/LicensingGuidelines#Multiple_Licensing_Scenarios

OK: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package must be included in %doc.[4]
OK: The spec file must be written in American English. [5]
OK: The spec file for the package MUST be legible. [6]

OK: The sources used to build the package must match the upstream source, as provided in the spec URL. Reviewers should use md5sum for this task. If no upstream URL can be specified for this package, please see the Source URL Guidelines for how to deal with this.
$ md5sum pyjamas-0.7.tgz pyjamas-0.7-2.fc14.src/pyjamas-0.7.tgz 
8441b60bb3c88051799537852cceefd0  pyjamas-0.7.tgz
8441b60bb3c88051799537852cceefd0  pyjamas-0.7-2.fc14.src/pyjamas-0.7.tgz

$ sha256sum pyjamas-0.7.tgz pyjamas-0.7-2.fc14.src/pyjamas-0.7.tgz 
648f62c1ddf99392fb926ca8d3900c39109b09e06d93cc7de3668933d7084d2b  pyjamas-0.7.tgz
648f62c1ddf99392fb926ca8d3900c39109b09e06d93cc7de3668933d7084d2b  pyjamas-0.7-2.fc14.src/pyjamas-0.7.tgz

OK: The package MUST successfully compile and build into binary rpms on at least one primary architecture. [7]
(see scratch build in comment #0)

N/A: noarch: If the package does not successfully compile, build or work on an architecture, then those architectures should be listed in the spec in ExcludeArch. Each architecture listed in ExcludeArch MUST have a bug filed in bugzilla, describing the reason that the package does not compile/build/work on that architecture. The bug number MUST be placed in a comment, next to the corresponding ExcludeArch line. [8]

OK: All build dependencies must be listed in BuildRequires, except for any that are listed in the exceptions section of the Packaging Guidelines ; inclusion of those as BuildRequires is optional. Apply common sense.

N/A: The spec file MUST handle locales properly. This is done by using the %find_lang macro. Using %{_datadir}/locale/* is strictly forbidden.[9]
./pyjd/pywebkitgtk.py uses gettext, but there don't seem to be any translations

N/A: Every binary RPM package (or subpackage) which stores shared library files (not just symlinks) in any of the dynamic linker's default paths, must call ldconfig in %post and %postun. [10]

OK: Packages must NOT bundle copies of system libraries.[11]
Special exception: there are copies of Python "system" libraries within pyjs/src/pyjs/lib - however these are there to support the JavaScript target environment, rather than CPython.

These seem to end up in: /usr/share/pyjamas/pyjs/src/pyjs/lib (and of course, CPython will break horribly if this ends up in someone's PYTHONPATH, I suspect, so good to keep it segregated)

N/A: If the package is designed to be relocatable, the packager must state this fact in the request for review, along with the rationalization for relocation of that specific package. Without this, use of Prefix: /usr is considered a blocker. [12]

OK: A package must own all directories that it creates. If it does not create a directory that it uses, then it should require a package which does create that directory. [13]

OK: A Fedora package must not list a file more than once in the spec file's %files listings. [14]

OK: Permissions on files must be set properly. Executables should be set with executable permissions, for example. Every %files section must include a %defattr(...) line. [15]

OK: Each package must consistently use macros. [16]

OK: The package must contain code, or permissable content. [17]

OK: Large documentation files must go in a -doc subpackage. (The definition of large is left up to the packager's best judgement, but is not restricted to size. Large can refer to either size or quantity). [18]

OK: If a package includes something as %doc, it must not affect the runtime of the application. To summarize: If it is in %doc, the program must run properly if it is not present. [18]

N/A: Header files must be in a -devel package. [19]
N/A: Static libraries must be in a -static package. [20]
N/A: If a package contains library files with a suffix (e.g. libfoo.so.1.1), then library files that end in .so (without suffix) must go in a -devel package. [19]

OK: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency: Requires: %{name} = %{version}-%{release} [21]

N/A: Packages must NOT contain any .la libtool archives, these must be removed in the spec if they are built.[20]

OK: Packages containing GUI applications must include a %{name}.desktop file, and that file must be properly installed with desktop-file-install in the %install section. If you feel that your packaged GUI application does not need a .desktop file, you must put a comment in the spec file with your explanation. [22]

There are some example GUI apps, but I don't think it's worth adding a slew of .desktop files for each one.

OK: Packages must not own files or directories already owned by other packages. The rule of thumb here is that the first package to be installed should own the files or directories that other packages may rely upon. This means, for example, that no package in Fedora should ever share ownership with any of the files or directories owned by the filesystem or man package. If you feel that you have a good reason to own a file or directory that another package owns, then please present that at package review time. [23]

OK: All filenames in rpm packages must be valid UTF-8. [24]

I went ahead and added a link to this review to the list of Python runtimes here:
https://fedoraproject.org/wiki/SIGs/Python#Python_Runtimes

ok, I have the inconsistent time fixed. 

I _think_ I have the license tags right, but it's nicely complicated. 
Could you check it over or can we ask spot to doublecheck?

new spec: 

http://www.scrye.com/~kevin/fedora/pyjamas/pyjamas.spec

(In reply to comment #6)
> ok, I have the inconsistent time fixed. 

Looks good

> I _think_ I have the license tags right, but it's nicely complicated. 
> Could you check it over or can we ask spot to doublecheck?

Thanks.  I agree that it's complicated :(

I'm afraid I think that it still needs a little work.

The "copyright" file in the upstream tarball appears to be a correct listing of the various licenses attached to the various files, though some of the files it lists aren't present anymore (e.g. jquery)

I'm reading:
https://fedoraproject.org/wiki/Packaging/LicensingGuidelines#Multiple_Licensing_Scenarios

and it does seem like the package "contains files which are under multiple, distinct, and independent licenses" (as per that wiki page).

My reading of that section is that each subpackage should have its own "License" field, listing all of the licenses with an "and" separator.

Going through the subpackages
- core subpackage: your license tag appears to be correct
- desktop subpackage: "pyjd" has some GPL-2 files, so I think that needs adding
- ui subpackage: seems to be all Apache, I think
- examples subpackage: has Apache, GPL-2, MIT, (MPL-1.1 or LGPL-2.1 or GPL-2)
- doc subpackage: apache, I think.

I am not a lawyer.  My recommendation is to ask for this to be doublechecked by https://admin.fedoraproject.org/mailman/listinfo/legal

Yeah, ok. Agreed. 

New spec at: 

http://www.scrye.com/~kevin/fedora/pyjamas/pyjamas.spec 

Blocking FE_LEGAL. Spot: can you see if I messed up anywhere with the current spec and the licensing tags?

Will it be better to install examples and doc to %{_docdir} instead of %{datadir}?

https://fedoraproject.org/wiki/Packaging_tricks#Examples

Here's what I see:

pyjamas:

License: ASL 2.0 and MIT 

(Which files are Python or ZPL?)

pyjamas-desktop:

License: ASL 2.0 and GPLv2+ and Python

pyjamas-doc:

License: ASL 2.0

pyjamas-examples:

License: ASL 2.0 and BSD and GPL+ and GPLv2+ and LGPLv2+ and MIT and Python and (MPLv1.1+ or LGPLv2+ or GPLv2+)

pyjamas-ui:

License: ASL 2.0

Hope that helps. Lifting FE-Legal.

That all looks good to me. I'm not sure why I was thinking there were Python or ZPL files in the base package. ;( 

Anyhow, new spec: 

http://www.scrye.com/~kevin/fedora/pyjamas/pyjamas.spec 

Matches the above license tags.

Thanks - this looks good

APPROVED

Thanks!

New Package CVS Request
=======================
Package Name: pyjamas
Short Description: A python to javascript compiler, Widget set, Framework and Toolkit
Owners: kevin
Branches: F-13 devel EL-5 EL-6
InitialCC:

CVS done (by process-cvs-requests.py).

pyjamas-0.7-5.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/pyjamas-0.7-5.fc13

pyjamas-0.7-6.el5 has been submitted as an update for Fedora EPEL 5.
http://admin.fedoraproject.org/updates/pyjamas-0.7-6.el5

pyjamas-0.7-5.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 pyjamas'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/pyjamas-0.7-5.fc13

pyjamas-0.7-6.el5 has been pushed to the Fedora EPEL 5 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 pyjamas'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/pyjamas-0.7-6.el5

(In reply to comment #0)
> pyjamas.noarch: W: no-manual-page-for-binary pyjsbuild

 there's a pyjsbuild.1 manpage in the debian packaging if you particularly want one.

> NOTE: This package overrides the normal python 'bytecompile everything ending
> in .py' because it has some library files that should not/cannot be byte
> compiled because they override system modules.    

yep. correct.  or, more to the point, you can _try_ but the specialist version of "imputil.py" in pyjd will ignore them.

(In reply to comment #3)
> > I found the above specfile comment a little unclear; it looks like it was
> > copied from upstream instructions, but it immediately raises questions: do the
> > pyjamas libraries get installed, and if so where?
> 
> yeah, I copied this from upstream. Reworded it now. 
> New wording: 
> 
> # NOTE: we only byte-compile the library files installed in the normal
> # fedora python_sitelib. All other .py files are left alone. 
> # This is due to those files replacing/overriding core modules. 
> # They should never be run from anything but pyjamas itself, nor 
> # should they be byte compiled. 

 that's a pretty good rewording.

> On licensing: 
> 
> - I think it does boil down to ASL2
> - The pygwt.js should be ASL2 (at least google web toolkit is ASL2.0). 

 yes.  we had to ask the GWT about this, and they specifically placed all files with a copyright notice, ASL2, juuust for us, explicitly, whereas it was implicit before.

> - The contrib/copyright_check.py doesn't run,

 ohh... ah, that'll be... yes.  change it to read ../copyright not ../debian/copyright and it'll work.

 l.

ok i can confirm that pyjamas-desktop works, which is a really nice surprise.  right now we have a bit of a situation with python-xpcom / xulrunner on ubuntu: they've upgraded to xulrunner 1.9.2, and it's a dog's dinner, with _all_ reverse-dependencies being simply cut (including hulahop).  python-xpcom has been _removed_ from xulrunner by the mozilla foundation, and the mozilla foundation is distinctly losing the plot, focussing on javascript, javascript, javascript, and speed, speed, speed.

but please _don't_ do what ubuntu did, which was, on discovering the above, entirely remove pyjamas from ubuntu.  lots of users are complaining: all they had to do was temporarily remove pyjamas-desktop, not the entire damn package.

at some point it may be the case that pywebkitgtk gets its DOM bindings: pyjamas-desktop can then use that as a dependency.  so, all is not lost.

l.

Excellent. Thanks for the info. :) 

Happy to adjust or tweak the fedora package to work better. Feel free to file bugs on it, or drop me email directly.

(In reply to comment #22)
> Excellent. Thanks for the info. :) 

 no problem.

> Happy to adjust or tweak the fedora package to work better. 

 cool.  i have to do a 0.7p1 or 0.7.1 soon, there's a couple of niggling bugs in 0.7

 but the actual packaging itself?  it looks solid to me.

> Feel free to file bugs on it, or drop me email directly.    

 willdo kevin.

pyjamas-0.7-5.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.

pyjamas-0.7-6.el5 has been pushed to the Fedora EPEL 5 stable repository.  If problems still persist, please make note of it in this bug report.