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@redhat.com
Branches: 
InitialCC: overholt@redhat.com

Built as eclipse-pydev-1.3.1-5 in devel.