Bug 477190 - Review Request: cas - core analysis system
Review Request: cas - core analysis system
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Tom "spot" Callaway
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-12-19 10:28 EST by Adam Stokes
Modified: 2009-03-04 17:28 EST (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-03-04 17:28:26 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
tcallawa: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Adam Stokes 2008-12-19 10:28:42 EST
Spec URL: http://astokes.fedorapeople.org/cas.spec
SRPM URL: http://astokes.fedorapeople.org/cas-0.13-112.src.rpm
Description:
CAS provides a support engineer the ability to configure an environment for
core analysis quickly. All the hassles of configuring architecture specific
boxes, finding machines suitable for a particular core is now taken care of.

-- 

This is my first package submission and need a sponsor
Comment 1 Tom "spot" Callaway 2008-12-19 10:41:57 EST
Okay, a few things to start with:

* Please don't use a source file for Version/Release. That's what the spec file is for.
* You can't use "GPL" as a license tag, see: 
https://fedoraproject.org/wiki/Packaging/LicensingGuidelines#GPL_and_LGPL
* Please put the full upstream path to your sources in your Source definitions.
* Run rpmlint on the packages (both SRPM and noarch RPM) and make sure you've silenced the output.

When you've made these changes, be sure to increment Release and add a new changelog entry describing the changes that you've made. Once that's done, post a new SRPM and spec file and I'll pick up the review.
Comment 2 Adam Stokes 2008-12-19 11:14:13 EST
(In reply to comment #1)
> Okay, a few things to start with:
> 
> * Please don't use a source file for Version/Release. That's what the spec file
> is for.
> * You can't use "GPL" as a license tag, see: 
> https://fedoraproject.org/wiki/Packaging/LicensingGuidelines#GPL_and_LGPL

Thanks, set to GPLv3

> * Please put the full upstream path to your sources in your Source definitions.
Fixed

> * Run rpmlint on the packages (both SRPM and noarch RPM) and make sure you've
> silenced the output.
[adam@conans cas]$ rpmlint -i ~/redhat/cas/noarch/cas-0.13-113.noarch.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
[adam@conans cas]$ rpmlint -i ~/redhat/cas/cas-0.13-113.src.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

> 
> When you've made these changes, be sure to increment Release and add a new
> changelog entry describing the changes that you've made. Once that's done, post
> a new SRPM and spec file and I'll pick up the review.
Done
SRPM: http://astokes.fedorapeople.org/cas-0.13-113.src.rpm
SPEC: http://astokes.fedorapeople.org/cas.spec

thanks
Comment 3 Tom "spot" Callaway 2008-12-19 11:51:54 EST
Looking at the code, it looks like it is GPLv3+.

Why are the man pages separate from the upstream source? Seems like they'd be a logical fit to go inside the tarball, or at the very least, uploaded to the fedorahosted site. If you opt for the latter route, please provide full upstream URLs.

Do you still need Source1? It doesn't seem to be used anymore.

You also don't need the Requires: python >= 2.4

Fedora's RPM will detect the python bits in the package and add a proper versioned Requires on python(abi). For example, in rawhide, we get:

[spot@velociraptor ~]$ rpm -qp /home/spot/rpmbuild/RPMS/noarch/cas-0.13-113.fc11.noarch.rpm --requires
/usr/bin/python  
config(cas) = 0.13-113.fc11
crash  
python >= 2.4
python(abi) = 2.6
rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(PartialHardlinkSets) <= 4.0.4-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1

Thus, your manual python Requires is unnecessary.

Also, the last sentence of the description doesn't make much sense. Can you fix that up a bit? :)
Comment 4 Adam Stokes 2008-12-29 06:34:27 EST
(In reply to comment #3)
> Looking at the code, it looks like it is GPLv3+.

Thanks for the catch, this makes sense as I am not committed to v3 only

> 
> Why are the man pages separate from the upstream source? Seems like they'd be a
> logical fit to go inside the tarball, or at the very least, uploaded to the
> fedorahosted site. If you opt for the latter route, please provide full
> upstream URLs.

Fixed
> 
> Do you still need Source1? It doesn't seem to be used anymore.

Fixed
> 
> You also don't need the Requires: python >= 2.4
> 
> Fedora's RPM will detect the python bits in the package and add a proper
> versioned Requires on python(abi). For example, in rawhide, we get:
> 
> [spot@velociraptor ~]$ rpm -qp
> /home/spot/rpmbuild/RPMS/noarch/cas-0.13-113.fc11.noarch.rpm --requires
> /usr/bin/python  
> config(cas) = 0.13-113.fc11
> crash  
> python >= 2.4
> python(abi) = 2.6
> rpmlib(CompressedFileNames) <= 3.0.4-1
> rpmlib(PartialHardlinkSets) <= 4.0.4-1
> rpmlib(PayloadFilesHavePrefix) <= 4.0-1
> 
> Thus, your manual python Requires is unnecessary.
Fixed

> 
> Also, the last sentence of the description doesn't make much sense. Can you fix
> that up a bit? :)

Hopefully I cleared up the description, my writing skills are subpar :(

Thanks

http://astokes.fedorapeople.org/cas-0.13-114.src.rpm
http://astokes.fedorapeople.org/cas.spec
Comment 5 Tom "spot" Callaway 2009-01-06 14:03:36 EST
= Review =

Good:

- rpmlint checks return nothing
- package meets naming guidelines
- package meets packaging guidelines
- license (GPLv3+) OK, text in %doc, matches source
- spec file legible, in am. english
- source matches upstream
- package compiles on devel (x86)
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- %clean ok
- macro use consistent (well, 99%)
- code, not content
- no need for -docs
- nothing in %doc affects runtime
- no need for .desktop file 

One very minor item, you're using "${RPM_BUILD_ROOT}" and "$RPM_BUILD_ROOT" in the spec. Pick one and use it consistently. You can make this change before you commit to CVS.

APPROVED. I will sponsor you. Please follow the process here: https://fedoraproject.org/wiki/PackageMaintainers/Join#Get_Sponsored
Comment 6 Adam Stokes 2009-01-07 07:56:51 EST
New Package CVS Request
=======================
Package Name: cas
Short Description: automated core analysis tool 
Owners: astokes
Branches: F-9 F-10 EL-4 EL-5
InitialCC:
Comment 7 Kevin Fenzi 2009-01-09 00:39:34 EST
I don't see you in the packager group yet. Have you requested that yet?
See step 4 under here: 
http://fedoraproject.org/wiki/PackageMaintainers/Join#Get_a_Fedora_Account
Comment 8 Adam Stokes 2009-01-12 06:20:53 EST
Under the status is says 'unapproved' could this be why?

Thanks
Comment 9 Adam Stokes 2009-01-12 06:41:00 EST
Ok I re-applied, may I get approval now?

Thanks!
Comment 10 Adam Stokes 2009-01-12 11:37:12 EST
Ok thanks to spot I am now sponsored.
Comment 11 Adam Stokes 2009-01-15 07:39:34 EST
Hi, could i get a status update on this? Is there anything else needed from me?

Thanks!
Comment 12 Tom "spot" Callaway 2009-01-15 10:59:37 EST
Adam, please follow the process at this step (and read that document from top to bottom, so you understand how things work).

http://fedoraproject.org/wiki/PackageMaintainers/Join#Add_Package_to_CVS_and_Set_Owner
Comment 13 Alex Lancaster 2009-01-22 04:37:21 EST
(In reply to comment #12)
> Adam, please follow the process at this step (and read that document from top
> to bottom, so you understand how things work).
> 
> http://fedoraproject.org/wiki/PackageMaintainers/Join#Add_Package_to_CVS_and_Set_Owner

Adam you should always build in rawhide first before pushing updates to the F-10 and F-9 branches:

http://fedoraproject.org/wiki/PackageMaintainers/Join#Request_Builds

This requirement should be made explicit in that doc.

In any case your push of cas has caused broken deps in rawhide because koji "inherited" the F-10 build in rawhide (since there wasn't any F-11 cas build to "override" it).  But that F-10 package of cas won't work on rawhide because rawhide requires Python 2.6, not Python 2.5 (which is shipped on F-10):

Broken deps for i386
----------------------------------------------------------
	cas-0.13-118.fc10.noarch requires python(abi) = 0:2.5

http://koji.fedoraproject.org/mash/rawhide-20090122/logs/depcheck

So I took the liberty of rebuilding it for you:

https://koji.fedoraproject.org/koji/buildinfo?buildID=79757
Comment 14 Adam Stokes 2009-01-22 09:33:35 EST
Thank you, still getting used to the processes here and you're right the documentation should perhaps mention to build in devel branch first.

Thanks again
Comment 15 Tom "spot" Callaway 2009-01-22 10:02:59 EST
(In reply to comment #13)

> Adam you should always build in rawhide first before pushing updates to the
> F-10 and F-9 branches:
> 
> http://fedoraproject.org/wiki/PackageMaintainers/Join#Request_Builds
> 
> This requirement should be made explicit in that doc.

I've added a warning to that section which should hopefully make that clear.
Comment 16 Tom "spot" Callaway 2009-03-04 17:28:26 EST
I see this in rawhide, closing out this bug.

Adam, in the future, make sure your Review Request bug is closed out when the package is built.

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