Bug 248120 - Review Request: soprano - Qt wrapper API to different RDF storage solutions
Review Request: soprano - Qt wrapper API to different RDF storage solutions
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Kevin Kofler
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-07-13 08:00 EDT by Rex Dieter
Modified: 2007-11-30 17:12 EST (History)
2 users (show)

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


Attachments (Terms of Use)

  None (edit)
Description Rex Dieter 2007-07-13 08:00:54 EDT
Spec URL: http://kdeforge.unl.edu/apt/kde-redhat/SOURCES/soprano/devel/soprano.spec
SRPM URL: http://kdeforge.unl.edu/apt/kde-redhat/SOURCES/soprano/devel/soprano-0.9.0-1.src.rpm
Description:
Qt wrapper API to different RDF storage solutions
Comment 1 Rex Dieter 2007-07-13 08:02:49 EDT
As promised Kevin... :)
Comment 2 Kevin Kofler 2007-07-13 19:01:55 EDT
MUST Items:
+ rpmlint output OK:
  SRPM, soprano, soprano-debuginfo produce no warnings
  W: soprano-devel no-documentation - see "relevant documentation" below
+ named and versioned according to the Package Naming Guidelines
+ spec file name matches base package name
+ Packaging Guidelines:
  + License LGPL OK, matches actual license
  + No known patent problems
  + No emulator, no firmware, no binary-only or prebuilt components
  + Complies with the FHS
  + proper changelog, tags, BuildRoot, Requires, BuildRequires, Summary, 
Description
  + no non-UTF-8 characters
  ! relevant documentation is included
    While the minimum documentation is present, the upstream package also 
supports generating Doxygen documentation, which would be useful to package (in 
a -apidocs subpackage).
  + RPM_OPT_FLAGS are used (%cmake macro)
  + debuginfo package is valid
  + no static libraries nor .la files
  + no duplicated system libraries
  + no rpaths, at least on i386 (I ran readelf -d on the shared objects)
  + no configuration files, so %config guideline doesn't apply
  + no init scripts, so init script guideline doesn't apply
  + no executables, so no .desktop file present or needed
  + no timestamp-clobbering file commands
  + _smp_mflags used
  + scriptlets are valid
  + not a web application, so web application guideline doesn't apply
  + no conflicts
+ complies with all the legal guidelines
+ COPYING included as %doc
+ spec file written in American English
+ spec file is legible
+ source matches upstream:
  MD5SUM: 703f11ca18f50c500b62cd44c84145e5
  SHA1SUM: df2179aa29eb1a7d4d21e82ef94699dfc497f388
+ builds on at least one arch (F7 i386 live system)
+ no known non-working arches, so no ExcludeArch needed
+ all build dependencies listed in CMakeLists.txt are listed in BuildRequires
  (However, an additional BuildRequires: doxygen will be needed for 
the -apidocs.)
+ no translations in original tarball, so translation/locale guidelines don't 
apply
+ ldconfig correctly called in %post and %postun
+ package not relocatable
+ ownership correct (owns package-specific directories, doesn't own directories 
owned by another package)
+ no duplicate files in %files
+ permissions set properly
+ %clean section present and correct
+ macros used where possible
+ no non-code content
+ no large documentation files, so no -doc package needed
+ %doc files not required at runtime
+ all header files in -devel
+ no static libraries, so no -static package needed
+ no .pc files, so no Requires: pkgconfig needed
+ /usr/lib/*.so symlink is correctly in -devel
+ /usr/lib/soprano/*.so plugin (NOT a symlink) is correctly NOT in -devel
+ -devel requires %{name} = %{version}-%{release} 
+ no .la files
+ no GUI programs (in fact, no executables at all), so no .desktop file needed
+ buildroot is deleted at the beginning of %install
  But I strongly recommend a:
  mkdir $RPM_BUILD_ROOT
  after the:
  rm -rf $RPM_BUILD_ROOT
  to prevent a potential symlink attack as pointed out by the OpenSUSE folks.
+ all filenames are valid UTF-8

SHOULD Items: 
+ license already included upstream
+ no translations for description and summary provided by upstream
! I get a weird error in mock (tested FC6 i386 with Plague results and Rawhide 
i386, both on F7 build host):
  + %cmake .
  /var/tmp/rpm-tmp.20248: line 28: fg: no job control
  error: Bad exit status from /var/tmp/rpm-tmp.20248 (%build)
  I don't know if this is my fault or if something's screwed in your package or 
the %cmake macro.
* Skipping the "all architectures" test, I only have i386.
* Can't really test that the package functions as described without building 
some KDE 4 stuff against it first, skipping for now.
+ scriptlets are sane
+ no subpackages other than -devel, so "Usually, subpackages other than devel 
should require the base package using a fully versioned dependency." is 
irrelevant
+ no .pc files, so "placement of .pc files" is irrelevant
+ no file dependencies

I see no real blockers except the strange error in mock. Thus, consider this 
APPROVED if you can get it to build in mock.

However:
1. It would be useful to package the Doxygen documentation:
* add BuildRequires: doxygen
* create an -apidocs subpackage
2. I'd also suggest adding the mkdir $RPM_BUILD_ROOT.
Comment 3 Kevin Kofler 2007-07-13 19:06:47 EDT
Uhm, duh, no wonder it doesn't build in mock, you're missing the BuildRequires: 
cmake!

So please add this before importing.
Comment 4 Kevin Kofler 2007-07-13 19:19:58 EDT
Just to confirm:

After adding the:
BuildRequires: cmake
the package builds in mock for both Rawhide and FC6, so with this change, it is 
APPROVED (but see the 2 suggested changes).
Comment 5 Mamoru TASAKA 2007-07-13 20:36:10 EDT
... and make the compilation log more verbose (i.e.
add VERBOSE=1)
Comment 6 Kevin Kofler 2007-07-13 20:37:37 EDT
How would that be useful? We don't need useless console spewage. Errors are 
already printed as it is.
Comment 7 Rex Dieter 2007-07-15 14:15:28 EDT
New Package CVS Request
=======================
Package Name: soprano
Short Description: Qt wrapper API to different RDF storage solutions
Owners: rdieter@math.unl.edu, kevin@tigcc.ticalc.org
Branches: F-7 FC-6
InitialCC: 
Comment 8 Rex Dieter 2007-07-15 14:19:37 EDT
%changelog
* Sun Jul 15 2007 Rex Dieter <rdieter[AT]fedoraproject.org> 0.9.0-2
- BR: cmake (doh)
Comment 9 Rex Dieter 2007-07-15 16:02:14 EDT
per comment #6, only devel/ and no FC-6/F-7 branches were created.  Am I missing
something?
Comment 10 Rex Dieter 2007-07-15 16:50:41 EDT
Not sure if it's related, but I can't build in devel/ either, koji reports error:
BuildError: package soprano not in list for tag dist-f8
??
http://koji.fedoraproject.org/koji/taskinfo?taskID=67102
Comment 11 Kevin Kofler 2007-07-15 16:53:46 EDT
That generally happens for new packages where the information hasn't been 
propagated to Koji yet (I had that happen to me with redland too), however it 
shouldn't take more than an hour for the information to propagate.
Comment 12 Rex Dieter 2007-07-16 08:30:31 EDT
fyi, koji devel/ builds succeeded this (the next) morning, non-devel branches
still awol.
Comment 13 Kevin Fenzi 2007-07-16 14:27:05 EDT
Added the F-7 and FC-6 branches. 
Comment 14 Kevin Kofler 2007-07-17 01:48:19 EDT
Package Change Request
======================
Package Name: soprano
Updated Fedora Owners: rdieter@math.unl.edu,Kevin@tigcc.ticalc.org

My e-mail address is entered with the capital 'K' in the account system so the 
ACL system doesn't give me access when it's in owners.list with a 
lowercase 'k'. This mess is mainly because Bugzilla now lowercases all the 
e-mail addresses, it didn't do that when I registered my Fedora account.
Comment 15 Kevin Kofler 2007-07-17 01:55:03 EDT
Scrap the above, I changed it to lowercase in the account system for 
consistency, I'm going to request it changed to lowercase elsewhere instead.

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