Bug 442055

Summary: Review Request: mono-sharpcvslib - Client cvs library written in c#
Product: [Fedora] Fedora Reporter: Tom "spot" Callaway <tcallawa>
Component: Package ReviewAssignee: Alex Lancaster <alex>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, notting
Target Milestone: ---Flags: alex: fedora-review+
tcallawa: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2008-04-13 00:16:41 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:

Description Tom "spot" Callaway 2008-04-11 14:44:04 UTC
Spec URL: http://www.auroralinux.org/people/spot/review/new/mono-sharpcvslib.spec
SRPM URL: http://www.auroralinux.org/people/spot/review/new/mono-sharpcvslib-0.35-1.fc9.src.rpm
Description: Gives C# projects the ability to communicate with a CVS server.

Comment 1 Alex Lancaster 2008-04-12 12:32:51 UTC
Not full review yet but first thing I noticed that it doesn't build on F-8 even
with all BuildRequires installed:

+ rm -rf src/lib/ICSharpCode.SharpZipLib.dll src/lib/ICSharpCode.SharpZipLib.pdb
src/lib/ext
+ rm -rf src/tools/nant/linux/ICSharpCode.SharpCvsLib.dll
src/tools/nant/linux/ICSharpCode.SharpZipLib.dll
src/tools/nant/linux/NAnt.Contrib.Tasks.dll
src/tools/nant/linux/NAnt.Core.Tests.dll src/tools/nant/linux/NAnt.Core.dll
src/tools/nant/linux/NAnt.DotNet.Tests.dll
src/tools/nant/linux/NAnt.DotNetTasks.dll src/tools/nant/linux/NAnt.NUnit.dll
src/tools/nant/linux/NAnt.NUnit1Tasks.dll
src/tools/nant/linux/NAnt.NUnit2Tasks.dll
src/tools/nant/linux/NAnt.SourceControlTasks.dll
src/tools/nant/linux/NAnt.Tests.dll src/tools/nant/linux/NAnt.Zip.Tests.dll
src/tools/nant/linux/NAnt.ZipTasks.dll src/tools/nant/linux/NAnt.exe
src/tools/nant/linux/NAnt.exe.config src/tools/nant/linux/NDoc.Core.dll
src/tools/nant/linux/NDoc.Documenter.Msdn.dll src/tools/nant/linux/NUnitCore.dll
src/tools/nant/linux/README.txt src/tools/nant/linux/log4net.dll
src/tools/nant/linux/nunit.framework.dll
+ mkdir -p src/lib/ext/
+ cp /usr/lib/mono/log4net/log4net.dll src/lib/ext/
+ cp /usr/lib/mono/2.0/ICSharpCode.SharpZipLib.dll src/lib/
+ cp /usr/lib/mono/2.0/nunit.framework.dll src/lib/ext/
cp: cannot stat `/usr/lib/mono/2.0/nunit.framework.dll': No such file or directory
error: Bad exit status from /var/tmp/rpm-tmp.23525 (%prep)

Secondly about to build on F-9 using koji, but first need to add ppc to
ExcludeArch because no nant on ppc currently.

Comment 2 Alex Lancaster 2008-04-12 13:01:36 UTC
Full review:

 - Package meets naming and packaging guidelines: YES
 - Spec file matches base package name: YES
 - Spec has consistant macro usage: YES
 - Meets Packaging Guidelines.: YES
 - License: GPLv2+ with exception
 - License field in spec matches: YES
 - License file included in package: YES
 - Spec in American English: YES
 - Spec is legible.: YES
 - Sources match upstream md5sum: no, because source needed to be rezipped
because of Windows separators:

md5sum SharpCvsLib-0.35.3721.507-src-unix.zip SharpCvsLib-0.35.3721.507-src.zip 
e480d341ff54298ddae2acbedf03d00b  SharpCvsLib-0.35.3721.507-src-unix.zip
968c3ee0333da891ee8f52e30e6b526e  /tmp/SharpCvsLib-0.35.3721.507-src.zip

 - Package needs ExcludeArch: ppc and ppc64 (no nant on those platforms)
 - BuildRequires correct: N/A
 - Spec handles locales/find_lang: N/A
 - Package is relocatable and has a reason to be. N/A
 - Package has %defattr and permissions on files is good. YES
 - Package has a correct %clean section. YES
 - Package has correct buildroot
	%(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
 - Package is code or permissible content.: YES
 - Doc subpackage needed/used. N/A
 - Packages %doc files don't affect runtime.: YES

 - Headers/static libs in -devel subpackage. Only .pc file
 - Spec has needed ldconfig in post and postun: N/A
 - .pc files in -devel subpackage/requires pkgconfig: YES
 - .so files in -devel subpackage. N/A
 - -devel package Requires: %{name} = %{version}-%{release} YES 

 - Package is a GUI app and has a .desktop file: N/A

 - Package compiles and builds on at least one arch., YES, only rawhide/F-9,
koji build:

http://koji.fedoraproject.org/koji/taskinfo?taskID=563289

 - Package has no duplicate files in %files. YES
 - Package doesn't own any directories other packages own. YES
 - Package owns all the directories it creates. YES
 - No rpmlint output.:  Some output, which is acceptable according to Mono
packaging guidelines:

rpmlint mono-sharpcvslib-*
mono-sharpcvslib.x86_64: E: no-binary
mono-sharpcvslib.x86_64: E: only-non-binary-in-usr-lib

 - final provides and requires are sane:  YES

Provides:

mono(ICSharpCode.SharpCvsLib) = 0.35.3721.507
mono(ICSharpCode.SharpCvsLib.Tests) = 0.0.0.0
mono(ICSharpCode.SharpCvsLib.Tests-sample) = 0.0.0.0
mono(cvs) = 0.35.3721.507
mono-sharpcvslib = 0.35-1.fc9

Requires:

mono(ICSharpCode.SharpCvsLib) = 0.35.3721.507
mono(ICSharpCode.SharpZipLib) = 2.84.0.0
mono(System) = 2.0.0.0
mono(System.Configuration) = 2.0.0.0
mono(System.EnterpriseServices) = 2.0.0.0
mono(System.Xml) = 2.0.0.0
mono(log4net) = 1.2.10.0
mono(mscorlib) = 2.0.0.0
mono(nunit.framework) = 2.2.0.0


SHOULD Items:

 - Should build in mock. YES
 - Should build on all supported archs: YES only i386 and x86_64 currently (see
koji build above)
 - Should function as described. NOT TESTED
 - Should have sane scriptlets. YES
 - Should have subpackages require base package with fully versioned depend. YES
 - Should have dist tag: YES
 - Should package latest version: It looks like 0.35 is the latest stable
version but it's from 2005, there is a "dev" version from 2005.  Does it fix any
outstanding issues with 0.35?

Issues:

1. Add ppc to ExcludeArch
2. See if BR's can be adjusted to build on F-8
3. nant currently provides mono(cvs) which it shouldn't, I assume that this
package will replace that requirement?

Only 1) is a blocker, 2) would be nice, but not critical for approval, since
this is necessary to get the nant/mono stack in a sane condition. 

Comment 3 Alex Lancaster 2008-04-12 13:17:44 UTC
Minor point, the spec file uses "c#" and "C#" in the Summary and Descriptions. 
I think the standard is "C#" capitalised.  Can these be fixed too?

Comment 4 Tom "spot" Callaway 2008-04-12 15:22:03 UTC
Sure. All items fixed (the "dev" version doesn't fix any outstanding issues that
I can see):

New SRPM:
http://www.auroralinux.org/people/spot/review/new/mono-sharpcvslib-0.35-2.fc9.src.rpm
New SPEC: http://www.auroralinux.org/people/spot/review/new/mono-sharpcvslib.spec

Comment 5 Alex Lancaster 2008-04-12 23:37:46 UTC
Looks good.   APPROVED.

Comment 6 Tom "spot" Callaway 2008-04-12 23:41:09 UTC
New Package CVS Request
=======================
Package Name: mono-sharpcvslib
Short Description: Client cvs library written in C#
Owners: spot
Branches: F-7 F-8 F-9 devel
InitialCC: 
Cvsextras Commits: yes


Comment 7 Tom "spot" Callaway 2008-04-12 23:45:31 UTC
cvs done.

Comment 8 Tom "spot" Callaway 2008-04-13 00:16:41 UTC
Built in F-9 and devel (earlier mono is... unhappy).