Bug 222365

I'll take this one.

rpmlint output:

W: eclipse-changelog non-standard-group Text Editors/Integrated Development
Environments (IDE)

Let's leave this one for now.  I'll investigate and get back to you.

W: eclipse-changelog invalid-license Eclipse Public License v1.0

Just put EPL and not the full name or version.

W: eclipse-changelog mixed-use-of-spaces-and-tabs (spaces: line 3, tab: line 11)

You've got mixed tabs and spaces.  One way to fix this is to open the specfile
in emacs and type Alt-x and then type untabify<Enter>.

(In reply to comment #2)
> W: eclipse-changelog invalid-license Eclipse Public License v1.0
> 
> Just put EPL and not the full name or version.

I used "Eclipse Public License" in the eclipse spec file and didn't get a warning.

(In reply to comment #3)
> (In reply to comment #2)
> > W: eclipse-changelog invalid-license Eclipse Public License v1.0
> > 
> > Just put EPL and not the full name or version.
> 
> I used "Eclipse Public License" in the eclipse spec file and didn't get a warning.

Yeah, that's fine.

. Since this is Fedora, we can do away with the fedora and redhat defines.  That
can be simplified to just %define gcj_support 1
. replace %{eclipse_name} in the definition of eclipse_base to just be eclipse
and remove eclipse_name entirely
. please add a little explanation (or a script a la eclipse-emf and eclipse-gef
... in Extras) as to how the src zip is generated
. "this plugins is really noarch but it" -> "These plugins are really noarch but
they"
. you've got an extra line between %prep and %build
. it's also unfortunate that we have an epoch but we can't do anything about
that now
. do we require a specific version of the CDT?
. how do we get around the optional python parser and a PyDev dependency?

Otherwise, things look good!

(In reply to comment #5)
> . please add a little explanation (or a script a la eclipse-emf and
eclipse-gef ... in Extras) as to how the src zip is generated

Since Kyu is a developer on this plugin, he should make a propper source zip
available for all to use instead of creating it each with a script.

Created attachment 145596 [details]
Edited .spec file

(In reply to comment #5)
> . Since this is Fedora, we can do away with the fedora and redhat defines.  That
> can be simplified to just %define gcj_support 1

Done

> . replace %{eclipse_name} in the definition of eclipse_base to just be eclipse
> and remove eclipse_name entirely

Done

> . "this plugins is really noarch but it" -> "These plugins are really noarch but
> they"

Done

> . you've got an extra line between %prep and %build

Done

> . it's also unfortunate that we have an epoch but we can't do anything about
> that now

Nothing to be done.

> . do we require a specific version of the CDT?

eclipse-cdt >= 3.1.1

Done

(In reply to comment #5)
> . please add a little explanation (or a script a la eclipse-emf and eclipse-gef
> ... in Extras) as to how the src zip is generated

Add a comment in .spec file you mean? If so, is there a common place to write
comment in .spec file about how the src zip is generated?

> . how do we get around the optional python parser and a PyDev dependency?
Right now python parser is disabled so it does not require PyDev dependency.
I'll have to do some more work to make python parser as an optional feature. So
in next release, I'll include it and make PyDev as build dependency.

(In reply to comment #9)
> (In reply to comment #5)
> > . please add a little explanation (or a script a la eclipse-emf and eclipse-gef
> > ... in Extras) as to how the src zip is generated
> 
> Add a comment in .spec file you mean? If so, is there a common place to write
> comment in .spec file about how the src zip is generated?

We usually try to put it right before the SourceN line.  If it's not
complicated, put the steps directly there.  If it's a bit more complicated, it
might be nice to have a shell script like the one for EMF
(fetch-eclipse-emf.sh).  I'll leave that up to you.

Ben's comment is interesting, though.  Can you make a src zip that can be
imported directly?  Then the Debian guys and others can use it directly.  What
do you think?

(In reply to comment #10)

> We usually try to put it right before the SourceN line.  If it's not
> complicated, put the steps directly there.  If it's a bit more complicated, it
> might be nice to have a shell script like the one for EMF
> (fetch-eclipse-emf.sh).  I'll leave that up to you.
> 

Generating src zip is really simple. I click on changelog-core,feature,doc
folder from Package Explorer and do Export as Archive. Just have to exclude bin
directories from each folders. I'll just write this down in spec file.

> Ben's comment is interesting, though.  Can you make a src zip that can be
> imported directly?  Then the Debian guys and others can use it directly.  What
> do you think?

Yes, the src zip I use can be imported directly from eclipse. We just have to
extract it in a workspace and do Create new project from existing location. It
will set up all the environment needed to start hacking on it.

(In reply to comment #11)
> Generating src zip is really simple. I click on changelog-core,feature,doc
> folder from Package Explorer and do Export as Archive. Just have to exclude bin
> directories from each folders. I'll just write this down in spec file.

No, it needs to be programmatic -- ie. scriptable on the command line.  Better
yet, you'll just do it and put it somewhere like below.

> Ben's comment is interesting, though.  Can you make a src zip that can be
> > imported directly?  Then the Debian guys and others can use it directly.  What
> > do you think?
> 
> Yes, the src zip I use can be imported directly from eclipse. We just have to
> extract it in a workspace and do Create new project from existing location. It
> will set up all the environment needed to start hacking on it.

What Ben's asking is for you to roll src zips when you cut releases.  So if you
were to create release 2.5 or something, you'd do whatever you do now (tag CVS,
etc.) *plus* you'd run some ant task to plop a src zip somewhere that can be
used directly by this RPM and by the Debian guys.

If this is possible -- and I think it is -- I'd prefer it to the "here's how we
generated this source tarball" current situation.

Here's a doc that explains how to make a src drop with eclipse:

http://people.redhat.com/bkonrath/eclipse/exporting-buildable-source-archives.html

Created attachment 145724 [details]
Edited spec file

source is now on sourceware.org.

Thanks.  You've still got mixed tabs and spaces.

Also, don't use %{name} in the source URL ... it's just another thing to fix
when trying to wget :)

(In reply to comment #15)
> Thanks.  You've still got mixed tabs and spaces.
> 
> Also, don't use %{name} in the source URL ... it's just another thing to fix
> when trying to wget :)

So why not use:

$ spectool --gf blah.spec

instead? spectool handles the macro expansion and then calls wget on the result.

(In reply to comment #16)
> (In reply to comment #15)
> > Also, don't use %{name} in the source URL ... it's just another thing to fix
> > when trying to wget :)
> 
> So why not use:
> 
> $ spectool --gf blah.spec
> 
> instead? spectool handles the macro expansion and then calls wget on the result.

'cause I didn't know about spectool :)  Thanks!

Created attachment 145844 [details]
fixed tab/space mix

Updated specfile/srpm uploaded to same location.

Spec URL: http://sourceware.org/eclipse/changelog/eclipse-changelog.spec
SRPM URL:
http://sourceware.org/eclipse/changelog/eclipse-changelog-2.3.3-3.fc7.src.rpm

Almost there.  Just fix the lines beginning with an X:

MUST:
X rpmlint on eclipse-changelog srpm gives this as output

W: eclipse-changelog non-standard-group Text Editors/Integrated Development
Environments (IDE)

Let's change this to Development/Tools as per:

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=204525#c5

* package is named appropriately
* specfile name matches %{name}
* package meets packaging guidelines.
* license field matches the actual license.
* license is open source-compatible.
X license text included in package and marked with %doc

you'll need to include a copy of epl-v10.html and mark it with %doc in the
%files section

* specfile written in American English
* specfile is legible
* source files match upstream
* package successfully compiles and builds on at least x86 (it's building on
the other arches in Fedora Core presently)

> please wait for the CDT bug to be filed regarding investigating building on
> all arches and then add its URL to the comment

* BuildRequires are proper
* no locale data so no find_lang necessary
* package is not relocatable
* package owns all directories and files
* no %files duplicates
* file permissions are fine; %defattrs present
* %clean present
* macro usage is consistent
* package contains code
* no large docs so no -doc subpackage
  the doc plugins aren't usable outside of Eclipse so there's no point marking
  them as %doc
* %doc files don't affect runtime
* shared libraries are present, but no ldconfig required.
* no pkgconfig or header files
* no -devel package
* no .la files
* no desktop file
* not a web app.
* file ownership fine
* final provides and requires are sane

$ rpm -qp --requires eclipse-changelog-2.3.3-3.i386.rpm
/usr/bin/rebuild-gcj-db  
/usr/bin/rebuild-gcj-db  
eclipse-platform >= 1:3.2.0
java-gcj-compat  
java-gcj-compat  
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  
rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1
rtld(GNU_HASH) 

$ rpm -qp --provides eclipse-changelog-2.3.3-3.i386.rpm
changelog.jar.so  
eclipse-changelog = 1:2.3.3-3

SHOULD:
X package does not include license text
* package builds on i386 (and others in brew ATM ... I don't envision a
problem here)
* package functions in Eclipse (on FC6 ... not much has changed in rawhide in
the Eclipse stack so I don't anticipate anything rawhide-specific)
X package builds in mock
  my mock setup doesn't seem to be working but I don't anticipate any problems
  here as the package currently builds fine in brew

(In reply to comment #20)
> Almost there.  Just fix the lines beginning with an X:
> 
> MUST:
> X rpmlint on eclipse-changelog srpm gives this as output
> 
> W: eclipse-changelog non-standard-group Text Editors/Integrated Development
> Environments (IDE)
> 
> Let's change this to Development/Tools as per:
> 
> https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=204525#c5

Done.



> X license text included in package and marked with %doc
> 
> you'll need to include a copy of epl-v10.html and mark it with %doc in the
> %files section
> 

Done.


> X package does not include license text

Done.

> X package builds in mock
>   my mock setup doesn't seem to be working but I don't anticipate any problems
>   here as the package currently builds fine in brew

Nothing to do.


New versions uploaded to 

Spec URL: http://sourceware.org/eclipse/changelog/eclipse-changelog.spec
SRPM URL:
http://sourceware.org/eclipse/changelog/eclipse-changelog-2.3.3-3.fc7.src.rpm

(In reply to comment #21)
> (In reply to comment #20)
> > MUST:
> > X rpmlint on eclipse-changelog srpm gives this as output
> > 
> > W: eclipse-changelog non-standard-group Text Editors/Integrated Development
> > Environments (IDE)
> > 
> > Let's change this to Development/Tools as per:
> > 
> > https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=204525#c5
> 
> Done.

You've got "Development/Toos" :)

> > X license text included in package and marked with %doc
> > 
> > you'll need to include a copy of epl-v10.html and mark it with %doc in the
> > %files section
> 
> Done.

Almost:

RPM build errors:
    File not found by glob:
/var/tmp/eclipse-changelog-2.3.3-3-root-overholt/usr/share/eclipse/features/com.redhat.eclipse.changelog_*/epl-v10.html

You need to actually include the file.  I'd say put it in ChangeLog CVS (in
features/com.redhat.eclipse.changelog) and roll a new tarball.  Call it 2.3.3.1
or something.

> > X package does not include license text
> 
> Done.
> 
> > X package builds in mock
> >   my mock setup doesn't seem to be working but I don't anticipate any problems
> >   here as the package currently builds fine in brew
> 
> Nothing to do.
> 
> 
> New versions uploaded to 
> 
> Spec URL: http://sourceware.org/eclipse/changelog/eclipse-changelog.spec
> SRPM URL:
> http://sourceware.org/eclipse/changelog/eclipse-changelog-2.3.3-3.fc7.src.rpm
> 

All done again.. Sorry I should've double checked.

New versions uploaded to 
 
Spec URL: http://sourceware.org/eclipse/changelog/eclipse-changelog.spec
SRPM URL:
http://sourceware.org/eclipse/changelog/eclipse-changelog-2.3.4-1.fc7.src.rpm

APPROVED

Thanks, Kyu!

Now for the question of how we go about moving something from Core to Extras ...

Jesse:  eclipse-changelog is currently in Core but in anticipation of the merge,
it's been reviewed and is ready to be moved to Extras.  It's a leaf package. 
How should we proceed?

We're working on a mechanism to allow to move with SCM history. That mechanism
is not finished yet. Give us a day or three?

(In reply to comment #26)
> We're working on a mechanism to allow to move with SCM history. That mechanism
> is not finished yet. Give us a day or three?

Cool, thanks for the info, Bill.

Hey Andrew. Can this be closed now? 
You might also set the fedora-review to + also to conform to the current rules... 
(although I don't think it matters too much either way)

Yes, we can close this now and set it to +.  Thanks for the reminder.