Bug 222365
Summary: | Review Request: eclipse-changelog - simplifies the task of maintaining ChangeLogs for projects | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Kyu Lee <klee> | ||||||||
Component: | Package Review | Assignee: | Andrew Overholt <overholt> | ||||||||
Status: | CLOSED RAWHIDE | QA Contact: | Fedora Package Reviews List <fedora-package-review> | ||||||||
Severity: | medium | Docs Contact: | |||||||||
Priority: | medium | ||||||||||
Version: | rawhide | CC: | dcantrell, kevin, notting | ||||||||
Target Milestone: | --- | Flags: | overholt:
fedora-review+
|
||||||||
Target Release: | --- | ||||||||||
Hardware: | All | ||||||||||
OS: | Linux | ||||||||||
Whiteboard: | |||||||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||||||
Doc Text: | Story Points: | --- | |||||||||
Clone Of: | Environment: | ||||||||||
Last Closed: | 2007-06-11 15:55:25 UTC | Type: | --- | ||||||||
Regression: | --- | Mount Type: | --- | ||||||||
Documentation: | --- | CRM: | |||||||||
Verified Versions: | Category: | --- | |||||||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||||
Cloudforms Team: | --- | Target Upstream Version: | |||||||||
Embargoed: | |||||||||||
Bug Depends On: | |||||||||||
Bug Blocks: | 163779, 222350 | ||||||||||
Attachments: |
|
Description
Kyu Lee
2007-01-11 21:02:47 UTC
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. |