Bug 253434 - Review Request: eclipse-rpm-editor - RPM Specfile editor for Eclipse
Review Request: eclipse-rpm-editor - RPM Specfile editor for Eclipse
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Andrew Overholt
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-08-19 09:37 EDT by Alphonse Van Assche
Modified: 2008-01-22 16:54 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-01-22 16:54:29 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
overholt: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Alphonse Van Assche 2007-08-19 09:37:31 EDT
Spec URL: http://alcapcom.fedorapeople.org/SPECS/eclipse-rpm-editor.spec
SRPM URL: http://alcapcom.fedorapeople.org/SRPMS/eclipse-rpm-editor-0.1.0-1.fc7.src.rpm
Description: The Eclipse Specfile Editor package contains Eclipse plugins that are
useful for RPM specfiles maintenance within the Eclipse IDE.

The specfile editor need some code of rpmlint 0.81, I will ask Ville if it's possible to include needed patch at less in rawhide.
Comment 1 Ville Skyttä 2007-08-19 09:47:04 EDT
I expect that rpmlint 0.81 will be released fairly soon.  It'll be a slightly
backwards incompatible release with <= 0.80, so I'm not yet sure whether it will
be pushed to <= F7, but it'll be in F8+.
Comment 2 Alphonse Van Assche 2007-08-19 09:57:09 EDT
The only thing that the specfile editor need is the patch to run rpmlint on
.spec file. Is there a way to just include this patch for <= F7?
Comment 3 Ville Skyttä 2007-08-19 10:46:21 EDT
It is premature to think about that too much right now.  I don't plan to do that
myself, but in a nutshell, I suppose if the decision is not to ship 0.81+ for <=
F7 and someone volunteers to extract that functionality from 0.81 and backport
and maintain it as a patch to earlier versions, I'll look into it, but only
after 0.81 is out.
Comment 4 Alphonse Van Assche 2007-08-19 10:56:58 EDT
Okay thanks for the clarification, once 0.81 is out I would like to backport
this functionality as a patch for <= F7 rpmlint package, so that I can maintain
a F7 and EPEL branch for this package.
Comment 5 Andrew Overholt 2007-08-20 10:15:22 EDT
So we can at least review this for rawhide, right?
Comment 6 Andrew Overholt 2007-08-20 10:26:14 EDT
I'm getting:

W: eclipse-rpm-editor invalid-license Eclipse Public License

from rpmlint.  Perhaps make it EPL?
Comment 7 Alphonse Van Assche 2007-08-20 14:37:46 EDT
Oups, I use a old rpmlint (0.81) without the last license checks. I must replace
Eclipse Public License by EPL in all my eclipse packages, perhaps we may do that
in all the eclipse packages.

About review without rpmlint 0.81 in rawhide, I think that we can begin the
review  because rpmlint is only need at "use" time.



Comment 8 Alphonse Van Assche 2007-08-26 15:28:53 EDT
Spec URL: http://alcapcom.fedorapeople.org/SPECS/eclipse-rpm-editor.spec
SRPM URL:
http://alcapcom.fedorapeople.org/SRPMS/eclipse-rpm-editor-0.1.0-2.fc7.src.rpm

I have just fixed the License tag problem, any other stuff to do?

Comment 9 Andrew Overholt 2007-08-27 15:06:49 EDT
Okay, everything is perfect except for one small nit in the desription.  Thanks!

MUST items:

OK package is named appropriately
OK is it legal for Fedora to distribute this?
OK license field matches the actual license.
OK license is open source-compatible.
OK specfile name matches %{name}
OK verify source and patches
NEEDS_FIXING skim the summary and description for typos, etc.
 - "... RPM specfiles maintenance ..." -> "... maintenance of RPM specfiles ..."
OK correct buildroot
OK if %{?dist} is used, it should be in that form
OK license text included in package and marked with %doc
OK keep old changelog entries; use judgement when removing
OK packages meets FHS (http://www.pathname.com/fhs/)
OK rpmlint on <this package>.srpm gives no output
 - I'm fine with the odd permissions on the fetch script
OK changelog should be in one of these formats:
 [...]
OK Vendor tag should not be used
OK Distribution tag should not be used
OK use License and not Copyright 
OK Summary tag should not end in a period
OK if possible, replace PreReq with Requires(pre) and/or Requires(post)
OK specfile is legible
OK package successfully compiles and builds on at least x86
OK BuildRequires are proper
OK summary should be a short and concise description of the package
OK description expands upon summary
OK make sure lines are <= 80 characters
OK specfile written in American English
OK make a -doc sub-package if necessary
OK should the package contain a -devel sub-package?
OK use macros appropriately and consistently
OK don't use %makeinstall
OK install section must begin with rm -rf $RPM_BUILD_ROOT or %{buildroot}
OK locale data handling correct (find_lang)
OK consider using cp -p to preserve timestamps
OK split Requires(pre,post) into two separate lines
OK package should probably not be relocatable
OK package contains code
OK package should own all directories and files
OK there should be no %files duplicates
OK file permissions should be okay; %defattrs should be present
OK %clean should be present
OK %doc files should not affect runtime
OK verify the final provides and requires of the binary RPMs

  $ rpm -qp --provides ../RPMS/x86_64/eclipse-rpm-editor-0.1.0-2.fc8.x86_64.rpm
  org.eclipse.linuxtools.rpm.rpmlint_0.0.1.jar.so()(64bit)  
  org.eclipse.linuxtools.rpm.ui.editor_0.0.1.jar.so()(64bit)  
  eclipse-rpm-editor = 0.1.0-2.fc8

  $ rpm -qp --requires ../RPMS/x86_64/eclipse-rpm-editor-0.1.0-2.fc8.x86_64.rpm
  /bin/sh  
  /bin/sh  
  eclipse-changelog >= 2.5.1
  eclipse-platform >= 3.3.1
  java-gcj-compat  
  java-gcj-compat  
  libc.so.6()(64bit)  
  libc.so.6(GLIBC_2.2.5)(64bit)  
  libdl.so.2()(64bit)  
  libgcc_s.so.1()(64bit)  
  libgcc_s.so.1(GCC_3.0)(64bit)  
  libgcj_bc.so.1()(64bit)  
  libm.so.6()(64bit)  
  libpthread.so.0()(64bit)  
  librt.so.1()(64bit)  
  libz.so.1()(64bit)  
  rpmdevtools  
  rpmlib(CompressedFileNames) <= 3.0.4-1
  rpmlib(PayloadFilesHavePrefix) <= 4.0-1
  rpmlint >= 0.81
  rtld(GNU_HASH)

OK run rpmlint on the binary RPMs (no output)

SHOULD items:

OK package should include license text in the package and mark it with %doc
? package should build on i386 (I tried x86_64)
? package should build in mock
Comment 10 Andrew Overholt 2007-08-27 15:08:19 EDT
Is rpmlint >= 0.81 expected to hit rawhide soon?
Comment 11 Ville Skyttä 2007-08-27 15:16:56 EDT
rpmlint 0.81 does not even exist upstream yet.  My guess is that it's a matter
of a week or few until it will be released, and it will be available in Rawhide
shortly after that.
Comment 12 Andrew Overholt 2007-08-28 10:07:25 EDT
I forgot to assign this to myself.

What should we do about the rpmlint 0.81 requirement?  Lower the required
version of rpmlint or block on this package until that gets in?  Alphonse?  Ville?
Comment 13 Ville Skyttä 2007-08-28 11:21:38 EDT
I haven't checked how this package works, if it works (even partially) without
rpmlint installed, or if it works (even partially) with an older rpmlint.  If it
actually absolutely requires a version of rpmlint that can be invoked on
specfiles, my humble opinion would be to block it until such a rpmlint is
available in the target repos.  See also comment 3 and 4.
Comment 14 Alphonse Van Assche 2007-08-29 07:43:52 EDT
I lower the version of rpmlint and backport the needed patch, Ville can you
please take a look on because the original patch
http://rpmlint.zarb.org/cgi-bin/trac.cgi/changeset/1349?format=diff&new=1349
don't will apply on CheckSpec.py file. I have also patched the specfile so that
you have less stuff todo to include it.

http://alcapcom.fedorapeople.org/patches/rpmlint-0.80-rpmlint-on-specfiles.patch
http://alcapcom.fedorapeople.org/patches/rpmlint.spec-backport-rpmlint-on-specfiles.patch

Andrew the package is modify according with the review, FYI the package build on
i386 arch and using mock on rawhide.

Spec URL: http://alcapcom.fedorapeople.org/SPECS/eclipse-rpm-editor.spec
SRPM URL:
http://alcapcom.fedorapeople.org/SRPMS/eclipse-rpm-editor-0.1.0-3.fc7.src.rpm
Comment 15 Andrew Overholt 2007-08-29 09:24:08 EDT
The plugin will still function without the patched rpmlint, though, right?  If
so, I'm willing to approve this.  Otherwise, we can hold off until Ville gets
time to look at the rpmlint situation.
Comment 16 Alphonse Van Assche 2007-08-30 11:59:34 EDT
Right, the plugins should work without rpmlint but it that case rpmlint warnings
should not be showed in the editor.
Comment 17 Alphonse Van Assche 2007-08-30 12:05:00 EDT
Oups, without the patch I expect the same result.
Comment 18 Andrew Overholt 2007-08-30 15:05:26 EDT
Alright, then let's get this in there and we can work out any kinks with the
rpmlint version later (including removing that functionality temporarily if
necessary).

Approved

Thanks, Alphonse.  Don't forget to do the fedora-cvs ? and template thing.
Comment 19 Alphonse Van Assche 2007-08-31 06:15:54 EDT
New Package CVS Request
=======================
Package Name:  eclipse-rpm-editor 
Short Description: RPM Specfile editor for Eclipse
Owners: alcapcom,overholt
Branches: FC-6 F-7
InitialCC: overholt
Cvsextras Commits: yes
Comment 20 Kevin Fenzi 2007-08-31 22:06:46 EDT
cvs done. 
Comment 21 Peter Lemenkov 2008-01-22 08:38:42 EST
Should we close this Review Request since eclipse-rpm-editor already in Fedora?
Comment 22 Alphonse Van Assche 2008-01-22 08:43:29 EST
Seems to be yes, naturally if the rules is to close reviews when the package is in.

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