Bug 613067 - Review Request:spice - Implements the SPICE protocol
Summary: Review Request:spice - Implements the SPICE protocol
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Matthias Clasen
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-07-09 15:57 UTC by Gerd Hoffmann
Modified: 2010-07-13 12:39 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-07-13 12:39:07 UTC
Type: ---
Embargoed:
mclasen: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Gerd Hoffmann 2010-07-09 15:57:38 UTC
Spec URL: http://kraxel.fedorapeople.org/review/spice/spice.spec
SRPM URL: http://kraxel.fedorapeople.org/review/spice/spice-0.5.2-1.fc14.src.rpm
Description:
The Simple Protocol for Independent Computing Environments (SPICE) is
a remote display system built for virtual environments which allows
you to view a computing 'desktop' environment not only on the machine
where it is running, but from anywhere on the Internet and from a wide
variety of machine architectures.

Comment 1 Gerd Hoffmann 2010-07-09 16:05:54 UTC
Note that this package depends on celt051 (bug #612979) and spice-protocol (bug #612943).

Comment 2 Matthias Clasen 2010-07-09 21:40:35 UTC
Builds fine in mock.
rpmlint output:

rpmlint /var/lib/mock/fedora-rawhide-x86_64/result/*.rpm
spice.src: W: name-repeated-in-summary C SPICE
spice-client.x86_64: W: no-manual-page-for-binary spicec
5 packages and 0 specfiles checked; 0 errors, 2 warnings.

Comment 3 Matthias Clasen 2010-07-09 21:48:51 UTC
Looking at the license situation:

- the included COPYING is LGPL
- the majority of sources seem to say LGPL, some say GPL, and then there's eg server/jpeg_encoder.h which looks BSD
- the spec file says GPL

Comment 4 Matthias Clasen 2010-07-09 23:10:25 UTC
package name: ok
spec file name: ok
packaging guidelines:
 small cleanups are possible if the package is only for F13+:
 - remove BuildRoot
 - remove initial rm -rf in %install
 - ditch %clean
 things that need fixing:
 - ExclusiveArch: should have a comment explaining why the package only 
   works on some arches and ideally a bug ref. See https://fedoraproject.org/wiki/Packaging/Guidelines#Architecture_Build_Failures
 - the CFLAGS munging needs justification in a comment. Why remove -Wall ? and in particular, why remove _FORTIFY_SOURCE ? See https://fedoraproject.org/wiki/Packaging/Guidelines#Compiler_flags
 - if libspice-server.a must be packaged, it needs to go into a -static subpackage. See https://fedoraproject.org/wiki/Packaging/Guidelines#Packaging_Static_Libraries
license: ok
license field/license file: see previous comment
spec file language: ok
spec file readable: ok
buildable: ok
ExcludeArch: see above
BuildRequires: ok
locale handling: ok
ldconfig: ok
system libraries: ok
relocatable: ok
directory ownership: ok
duplicate files: ok
file permissions: ok. (pedants prefer the 4-argument form of %defattr)
macro use: ok
permissable content: ok
large docs: ok
%doc content: ok
headers: ok
shared libs: ok
static libs: see above
devel deps: ok
libtool archives: ok
gui apps: ok
file ownership: ok
utf8 filenames: ok

Comment 5 Gerd Hoffmann 2010-07-12 07:31:37 UTC
Created bug #613529 for the portability issues which make spice x86 only.

Comment 6 Gerd Hoffmann 2010-07-12 09:48:40 UTC
Updated packages + specfile uploaded to
http://kraxel.fedorapeople.org/review/spice/

Comment 7 Matthias Clasen 2010-07-12 14:37:39 UTC
Setting CFLAGS as you do now is probably a nop, looking at the definition of %configure:

%configure \
  CFLAGS="${CFLAGS:-%optflags}" ; export CFLAGS ; \
  CXXFLAGS="${CXXFLAGS:-%optflags}" ; export CXXFLAGS ; \
...

but that is not a big deal. The rest looks fine now. Approved

Comment 8 Gerd Hoffmann 2010-07-12 15:07:47 UTC
New Package CVS Request
=======================
Package Name: spice
Short Description: Implements the SPICE protocol
Owners: kraxel alexl
Branches: 
InitialCC:

Comment 9 Kevin Fenzi 2010-07-12 17:26:53 UTC
CVS done (by process-cvs-requests.py).

Comment 10 Chen Lei 2010-07-13 09:23:25 UTC
%changelog
 
* Mon Jul 12 2010 Gerd Hoffmann <kraxel> - 0.5.2-3
 
- %configure handles CFLAGS automatically, no need to fiddle
 
with %{optflags} manually.
 
should be changed to

* Mon Jul 12 2010 Gerd Hoffmann <kraxel> - 0.5.2-3
 
- %%configure handles CFLAGS automatically, no need to fiddle
 
with %%{optflags} manually.
 
See http://koji.fedoraproject.org/koji/buildinfo?buildID=183372

Comment 11 Gerd Hoffmann 2010-07-13 12:39:07 UTC
rawhide builds are done.


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