Bug 506825 - Review Request: bickley - A meta data management API and framework
Summary: Review Request: bickley - A meta data management API and framework
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Christoph Wickert
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 507392
Blocks: FedoraMoblin20 506855 518446
TreeView+ depends on / blocked
 
Reported: 2009-06-18 19:28 UTC by Peter Robinson
Modified: 2009-09-01 13:13 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-09-01 13:13:49 UTC
Type: ---
Embargoed:
christoph.wickert: fedora-review+
j: fedora-cvs+


Attachments (Terms of Use)

Description Peter Robinson 2009-06-18 19:28:50 UTC
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 20:49:06 UTC
#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 21:40:57 UTC
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 22:29:39 UTC
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 15:59:03 UTC
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 16:03:54 UTC
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 16:04:36 UTC
(In reply to comment #7)
 
> libdir should be libdir64  

... should be lib64

Comment 9 Peter Robinson 2009-08-10 10:32:03 UTC
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 10:59:25 UTC
(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 10:34:01 UTC
> - 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 19:28:26 UTC
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 Lind 2009-08-21 23:12:50 UTC
Christopher?

Comment 14 Christoph Wickert 2009-08-21 23:34:11 UTC
Sorry, busy with Fedora as FROSCon this weekend. Please wait till Monday.

Comment 15 Peter Robinson 2009-08-26 18:27:57 UTC
Hi Christoph, any update?

Comment 16 Christoph Wickert 2009-08-29 13:46:34 UTC
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 16:09:20 UTC
(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 22:49:20 UTC
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 22:59:54 UTC
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 23:19:01 UTC
CVS done.

Comment 21 Peter Robinson 2009-08-31 23:57:49 UTC
Imported and built in rawhide

Comment 22 Peter Robinson 2009-09-01 13:13:49 UTC
In rawhide. Thanks again!


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