Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 222365 - Review Request: eclipse-changelog - simplifies the task of maintaining ChangeLogs for projects
Summary: Review Request: eclipse-changelog - simplifies the task of maintaining Change...
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Andrew Overholt
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT 222350
TreeView+ depends on / blocked
 
Reported: 2007-01-11 21:02 UTC by Kyu Lee
Modified: 2013-01-10 04:10 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-06-11 15:55:25 UTC
Type: ---
Embargoed:
overholt: fedora-review+


Attachments (Terms of Use)
Edited .spec file (deleted)
2007-01-15 18:33 UTC, Kyu Lee
no flags Details
Edited spec file (deleted)
2007-01-16 19:58 UTC, Kyu Lee
no flags Details
fixed tab/space mix (deleted)
2007-01-17 18:20 UTC, Kyu Lee
no flags Details

Description Kyu Lee 2007-01-11 21:02:47 UTC
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
Description: 
The ChangeLog plug-in simplifies the task of maintaining ChangeLogs for your projects. It has parsers that work with Java, C/C++, and Python. It currently has one formatter, which formats ChangeLog entries in the "GNU Style". You can add your own parsers and formatters by contributing to the extension points defined in the plug-in.

Comment 1 Andrew Overholt 2007-01-12 15:16:13 UTC
I'll take this one.

Comment 2 Andrew Overholt 2007-01-12 15:17:54 UTC
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>.

Comment 3 Ben Konrath 2007-01-12 15:49:55 UTC
(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.

Comment 4 Andrew Overholt 2007-01-12 16:16:36 UTC
(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.

Comment 5 Andrew Overholt 2007-01-12 18:07:55 UTC
. 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!

Comment 6 Ben Konrath 2007-01-12 18:11:26 UTC
(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.

Comment 7 Kyu Lee 2007-01-15 18:33:23 UTC
Created attachment 145596 [details]
Edited .spec file

Comment 8 Kyu Lee 2007-01-15 18:35:06 UTC
(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




Comment 9 Kyu Lee 2007-01-15 18:40:08 UTC
(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.

Comment 10 Andrew Overholt 2007-01-15 19:06:25 UTC
(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?

Comment 11 Kyu Lee 2007-01-15 19:59:43 UTC
(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.

Comment 12 Andrew Overholt 2007-01-15 20:43:14 UTC
(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.

Comment 13 Ben Konrath 2007-01-15 20:52:57 UTC
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

Comment 14 Kyu Lee 2007-01-16 19:58:18 UTC
Created attachment 145724 [details]
Edited spec file

source is now on sourceware.org.

Comment 15 Andrew Overholt 2007-01-16 21:35:38 UTC
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 :)

Comment 16 Paul Howarth 2007-01-17 10:19:41 UTC
(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.

Comment 17 Andrew Overholt 2007-01-17 14:44:41 UTC
(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!

Comment 18 Kyu Lee 2007-01-17 18:20:44 UTC
Created attachment 145844 [details]
fixed tab/space mix

Comment 19 Kyu Lee 2007-01-17 19:07:27 UTC
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



Comment 20 Andrew Overholt 2007-01-17 22:38:15 UTC
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

Comment 21 Kyu Lee 2007-01-22 16:19:04 UTC
(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


Comment 22 Andrew Overholt 2007-01-22 16:43:20 UTC
(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
> 



Comment 23 Kyu Lee 2007-01-22 18:21:11 UTC
All done again.. Sorry I should've double checked.

Comment 25 Andrew Overholt 2007-01-22 21:23:59 UTC
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?

Comment 26 Bill Nottingham 2007-01-22 21:44:36 UTC
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?

Comment 27 Andrew Overholt 2007-01-22 21:47:08 UTC
(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.

Comment 28 Kevin Fenzi 2007-06-09 04:30:03 UTC
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)

Comment 29 Andrew Overholt 2007-06-11 15:55:25 UTC
Yes, we can close this now and set it to +.  Thanks for the reminder.


Note You need to log in before you can comment on or make changes to this bug.