Bug 613067 - Review Request:spice - Implements the SPICE protocol
Review Request:spice - Implements the SPICE protocol
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Matthias Clasen
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2010-07-09 11:57 EDT by Gerd Hoffmann
Modified: 2010-07-13 08:39 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-07-13 08:39:07 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
mclasen: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Gerd Hoffmann 2010-07-09 11:57:38 EDT
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 12:05:54 EDT
Note that this package depends on celt051 (bug #612979) and spice-protocol (bug #612943).
Comment 2 Matthias Clasen 2010-07-09 17:40:35 EDT
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 17:48:51 EDT
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 19:10:25 EDT
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 03:31:37 EDT
Created bug #613529 for the portability issues which make spice x86 only.
Comment 6 Gerd Hoffmann 2010-07-12 05:48:40 EDT
Updated packages + specfile uploaded to
http://kraxel.fedorapeople.org/review/spice/
Comment 7 Matthias Clasen 2010-07-12 10:37:39 EDT
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 11:07:47 EDT
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 13:26:53 EDT
CVS done (by process-cvs-requests.py).
Comment 10 Chen Lei 2010-07-13 05:23:25 EDT
%changelog
 
* Mon Jul 12 2010 Gerd Hoffmann <kraxel@redhat.com> - 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@redhat.com> - 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 08:39:07 EDT
rawhide builds are done.

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