Bug 567348

Just csome quick comments on your spec file:

- Is 'CFLAGS="$RPM_OPT_FLAGS"' really needed? It's a noarch package.
- Please use '%global' instead of '%define'
  https://fedoraproject.org/wiki/Packaging:Guidelines#.25global_preferred_over_.25define
- The README is missing in %doc
- The license is GPLv3+, source header says '3 of the License, or (at your option) any later version.'
- The latest release is 1.0.2

Please take a look at he following page, too : https://fedoraproject.org/wiki/Packaging:Python

Thank you, Fabian

(In reply to comment #1)
> Just csome quick comments on your spec file:
> 
> - Is 'CFLAGS="$RPM_OPT_FLAGS"' really needed? It's a noarch package.
> - Please use '%global' instead of '%define'
>  
> https://fedoraproject.org/wiki/Packaging:Guidelines#.25global_preferred_over_.25define

Done.

> - The README is missing in %doc

I've not included the README because it contains installation instructions for Windows and Debian that I think are unneeded by fedora users.

"Irrelevant documentation include build instructions, the omnipresent INSTALL file containing generic build instructions, for example, and documentation for non-Linux systems, e.g. README.MSDOS."
https://fedoraproject.org/wiki/Packaging/Guidelines#Documentation

> - The license is GPLv3+, source header says '3 of the License, or (at your
> option) any later version.'

I've updated it and added LGPLv2+ too after finding the build/lib/dreampielib/gui/SimpleGladeApp.py module.

> - The latest release is 1.0.2
> 
> Please take a look at he following page, too :
> https://fedoraproject.org/wiki/Packaging:Python    

I have. Could you please be a bit more specific as to what exactly you think is wrong?

http://mapleoin.fedorapeople.org/pkgs/dreampie/dreampie.spec
http://mapleoin.fedorapeople.org/pkgs/dreampie/dreampie-1.0.2-1.fc12.src.rpm

%changelog
* Wed Mar  3 2010 Ionuț C. Arțăriși <mapleoin> - 1.0.2-1
- fixed spelling error in description
- don't make subp_main.py executable
- don't pass cflags to build command
- use global instead of define macros
- changed license field to GPLv3+ and LGPLv2+
- updated to 1.0.2

I was working on a package too, but didn't consider it ready for Fedora yet. I was working with upstream while this was submitted without contacting or following upstream. That's a stupid waste of work.

Some of my work and review got upstreamed, but please take what you can use from the spec at http://bitbucket.org/kiilerix/dreampie-rpm/src/tip/dreampie.spec - though there isn't much to take except these comments:

I would say that one file as LGPLv2+ (which is compatible with and thus relicensable to GPLv3) isn't worth mentioning in the license field. And there are other files with other compatible licenses.

Group: It is not a lib but more like Development/Tools

The package requires pygtk2-libglade, not just pygtk2

Man pages should be %doc

And finally and the reason I got stalled: I think the use of zip files is a show-stopper. It is a ugly hack and doesn't give the transparency we like in Fedora packaging. The py2 files should be installed as plain py files, and the py3 files moved to a py3 subpackage on the archs where it is available.

Hi, Mads

I couldn't find a link to the upstream's devel list (I found it now, buried at the bottom of the frontpage). If I had, I could've seen your message in time. On the other hand I think you too should've checked bugzilla or the fedora-package-review list earlier.

I agree with your observations, except for the license field, which should probably look like this:

License: GPLv3+ and LGPLv3+ and BSD and ASL 2.0 and Python

if guidelines were to be followed: https://fedoraproject.org/wiki/Packaging/LicensingGuidelines#Multiple_Licensing_Scenarios

Alternatively we could try convincing upstream to relicense some of those files.


I was leaving the zip file issue for discussion with the reviewer. I agree that we shouldn't keep them archived, but install them as plain files instead.

> The py2 files should be installed as plain py files, and the
py3 files moved to a py3 subpackage on the archs where it is available.
I supppose you mean Fedora releases instead of archs. 

I don't think there's any reason to create a subpackage since we can't determine at build time if the user has python3 installed and everyone who has python3 will probably want py3 support in dreampie. I think we can just install these "data files" (as upstream calls them) side-by-side for Fedora releases with python3 (>=F13). Running dreampie against either a py2 or py3 interpreter is one of the major features of dreampie imho, so it shouldn't be in a different subpackage.


I see you've already established a relationship with upstream, so if you want I can leave this package for you to take ownership of.

(In reply to comment #4)
> On the other hand I think you too should've checked bugzilla or the
> fedora-package-review list earlier.

Yeah. I checked before contacting upstream, but I took the wrong semaphore. I should have learned from the dining philosophers and taken all of them. Lesson learned for next time ;-)

> I see you've already established a relationship with upstream, so if you want I
> can leave this package for you to take ownership of.    

Thanks, but no thanks - you came here first, so please keep going.

> I agree with your observations, except for the license field, which should
> probably look like this:
> 
> License: GPLv3+ and LGPLv3+ and BSD and ASL 2.0 and Python
> 
> if guidelines were to be followed:
> https://fedoraproject.org/wiki/Packaging/LicensingGuidelines#Multiple_Licensing_Scenarios

You are right. But I think that we have to be pragmatic as long as there are no license violations.

For example, I think it is fair enough to say that the linux kernel is GPLv2 even though it (AFAIK) contains files under BSD.

> Alternatively we could try convincing upstream to relicense some of those
> files.

You as packager are free to do it as well. If the package re-license it as GPL then it is GPL. (IANAL)

> I was leaving the zip file issue for discussion with the reviewer.

IMHO the packager should do all the work and review the package himself to make sure it is perfect. The reviewer should just review and verify. But let us discuss it until we agree that the package is perfect and ready for a formal review.

> I don't think there's any reason to create a subpackage since we can't
> determine at build time if the user has python3 installed and everyone who has
> python3 will probably want py3 support in dreampie. I think we can just install
> these "data files" (as upstream calls them) side-by-side for Fedora releases
> with python3 (>=F13). Running dreampie against either a py2 or py3 interpreter
> is one of the major features of dreampie imho, so it shouldn't be in a
> different subpackage.

FWIW I disagree.

It is not data files but executable code.

If python3 is a build dependency (in fedora versions where it is available) then it _is_ installed. Users who want py3 should install py3 packages.

Optional dependencies should be in sub-packages. Nothing special here.

Ok, then I'll keep working on this package, here are the new files:
http://mapleoin.fedorapeople.org/pkgs/dreampie/dreampie.spec
http://mapleoin.fedorapeople.org/pkgs/dreampie/dreampie-1.0.2-2.fc12.src.rpm

* Thu Mar 11 2010 Ionuț C. Arțăriși <mapleoin> - 1.0.2-2
- changed group to Development/Tools
- marked manpages as %%doc
- added pygtk2-libglade dependency
- added more license fields: BSD, ASL, Python and Copyright only

I talked to tibbs and spot on #fedora-devel and cleared out the licensing issue. The right License tag is the really long and complicated one. One of the motivations for this is that any of those files can be taken and used separately after the package has been installed on a fedora system.

FYI, the kernel license is probably explained by the fact that the multiple licensed code is compiled together which is not the issue with python programs. There's more on that here:
https://fedoraproject.org/wiki/Licensing/FAQ#Multiple_licensing_situations

spot interpreted the "whatever you want" license as "Copyright only" so I added that too.

I haven't done anything in the spec about the zip files issue yet. I'm going to upstream's mailing list after I finish writing this comment.

> If python3 is a build dependency (in fedora versions where it is available)
> then it _is_ installed. Users who want py3 should install py3 packages.
I'm not sure what you're saying here. If python3 is a build dependency then it's installed on the build machine, not on the user's machine.

Anyway I've thought about this a bit more and I agree. There should be a py3 sub-package which depends on the main package. 
The rationale that I was getting caught in before was that dreampie running a python3 interpreter still runs on python2 itself.

Upstream has removed the zip files, so I've made an update:

- new upstream version
- build separate py3 package
- byte-compile py2 and py3 files manually

http://mapleoin.fedorapeople.org/pkgs/dreampie/dreampie.spec
http://mapleoin.fedorapeople.org/pkgs/dreampie/dreampie-1.0.3-1.fc13.src.rpm

Is this review on going?

A new version has been released upstream.

I also don't think that creating a python3 subpackage for the private modules used by dreampie itself is a proper practice. People can use dreampie with an individually compiled Python 3 but without the vendor provided one.

%py_byte_compile %{__python} $RPM_BUILD_ROOT%{python_sitelib}/%{name}lib/
Without the above line, .pyo files are not generated.

Manpages is marked as %doc by default. And explicit %attr setting seems not needed too.

Hello Robin,

This review is waiting for its reviewer.

> I also don't think that creating a python3 subpackage for the private modules
> used by dreampie itself is a proper practice. People can use dreampie with an
> individually compiled Python 3 but without the vendor provided one.

I don't think that's a very good reason. After all, people can use dreampie on a recompiled version of Python 2 or any other of the packages listed as Requires, but that's not a reason to stop using dependencies. 

The fact remains that those files are useless without a Python3 interpreter and so they need a python3 Requires. The rest of the package is fine without it so that's the reason that the python3 files are segregated in a subpackage, as well as providing extra functionality.
 
I've updated the package and fixed the other two issues, thanks!

http://mapleoin.fedorapeople.org/pkgs/dreampie/dreampie-1.1-1.fc13.src.rpm
http://mapleoin.fedorapeople.org/pkgs/dreampie/dreampie.spec

The name 'python3-dreampie' implies a DreamPie support for Python3. But in fact, the subpackage is a Python3 support for DreamPie. So it seems better named 'dreampie-python3'.

Ok, I think that makes sense.

http://mapleoin.fedorapeople.org/pkgs/dreampie/dreampie-1.1-2.fc13.src.rpm
http://mapleoin.fedorapeople.org/pkgs/dreampie/dreampie.spec

* 'rpmlint must be run on every package. The output should be posted in the review.', quoted from https://fedoraproject.org/wiki/Packaging:ReviewGuidelines

* Add '# The entire source code is GPLv3+ except'

* Relative path should be used to declare license, for example, use 'dreampielib/gui/SimpleGladeApp.py' instead of just the file name 'SimpleGladeApp.py'.

* py_zipimport.py is not found in the source of this release.

* 'LGPLv2.1+' should be 'LGPLv2+'

* BuildRoot tag can be removed.

* md5 matched.
$ md5sum dreampie-1.1.tar.gz 
57ced153616069ca6c3c7d37bb30633a  dreampie-1.1.tar.gz

* Should instruct users to install dreampie-python3 when they want to use dreampie with Python3. The proper way is to include a README.Fedora file.

* In this case, subpackage name can be defined through '%package python3'.

* In %description, a comma should be added before 'etc.'

* The summary of the subpackage is not accurate.

* The main rpm should own '%{_datadir}/%{name}/'.

* The subpackage should specify requirement of Python interpreter through 'python(abi)' not 'python3'.

Thank you, Robin.

$ rpmlint dreampie.spec ../SRPMS/dreampie-1.1-3.fc13.src.rpm ../RPMS/noarch/dreampie-1.1-3.fc13.noarch.rpm ../RPMS/noarch/dreampie-python3-1.1-3.fc13.noarch.rpm 
dreampie.spec: W: no-buildroot-tag
dreampie.src: W: no-buildroot-tag
dreampie-python3.noarch: W: no-documentation
3 packages and 1 specfiles checked; 0 errors, 3 warnings.

Here are the updated files:
http://mapleoin.fedorapeople.org/pkgs/dreampie/dreampie.spec
http://mapleoin.fedorapeople.org/pkgs/dreampie/dreampie-1.1-3.fc13.src.rpm

* Upstream README, which you may think just contains useless installation instructions, should still be contained, I think. It doesn't take much space.

* Don't use hard version of python(abi). Set it at buildtime:

%global py3_ver %(echo `%{__python3} -c "import sys; sys.stdout.write(sys.version[:3])"`)

.......

Requires:   python(abi) = %{py3_ver}

* The subpackage contains just GPLv3+ files. So a License tag should be added for it.

Ok, I added all 3 changes.

http://mapleoin.fedorapeople.org/pkgs/dreampie/dreampie-1.1-4.fc13.src.rpm
http://mapleoin.fedorapeople.org/pkgs/dreampie/dreampie.spec

 Copyright only is not a valid license type, you should ask upstream to clarify the license for dreampielib/gui/gtkexcepthook.py.

'Copyright only' is a valid short name, see http://fedoraproject.org/wiki/Licensing:Main#Good_Licenses and https://fedoraproject.org/wiki/Licensing/CopyrightOnly .

Created attachment 447458 [details]
A trick to let rpmbuild generate proper python(abi) requirement for the subpackage

I am sorry. My trick posted before to set python(abi) at buildtime does not work.
Because the macro %__python3 is not available if python3-devel is not installed. And then defining %py3_ver will fail, and finally rpmbuild will raise a syntax error.

You can remove the python3-devel and try to rebuild the package.


I now suggest another trick. See this patch.

Or better use that tricky file as SOURCE2.

I'm sorry for not being clear.  Your solution is more complex than it needs to be.  The following three lines will work:

# get python3 version
%global py3_ver 0
%{?__python3: %global py3_ver %(%{__python3} -c "import sys; sys.stdout.write(sys.version[:3])")}

When creating the srpm that's fed to the buildsystem, the spec file needs to be parsable but it doesn't need to have the correct values.  So we need py3_ver to have a number (in this case 0) so that when it's used in the Requires line later the spec file will parse correctly.

That srpm is fed to the buildsystem which then creates the binary rpms and a new srpm.  These files are created in a buildroot that has python3-devel installed and thus py3_ver is set to the proper python3 version when those are built.

(the other part of your patch to byte compile the things in %{python3_sitelib} is still necessary.

The above solution is excellently better. The requester should follow it and this package will be approved.

I have replaced the lines to get the python3 version. It seems to work fine now without the phantom file in %{python3_sitelib}.

http://mapleoin.fedorapeople.org/pkgs/dreampie/dreampie-1.1-5.fc13.src.rpm
http://mapleoin.fedorapeople.org/pkgs/dreampie/dreampie.spec

All issues are cleared and build in Rawhide succeeded: http://koji.fedoraproject.org/koji/taskinfo?taskID=2486155

This package is approved by 'cheeselee'.

New Package SCM Request
=======================
Package Name: dreampie
Short Description: A graphical cross-platform interactive Python shell
Owners: mapleoin
Branches: f13 f14

Git done (by process-git-requests).

dreampie-1.1-5.fc13 has been submitted as an update for Fedora 13.
https://admin.fedoraproject.org/updates/dreampie-1.1-5.fc13

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

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

Don't forget to add this package to the comps.
Refer to: https://fedoraproject.org/wiki/PackageMaintainers/CompsXml