Spec URL: http://people.redhat.com/ifoox/extras/eclipse-pydev.spec SRPM URL: http://people.redhat.com/ifoox/extras/eclipse-pydev-1.3.0-1.src.rpm Description: PyDev is a plugin for Eclipse for working with Python as well as Jython code.
Here's a new srpm that has jython as a dependency: http://people.redhat.com/ifoox/extras/eclipse-pydev-1.3.0-2.src.rpm
I'll take this. Initial stuff (full review to come): $ rpmlint rpmbuild/RPMS/i386/eclipse-pydev-1.3.0-2.i386.rpm W: eclipse-pydev no-documentation If there are no docs, then I guess we can live with that. They mention that it's EPL in feature.xml but I don't want to mark that as %doc 'cause it'll affect runtime. W: eclipse-pydev dangling-symlink /usr/share/eclipse/plugins/org.python.pydev.core_1.3.0/commons-codec.jar /usr/share/java/jakarta-commons-codec.jar W: eclipse-pydev symlink-should-be-relative /usr/share/eclipse/plugins/org.python.pydev.core_1.3.0/commons-codec.jar /usr/share/java/jakarta-commons-codec.jar W: eclipse-pydev dangling-symlink /usr/share/eclipse/plugins/org.python.pydev.jython_1.3.0/jython.jar /usr/share/java/jython.jar W: eclipse-pydev symlink-should-be-relative /usr/share/eclipse/plugins/org.python.pydev.jython_1.3.0/jython.jar /usr/share/java/jython.jar W: eclipse-pydev dangling-symlink /usr/share/eclipse/plugins/org.python.pydev.core_1.3.0/lib/junit.jar /usr/share/java/junit.jar W: eclipse-pydev symlink-should-be-relative /usr/share/eclipse/plugins/org.python.pydev.core_1.3.0/lib/junit.jar /usr/share/java/junit.jar I'm fine with all of these. $ rpmlint eclipse-pydev-1.3.0-2.src.rpm W: eclipse-pydev strange-permission fetch-eclipse-pydev.sh 0755 rpmlint wants this to be 644 but I don't really know why. I'll read the guidelines and see.
Thanks for taking this Andrew. The exceptions for the symlinks seem OK to me also. As for the fetch script's permissions, I guess rpmlint may think that a Source item should not be executable, I'm not sure what other packages do in this case. As you said for the documentation, there doesn't seem to be anything that would fall under documentation in this package. Also it seems that Fabio just released a new version of PyDev. Here's an updated SRPM: http://people.redhat.com/ifoox/extras/eclipse-pydev-1.3.1-1.src.rpm
By the way 1.3.1 build works on my messed up rawhide VM, unlike 1.3.0, which had the project creation hanging problem.
Now that we've got Mylar in Fedora, it'd be great to package up the Pydev Mylar integration. I just noticed that it had this here: http://pydev.sourceforge.net/ I'm doing the review now.
Lines beginning with an X need to be fixed. They're all minor - thanks! :) MUST: * package is named appropriately * it is legal for Fedora to distribute this * license field matches the actual license. * license is open source-compatible. - I really wish they packed the license text in their VCS. Can you please ask them to do so? I won't hold up the review on it but it would be great if it was more prominent than just in their "new in 0.9.8.4" section of their website. * specfile name matches %{name} X verify source and patches (md5sum matches upstream, know what the patches do) - I can't duplicate the md5sum of the tarball, but the contents match except for some timestamps of the generation time - can we get some comments for the patches? they could also be re-numbered if you feel like it :) - should we file the references to 0.9.7.1 issue upstream? * summary and description fine X correct buildroot - should be: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) ? %{?dist} isn't used ... should it be? X license text included in package and marked with %doc - upstream doesn't include license text outside of feature.xml and I don't want to mark that as %doc; we're okay here * package meets FHS (http://www.pathname.com/fhs/) * rpmlint on SRPM (see earlier bug comments) * changelog format fine * Packager tag not used * Vendor tag not used * Distribution tag not used * License used and not Copyright * Summary tag does not end in a period * if possible, replace PreReq with Requires(pre) and/or Requires(post) X specfile is legible - there is an extraneous '#' in the comment about how to generate the tarball - the instructions for generating the tarball should have an updated VCS tag (I know it says "substitute the correct version number" but get rid of that comment and fix the actual tag) - speaking of the tarball-generating instructions, can we clean them up a bit? Let's drop the "following the Eclipse Releng process" bit - remove pkg_summary and eclipse_name and just type them in directly * package successfully compiles and builds on at least x86 X BuildRequires are proper - why don't we just have BR: eclipse-pde? X make sure lines are <= 80 characters - can we split the commons-codec symlinking line? - also, the lines with symlinking in %install are too long * specfile written in American English * no -doc sub-package necessary * no libraries * no rpath * no config files * not a GUI app * no -devel sub-package necessary * macros used appropriately and consistently * no %makeinstall * install section must begin with rm -rf $RPM_BUILD_ROOT or %{buildroot} * no locale data * no cp usage so no need to worry about -p * split Requires(pre,post) into two separate lines * package not relocatable * package contains code * package owns all directories and files * no %files duplicates * file permissions fine ? %defattrs present - should there be another '-' after the 'root,root'? * %clean present * %doc files do not affect runtime * not a web app * verify the final provides and requires of the binary RPMs $ rpm -q --requires -p eclipse-pydev-1.3.1-1.i386.rpm /usr/bin/rebuild-gcj-db /usr/bin/rebuild-gcj-db commons-codec >= 1.3 eclipse-platform java-1.5.0-gcj >= 1.5.0 java-1.5.0-gcj >= 1.5.0 junit >= 3.8.1 jython >= 2.2 libc.so.6 libc.so.6(GLIBC_2.1.3) libdl.so.2 libgcc_s.so.1 libgcc_s.so.1(GCC_3.0) libgcj_bc.so.1 libm.so.6 libpthread.so.0 librt.so.1 libz.so.1 python rpmlib(CompressedFileNames) <= 3.0.4-1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 rtld(GNU_HASH) $ rpm -q --provides -p eclipse-pydev-1.3.1-1.i386.rpm ast.jar.so core.jar.so parser.jar.so pydev-debug.jar.so pydev-jython.jar.so pydev.jar.so refactoring.jar.so eclipse-pydev = 1:1.3.1-1 * run rpmlint on the binary RPMs - see previous bug comments SHOULD: X package should include license text in the package and mark it with %doc * package should build on i386 ? package should build in mock - I haven't tried, but I don't think it'll be a problem
I did a test build of the SRPM, if failed, after installing Jython*, the the build went fine, maybe Jython should be a build requirement.
(In reply to comment #7) > I did a test build of the SRPM, if failed, after installing Jython*, the the > build went fine, maybe Jython should be a build requirement. You're correct. Igor, can we add jython as a BR?
(In reply to comment #5) > Now that we've got Mylar in Fedora, it'd be great to package up the Pydev Mylar > integration. I just noticed that it had this here: > > http://pydev.sourceforge.net/ Agreed, this will be a good next step. But it should be separately packaged, like they do it upstream.
New files: (people.redhat.com doesn't seem to respond right now, so I'm uploading these files elsewhere and will move them together with the other ones once it's back) http://www.igorfoox.com/misc/fedora/eclipse-pydev-1.3.1-2.src.rpm http://www.igorfoox.com/misc/fedora/eclipse-pydev.spec (In reply to comment #6) > Lines beginning with an X need to be fixed. They're all minor - thanks! :) > > MUST: > * package is named appropriately > * it is legal for Fedora to distribute this > * license field matches the actual license. > * license is open source-compatible. > - I really wish they packed the license text in their VCS. Can you please > ask them to do so? I won't hold up the review on it but it would be great > if it was more prominent than just in their "new in 0.9.8.4" section of > their website. I will ask them to include the license. > * specfile name matches %{name} > X verify source and patches (md5sum matches upstream, know what the patches do) > - I can't duplicate the md5sum of the tarball, but the contents match except > for some timestamps of the generation time > - can we get some comments for the patches? they could also be re-numbered > if you feel like it :) Done. > - should we file the references to 0.9.7.1 issue upstream? I've brought this up before, this seems to be their preferred way of doing things, strange as it is. > * summary and description fine > X correct buildroot > - should be: > %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) Done. > ? %{?dist} isn't used ... should it be? I see no reason why it shouldn't be. Done. > X license text included in package and marked with %doc > - upstream doesn't include license text outside of feature.xml and I don't > want to mark that as %doc; we're okay here > * package meets FHS (http://www.pathname.com/fhs/) > * rpmlint on SRPM (see earlier bug comments) > * changelog format fine > * Packager tag not used > * Vendor tag not used > * Distribution tag not used > * License used and not Copyright > * Summary tag does not end in a period > * if possible, replace PreReq with Requires(pre) and/or Requires(post) > X specfile is legible > - there is an extraneous '#' in the comment about how to generate the tarball > - the instructions for generating the tarball should have an updated VCS tag > (I know it says "substitute the correct version number" but get rid of that > comment and fix the actual tag) > - speaking of the tarball-generating instructions, can we clean them up a > bit? Let's drop the "following the Eclipse Releng process" bit Simplified the whole comment and put the appropriate tag. > - remove pkg_summary and eclipse_name and just type them in directly Done. > * package successfully compiles and builds on at least x86 > X BuildRequires are proper > - why don't we just have BR: eclipse-pde? You're right, that would be better. Done. > X make sure lines are <= 80 characters > - can we split the commons-codec symlinking line? > - also, the lines with symlinking in %install are too long I've split them up, although some are still slightly over 80 chars. > * specfile written in American English > * no -doc sub-package necessary > * no libraries > * no rpath > * no config files > * not a GUI app > * no -devel sub-package necessary > * macros used appropriately and consistently > * no %makeinstall > * install section must begin with rm -rf $RPM_BUILD_ROOT or %{buildroot} > * no locale data > * no cp usage so no need to worry about -p > * split Requires(pre,post) into two separate lines > * package not relocatable > * package contains code > * package owns all directories and files > * no %files duplicates > * file permissions fine > ? %defattrs present > - should there be another '-' after the 'root,root'? I'm not sure, some other specs I've looked at have four components, some have 3. So I've added it, since it seems that's what most do. > * %clean present > * %doc files do not affect runtime > * not a web app > * verify the final provides and requires of the binary RPMs > $ rpm -q --requires -p eclipse-pydev-1.3.1-1.i386.rpm > /usr/bin/rebuild-gcj-db > /usr/bin/rebuild-gcj-db > commons-codec >= 1.3 > eclipse-platform > java-1.5.0-gcj >= 1.5.0 > java-1.5.0-gcj >= 1.5.0 > junit >= 3.8.1 > jython >= 2.2 > libc.so.6 > libc.so.6(GLIBC_2.1.3) > libdl.so.2 > libgcc_s.so.1 > libgcc_s.so.1(GCC_3.0) > libgcj_bc.so.1 > libm.so.6 > libpthread.so.0 > librt.so.1 > libz.so.1 > python > rpmlib(CompressedFileNames) <= 3.0.4-1 > rpmlib(PayloadFilesHavePrefix) <= 4.0-1 > rtld(GNU_HASH) > > $ rpm -q --provides -p eclipse-pydev-1.3.1-1.i386.rpm > ast.jar.so > core.jar.so > parser.jar.so > pydev-debug.jar.so > pydev-jython.jar.so > pydev.jar.so > refactoring.jar.so > eclipse-pydev = 1:1.3.1-1 > > * run rpmlint on the binary RPMs > - see previous bug comments > > SHOULD: > X package should include license text in the package and mark it with %doc > * package should build on i386 > ? package should build in mock > - I haven't tried, but I don't think it'll be a problem (In reply to comment #7) > I did a test build of the SRPM, if failed, after installing Jython*, the the > build went fine, maybe Jython should be a build requirement. You're right, added.
(In reply to comment #10) > > ? %defattrs present > > - should there be another '-' after the 'root,root'? > > I'm not sure, some other specs I've looked at have four components, some have 3. > So I've added it, since it seems that's what most do. It seems to not really matter. See http://www.rpm.org/max-rpm-snapshot/s1-rpm-inside-files-list-directives.html#S3-RPM-INSIDE-FLIST-DEFATTR-DIRECTIVE Also, it is redundant in rpm >= 4.4, but still required for older RHEL versions. http://www.redhat.com/archives/fedora-packaging/2007-February/msg00171.html
OK, so in my opinion we might as well leave the extra '-' in there since it does no harm. Andrew?
(In reply to comment #12) > OK, so in my opinion we might as well leave the extra '-' in there since it does > no harm. Andrew? Yeah, let's leave it. I'll finish the rest of the review ASAP. Thanks.
Bah, I completely forgot to put this in the review: X Remove ExclusiveArch Sorry.
New files: http://people.redhat.com/ifoox/extras/eclipse-pydev-1.3.1-3.src.rpm http://people.redhat.com/ifoox/extras/eclipse-pydev.spec Removed ExclusiveArch.
Created attachment 151442 [details] patch - mainly whitespace Here's a patch that fixes some whitespace and line length issues. If it can be applied, we're all set to go except for: - plugins/org.python.pydev.jython_1.3.1/Lib. Where does the content of this directory come from? Is it necessary? If so, can we symlink it? - plugins/org.python.pydev_1.3.1/PySrc. What is the content of this directory? If we want to ship it or if it's necessary, I think it should either have a clear source and license trail or be a separate package. - plugins/org.python.pydev.jython_1.3.1/jysrc. Same questions as above. - plugins/org.python.pydev.debug_1.3.1/pysrc. Same. Thanks, Igor - sorry for the legal annoyances, I wish we had caught them earlier.
eclipse-pydev-1.3.1-3.src.rpm b build fine in mock (development / i386) But it look like eclipse-jdt is needed as a requirement. $ rpm -qa | grep "eclipse" eclipse-subclipse-1.1.9-2.fc7 eclipse-pydev-1.3.1-3.fc7 eclipse-rcp-3.2.2-7.fc7 eclipse-ecj-3.2.2-7.fc7 eclipse-platform-3.2.2-7.fc7 Pydev is not working. $ rpm -qa | grep "eclipse" eclipse-subclipse-1.1.9-2.fc7 eclipse-pydev-1.3.1-3.fc7 eclipse-rcp-3.2.2-7.fc7 eclipse-jdt-3.2.2-7.fc7 eclipse-ecj-3.2.2-7.fc7 eclipse-platform-3.2.2-7.fc7 Pydev is working now
Andrew, I'll look into the legalities of these files right now. Tim, you may be right that JDT is required. What was the error you were getting without it?
(In reply to comment #18) > Tim, you may be right that JDT is required. What was the error you were getting > without it? I dont get any visible errors, it is just not working, there is no PyDev entry in Windows -> Preferences. In The About -> Plugin details i only see 2 org.python.pydev plugins (ast & online help) the other org.python.pydev is missing.
(In reply to comment #19) > I dont get any visible errors, it is just not working, there is no PyDev entry > in Windows -> Preferences. > In The About -> Plugin details i only see 2 org.python.pydev plugins (ast & > online help) the other org.python.pydev is missing. Thanks, I'll include a requires for JDT in the next revision. (In reply to comment #16) > > - plugins/org.python.pydev.jython_1.3.1/Lib. Where does the content of this > directory come from? Is it necessary? If so, can we symlink it? > > - plugins/org.python.pydev_1.3.1/PySrc. What is the content of this directory? > If we want to ship it or if it's necessary, I think it should either have a > clear source and license trail or be a separate package. This is some auxiliary scripts that are used for running tests I believe. Some of the files here come from outside python packages, see full reply bellow. > - plugins/org.python.pydev.jython_1.3.1/jysrc. Same questions as above. This is pydev's files, and they are jython scripts used to control pydev. > > - plugins/org.python.pydev.debug_1.3.1/pysrc. Same. These seem to be pydev's own scripts. Here's Fabio's reply about the origin of these files: http://sourceforge.net/mailarchive/message.php?msg_name=cfb578b20704060345n677f8a26u5e87bbc3a4a20c8%40mail.gmail.com What do you think is the appropriate thing to do with these?
Sorry for taking so long to look at this. I've CC'd spot (and emailed him some of the back story) to get his take on the situation. (In reply to comment #20) > (In reply to comment #16) > > > > - plugins/org.python.pydev.jython_1.3.1/Lib. Where does the content of this > > directory come from? Is it necessary? If so, can we symlink it? Fabio says (copied from [1]): "All beneath org.python.pydev.jython/Lib come from jython 2.1. I just extracted it and bundled it in pydev (the jython guys probably have taken some of them from python). I don't think it's really safe to change those files, as it could break some things... that internal jython distribution is actually only used for jython scripting inside of pydev, but those scripts would have to be re-checked if an upgrade is done (I'd rather only do that update once jython 2.2 is stable)." So is it okay for us to include this separate copy? Jython's BSD and PyDev is EPL. What's python? If it's okay to include them from Fedora's standpoint (with the plan to symlink once jython 2.2 goes final) and from a legal POV, I'm okay with keeping them. > > - plugins/org.python.pydev_1.3.1/PySrc. What is the content of this directory? "The files from org.python.pydev/PySrc/ThirdParty are either from Bycicle repair man or ctypes (they are distributed in pydev, and executed under a separate shell)." We don't seem to have bicycle repairman or ctypes in Fedora. If the bits of those projects are necessary for non-testing purposes, can we include them for now and try to package those projects or do we need to package those projects first? > This is some auxiliary scripts that are used for running tests I believe. How much can we remove that's not testing-related? > > - plugins/org.python.pydev.jython_1.3.1/jysrc. Same questions as above. > > This is pydev's files, and they are jython scripts used to control pydev. Okay, great. > > - plugins/org.python.pydev.debug_1.3.1/pysrc. Same. > These seem to be pydev's own scripts. Okay, great. [1] http://sourceforge.net/mailarchive/message.php?msg_name=cfb578b20704060345n677f8a26u5e87bbc3a4a20c8%40mail.gmail.com
While this is a mess, all of the files seem to be ok: pydev: Eclipse jython: BSD python: PSF License ctypes: MIT bicycle repairman: BSD If possible to not duplicate ctypes/bicycle repairman, please don't. But if its unavoidable, its ok.
Thanks, Tom. Igor: if we can't symlink any more than we already do, let's go ahead with this for now. APPROVED
Igor's going away for the weekend so he asked me to request the CVS module for him. New Package CVS Request ======================= Package Name: eclipse-pydev Short Description: An Eclipse plugin for working with Python as well as Jython code. Owners: ifoox Branches: InitialCC: overholt
Built as eclipse-pydev-1.3.1-5 in devel.