Bug 233236 - (epel-release) Review Request: epel-release EPEL repository configuration and setup
Review Request: epel-release EPEL repository configuration and setup
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Kevin Fenzi
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-03-21 00:20 EDT by Michael Stahnke
Modified: 2007-11-30 17:11 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-04-18 07:47:28 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
kevin: fedora‑review+
dennis: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Michael Stahnke 2007-03-21 00:20:04 EDT
Spec URL: 
SRPM URL:  http://www.stahnkage.com/tmp/epel-release-4-2.src.rpm
Description: EPEL repository configuration and setup
Comment 1 Jens Petersen 2007-03-22 20:41:04 EDT
Was wondering if it would be easier to have one generic package to cover
both EL4 and EL5?
Comment 2 Michael Stahnke 2007-03-22 22:20:55 EDT
Jens, it was planned to use the same package for both.  The only differences in
RHEL 5 should be the yum repo file, and a bump of epel-release.  

Up2date isn't involved in RHEL 5, so that's a plus.  I just submitted this to
get the ball rolling on the epel-release package.  I thought it would have to
approved only on EL 4 and then I would bump for EL 5 and build.  

I followed the model used by Fedora-release.  

Comment 3 Jens Petersen 2007-03-22 22:46:12 EDT
You're right: the version only appears in two places... :)
Comment 4 Kevin Fenzi 2007-03-24 19:21:06 EDT
Humm. I can't seem to get to the srpm url... just hangs on me. 

Eventually I get: 
504 Gateway Time-out
17:18:35 ERROR 504: Gateway Time-out.

Also, is there a spec url?
Comment 5 Kevin Fenzi 2007-03-24 19:27:58 EDT
Nevermind. Now it's working. ;) 

Will review this in a bit here... 
Comment 6 Kevin Fenzi 2007-03-24 19:45:35 EDT
OK - Package meets naming and packaging guidelines
OK - Spec file matches base package name.
OK - Spec has consistant macro usage.
OK - Meets Packaging Guidelines.
OK - License (GPL)
OK - License field in spec matches
OK - License file included in package
OK - Spec in American English
OK - Spec is legible.
See below - Sources match upstream md5sum:
OK - BuildRequires correct
OK - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
OK - Package has correct buildroot
OK - Package is code or permissible content.
OK - Packages %doc files don't affect runtime.
OK - Package has rm -rf RPM_BUILD_ROOT at top of %install
OK - Package compiles and builds on at least one arch.
OK - Package has no duplicate files in %files.
OK - Package doesn't own any directories other packages own.
See below - Package owns all the directories it creates.
See below - No rpmlint output.
See below - final provides and requires are sane.

SHOULD Items:

OK - Should build in mock.

Issues:

1. Since redhat/fedora is upstream for this package, can you add
a note as suggested in:
http://www.fedoraproject.org/wiki/Packaging/SourceURL#head-413e1c297803cfa9de0cc4c56f3ac384bff5dc9e

2. The up2date files present a problem. Should up2date be required
by this package (so that the post commands work right)? If so, that would
be anoying to places where up2date was not installed. Perhaps it should
be in a subpackage, only installed by those that have up2date?
Should this package require 'yum' ?

3. Where does the version come from? 4 for epel4?
Is the epel5 version of this going to be version 5? 
If so, perhaps there is no need for the dist tag?

4. rpmlint says:

W: epel-release non-conffile-in-etc /etc/epel-release
W: epel-release non-conffile-in-etc /etc/pki/rpm-gpg/RPM-GPG-KEY-EPEL

I think those can be ignored.

W: epel-release dangerous-command-in-%postun cp

Do we need to make a rpmsave there?

5. The description could be a bit more verbose.
Perhaps you could expand EPEL in case there are people who
don't know what it means?
Comment 7 Dennis Gilmore 2007-03-24 21:25:00 EDT
RHEL4 does not have yum it only has up2date  CentOS4 has yum  so by not 
requiring either it will work on both with what comes with the distro.  

I really dont think EPEL should have yum for EL-4
Comment 8 Kevin Fenzi 2007-03-24 21:39:06 EDT
Yeah, looking at centos-release and redhat-release, they provide the needed
directories, so there shouldn't be a need for yum or up2date Requires, so cross
that off. 

Although, it looks like centos-release also provides an apt-rpm source list.
Might be nice for this package to also provide one. 
Comment 9 Thorsten Leemhuis 2007-03-25 04:02:08 EDT
Why do we need a epel-release file in etc? That seems unneeded to me.

This cp in %postun looks dangerous to me, too. Can that be avoided somehow?
Comment 10 Michael Stahnke 2007-03-25 12:57:44 EDT
http://www.stahnkage.com/tmp/epel-release-4-3.src.rpm
http://www.stahnkage.com/tmp/epel-release.spec

My ISP has been having issues, so if you can't get these files downloaded, try
again in 20 minutes or so.

%changelog
* Sun Mar 25 2007 Michael Stahnke <mastahnke@gmail.com> - 4-3
- Removed cp in postun
- Removed the file epel-release - provides no value
- Removed dist tag as per review bug #233236
- Changed description

Comment 11 Kevin Fenzi 2007-03-25 13:16:19 EDT
That looks good. 

The summary seems fine now, but the description is still using epel... 

perhaps change it from: 

"This package contains the epel repository GPG key as well as configuration
for yum and up2date."

to

"This package contains the Extra Packages for Enterprise Linux (EPEL) repository
GPG key as well as configuration for yum and up2date."

Other than that nitpick, I don't see any blockers, so this package is APPROVED. 
Don't forget to close this once it's been imported and built. 
Comment 12 Michael Stahnke 2007-03-25 13:22:31 EDT
I'll change the description after CVS branch.  Thanks!

New Package CVS Request
=======================
Package Name: epel-release
Short Description: EPEL repository configuration and setup
Owners: mastahnke@gmail.com
Branches: EL-4, EL-5
Comment 13 Dennis Gilmore 2007-03-25 13:31:02 EDT
Branched
Comment 14 Kevin Fenzi 2007-04-17 22:53:43 EDT
This package appears to be in now... can we close this review request?

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