Bug 506825 - Review Request: bickley - A meta data management API and framework
Review Request: bickley - A meta data management API and framework
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Christoph Wickert
Fedora Extras Quality Assurance
:
Depends On: 507392
Blocks: FedoraMoblin20 506855 518446
  Show dependency treegraph
 
Reported: 2009-06-18 15:28 EDT by Peter Robinson
Modified: 2009-09-01 09:13 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-09-01 09:13:49 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
cwickert: fedora‑review+
tibbs: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Peter Robinson 2009-06-18 15:28:50 EDT
SPEC: http://pbrobinson.fedorapeople.org/bickley.spec
SRPM: http://pbrobinson.fedorapeople.org/bickley-0.2.0-1.fc11.src.rpm

Bickley is a meta data management API and framework. The core API allows 
storing and querying of URIs and associating key/value pairs. The core bickley
API in the libbickley library provides shared access to common meta-data
storage. Different processes can access and manipulate the same set of URIs and
meta data about them in parallel.

-------------

A couple of notes. It doesn't currently compile on PPC so its currently ExclusiveArch
http://bugzilla.moblin.org/show_bug.cgi?id=3643

It doesn't currently compile on rawhide due to the breakage in clutter*

http://koji.fedoraproject.org/koji/taskinfo?taskID=1423368
Comment 1 Bastien Nocera 2009-06-18 16:49:06 EDT
#undef HAVE_IO_PRIO
#if defined (__i386__)
#  define HAVE_IO_PRIO
#  define __NR_ioprio_set 289
#elif defined (__x86_64__)
#  define HAVE_IO_PRIO
#  define __NR_ioprio_set 251
#else
#  warn "Architecture does not support ioprio modification"
#endif

Change that to a "#warning" instead of "#warn" and you should be ready to go.
Comment 2 Peter Robinson 2009-06-18 17:40:57 EDT
Thanks Bastien! Compiles now on PPC :-)

SPEC as before
SRPM: http://pbrobinson.fedorapeople.org/bickley-0.2.0-2.fc11.src.rpm
Comment 4 Peter Robinson 2009-08-06 18:29:39 EDT
Updated and cleaned up the spec file

SPEC: same as above
SRPM: http://pbrobinson.fedorapeople.org/bickley-0.4.3-2.fc11.src.rpm

koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1587916
Comment 6 Christoph Wickert 2009-08-08 11:59:03 EDT
REVIEW FOR 
2aecdc6fffa19a097537cca3b8a775e7  bickley-0.4.3-3.fc11.src.rpm

OK - MUST: rpmlint /var/lib/mock/fedora-rawhide-x86_64/result/bickley-*
bickley.x86_64: W: non-conffile-in-etc /etc/gconf/schemas/bkl-orbiter.schemas
bickley-devel.x86_64: W: no-documentation
4 packages and 0 specfiles checked; 0 errors, 2 warnings.
(gconf schemas are no config files as they are not expected to be edited)
OK - MUST: named according to the Package Naming Guidelines
OK - MUST: spec file name matches the base package %{name}
OK - MUST: package meets the Packaging Guidelines
OK - MUST: Fedora approved license and meets the Licensing Guidelines: LGPLv2.1
??? - MUST: License field in spec does not match the actual license: GPLv2 vs. LGPLv2
OK - MUST: license file included in %doc
OK - MUST: spec is in American English
OK - MUST: spec is legible
OK - MUST: sources match the upstream source by MD5 0154c4813294c4fe76690795c7efd8ac
OK - MUST: successfully compiles and builds into binary rpms on all primary arches
OK - MUST: all build dependencies are listed in BuildRequires.
N/A - MUST: handles locales properly with %find_lang
OK - MUST: Every binary RPM package (or subpackage) which stores shared library files (not just symlinks) in any of the dynamic linker's default paths, must call ldconfig in %post and %postun.
OK - MUST: not relocatable
OK - MUST: owns all directories that it creates
OK - MUST: no duplicate files in the %files listing
OK - MUST: permissions on files are set properly, includes %defattr(...)
OK - MUST: package has a %clean section, which contains rm -rf %{buildroot}.
OK - MUST: consistently uses macros
OK - MUST: package contains code
N/A - MUST: Large documentation files should go in a -doc subpackage
OK - MUST: Files included as %doc do not affect the runtime of the application
OK - MUST: Header files are in a -devel package
N/A - MUST: Static libraries must be in a -static package
OK - MUST: - devel package 'Requires: pkgconfig'.
OK - MUST: library files that end in .so (without suffix) are in -devel package.
OK - MUST: -devel package requires base package using a fully versioned dependency: Requires: %{name} = %{version}-%{release}
OK - MUST: The package does not contain any .la libtool archives.
OK - MUST: No GUI app, no desktop file.
OK - MUST: packages does not own files or directories already owned by other packages.
OK - MUST: at the beginning of %install, the package runs rm -rf %{buildroot}.
OK - MUST: all filenames valid UTF-8


SHOULD Items:
OK - SHOULD: Source package includes license text(s) as a separate file.
N/A - SHOULD: The description and summary sections in the package spec file should contain translations for supported Non-English languages, if available.
OK - SHOULD: The the package builds in mock.
OK - SHOULD: The package should compile and build into binary rpms on all supported architectures.
OK - SHOULD: functions as described.
FIX - SHOULD: If scriptlets are used, those scriptlets must be sane.
N/A - SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency.
OK - SHOULD: pkgconfig(.pc) files are in -devel pkg.
N/A - SHOULD: If the package has file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the file instead of the file itself.


Other items:
OK - latest stable version packaged
OK - proper debuginfo package
OK - compiler flags honored
OK - timstamps preserved during install


Issues:
- Missing Requirements:
Requrires: dbus (only dbus-libs will be pulled in but not)
Requires(pre): GConf2
Requires(post): GConf2
Requires(post): /sbin/ldconfig
Requires(preun): GConf2
see https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#GConf

- devel package Requires: gtk-doc, but I see no gtk doc files

- %configure with -disable-schemas to avoid this build related error: 
/bin/sh: --makefile-install-rule: command not found
Warning: No permission to install schema files, install manually
add 

- %configure with --disable-rpath instead of the seed hack (tested and works)

- License is unclear. Spec: GPLv2+, COPYING is GPLv2 but the headers of the sources are LGPLv2. This looks like a mistake to me. Please ask upstream for clarification.
Comment 7 Christoph Wickert 2009-08-08 12:03:54 EDT
There is an issue with bickley-0.4.pc on 64 bit:

prefix=/usr
exec_prefix=${prefix}
libdir=${exec_prefix}/lib 
includedir=${prefix}/include

libdir should be libdir64
Comment 8 Christoph Wickert 2009-08-08 12:04:36 EDT
(In reply to comment #7)
 
> libdir should be libdir64  

... should be lib64
Comment 9 Peter Robinson 2009-08-10 06:32:03 EDT
License clarification sent upstream here, awaiting response:
http://lists.moblin.org/pipermail/dev/2009-August/005755.html

pkgconfig issue patched and bug filed to send patch upstream
http://bugzilla.moblin.org/show_bug.cgi?id=5295

All other issues are addressed. New package here:
SRPM: http://pbrobinson.fedorapeople.org/bickley-0.4.3-4.fc11.src.rpm

Thanks!
Comment 10 Peter Robinson 2009-08-13 06:59:25 EDT
(In reply to comment #7)
> There is an issue with bickley-0.4.pc on 64 bit:
> 
> prefix=/usr
> exec_prefix=${prefix}
> libdir=${exec_prefix}/lib 
> includedir=${prefix}/include
> 
> libdir should be libdir64  

The patch I provided has now been committed to upstream git. Will be in the next release.
Comment 11 Peter Robinson 2009-08-14 06:34:01 EDT
> - License is unclear. Spec: GPLv2+, COPYING is GPLv2 but the headers of the
> sources are LGPLv2. This looks like a mistake to me. Please ask upstream for
> clarification.  

Got clarification from the developer that the license is LGPL2+. Email on the list is here http://lists.moblin.org/pipermail/dev/2009-August/005801.html
Comment 12 Peter Robinson 2009-08-14 15:28:26 EDT
I've now reviewed the outstanding items and I think they're all fixed. Patch from upstream for the license applied and patch for the pgkconfig issue.

SRPM: http://pbrobinson.fedorapeople.org/bickley-0.4.3-5.fc11.src.rpm
koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=1606207
Comment 13 Michel Alexandre Salim 2009-08-21 19:12:50 EDT
Christopher?
Comment 14 Christoph Wickert 2009-08-21 19:34:11 EDT
Sorry, busy with Fedora as FROSCon this weekend. Please wait till Monday.
Comment 15 Peter Robinson 2009-08-26 14:27:57 EDT
Hi Christoph, any update?
Comment 16 Christoph Wickert 2009-08-29 09:46:34 EDT
Sorry for the delay, it was a busy week for me.

Good:
- Requires/Requires(pre,post,postun) are sane
- Requires of devel package are sane to, gtk-doc was dropped
- License was clarified, license tag matches
- bickley-0.4.pc fixed for 64 bit


Bad:
- Some redundant BRs that *could* (not must) be removed:
clutter-devel and clutter-gtk-devel require gtk2-devel
clutter-gtk-devel and clutter-gst-devel require clutter-devel

- I wonder if "Applications/Multimedia" really is the proper group tag for a framework.

- Are sqlite and xdg-user-dirs really required? I dont't see them referenced in the source

- %configure is still run twice.

- %configure correctly uses --disable-rpath and --disable-schemas, but now during build I see:

configure: WARNING: unrecognized options: --enable-maintainer-mode
...
configure: WARNING: unrecognized options: --disable-rpath, --disable-schemas

the first is a bug in autogen.sh and should be fixed upstream, but the second ones need fixing in the package. --disable-shemas can be dropped as you already have
export GCONF_DISABLE_MAKEFILE_SCHEMA_INSTALL=1
in %install

This leaves --disable-rpath. I'm pretty sure I checked that it works, but AFAIKS it can be dropped too as the applications contain no rpath (tested with chrpath).
Comment 17 Peter Robinson 2009-08-31 12:09:20 EDT
(In reply to comment #16)
> Sorry for the delay, it was a busy week for me.

NP: know the feeling all too well.

> Bad:
> - Some redundant BRs that *could* (not must) be removed:
> clutter-devel and clutter-gtk-devel require gtk2-devel
> clutter-gtk-devel and clutter-gst-devel require clutter-devel

Dropped, added gstreamer-devel to the list.

> - I wonder if "Applications/Multimedia" really is the proper group tag for a
> framework.

I couldn't find a better one and that's what gstreamer uses.

> - Are sqlite and xdg-user-dirs really required? I dont't see them referenced in
> the source

Dropped.

> - %configure is still run twice.

I think this is fixed. Seems there's a number of different ways to fix this, the cleanest being the one that's part of gnome-common.

> - %configure correctly uses --disable-rpath and --disable-schemas, but now
> during build I see:
>
> configure: WARNING: unrecognized options: --enable-maintainer-mode
> ...
> configure: WARNING: unrecognized options: --disable-rpath, --disable-schemas
> 
> the first is a bug in autogen.sh and should be fixed upstream, but the second
> ones need fixing in the package. --disable-shemas can be dropped as you already
> have
> export GCONF_DISABLE_MAKEFILE_SCHEMA_INSTALL=1
> in %install
> 
> This leaves --disable-rpath. I'm pretty sure I checked that it works, but
> AFAIKS it can be dropped too as the applications contain no rpath (tested with
> chrpath).  

Fixed. The rpath one was added at some point I think when a previous build needed it. I'll bring up the the --enable-maintainer-mode issue with upstream.

SPEC: as above
SRPM: http://pbrobinson.fedorapeople.org/bickley-0.4.3-6.fc11.src.rpm
koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=1646385
Comment 18 Christoph Wickert 2009-08-31 18:49:20 EDT
OK: Requires: sqlite dropped
OK: BuildRequires are correct with gnome-common
OK: configure options are same
OK: functions as described, although I noticed that blk-orbiter uses quite some CPU cycles when I plugged in a 200 GB USB HDD full of music. But I guess this is the normal indexing.

(In reply to comment #17)
> Dropped, added gstreamer-devel to the list.

Huh? I don't see gstreamer-devel in the spec and I doubt it needed 
You mean clutter-gst-devel, right?

> I couldn't find a better one and that's what gstreamer uses.

How about System Environment/Daemons or User Interface/Desktops? Doesn't really matter, bickley-0.4.3-6.fc11.src.rpm is APPROVED.
Comment 19 Peter Robinson 2009-08-31 18:59:54 EDT
Thank you as always Christoph!

New Package CVS Request
=======================
Package Name: bickley
Short Description: A meta data management API and framework
Owners: pbrobinson
Branches: F-11
InitialCC:
Comment 20 Jason Tibbitts 2009-08-31 19:19:01 EDT
CVS done.
Comment 21 Peter Robinson 2009-08-31 19:57:49 EDT
Imported and built in rawhide
Comment 22 Peter Robinson 2009-09-01 09:13:49 EDT
In rawhide. Thanks again!

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