Bug 580422 - Review Request: libdesktop-agnostic - Provides an extensible configuration API
Summary: Review Request: libdesktop-agnostic - Provides an extensible configuration API
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Rex Dieter
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-04-08 08:52 UTC by leigh scott
Modified: 2010-08-09 21:57 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-08-09 21:57:19 UTC
rdieter: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description leigh scott 2010-04-08 08:52:14 UTC
Spec URL: http://fedoraforum.org/leigh123linux/review/libdesktop-agnostic/libdesktop-agnostic.spec

SRPM URL: http://fedoraforum.org/leigh123linux/review/libdesktop-agnostic/libdesktop-agnostic-0.3.9-bzr383.01.fc13.src.rpm

Description: This library provides an extensible configuration API.
A unified virtual file system API, and a desktop item editor


I would like to get this package approved as I need it to build the new of avant-window-navigator.
It hasn't been easy.

https://answers.launchpad.net/awn/+question/105656

Comment 3 Rex Dieter 2010-04-14 23:22:30 UTC
I'll take a look.

Comment 4 leigh scott 2010-04-15 00:52:38 UTC
(In reply to comment #3)
> I'll take a look.    

Thanks, I have updated it again ( I have removed some sub-packages )



 Spec URL:

http://fedoraforum.org/leigh123linux/review/libdesktop-agnostic/2/libdesktop-agnostic.spec

 SRPM URL:

http://fedoraforum.org/leigh123linux/review/libdesktop-agnostic/2/libdesktop-agnostic-0.3.90-3.fc13.src.rpm

Comment 5 Rex Dieter 2010-04-15 01:50:06 UTC
Initial comments:

1.  scriptlets.  the only (sub)pkg that appears to contain shared libraries is the main one, the others don't need any /sbin/ldconfig scriptlets

2.  the main package should own
%{_libdir}/desktop-agnostic/modules/

3.  dup'd copies of
%doc COPYING COPYING.GPL-2
in subpkgs aren't needed, at least those that already
Requires: %{name} ...
since these are already included there.

Comment 6 Rex Dieter 2010-04-15 01:51:19 UTC
Scratch build:
 http://koji.fedoraproject.org/koji/taskinfo?taskID=2116314

Comment 7 Rex Dieter 2010-04-15 01:55:14 UTC
4.  in %build , use
./waf -v build
instead.  It leads to the next issue

5.  using increased verbosity, it's clear $RPM_OPT_FLAGS arent being used here.  For example,
[ 51/119] cc: build/default/libdesktop-agnostic/ui-icon-chooser-dialog.c -> build/default/libdesktop-agnostic/ui-icon-chooser-dialog_5.o
20:55:13 runner system command -> ['/usr/lib64/ccache/gcc', '-ggdb', '-D_GNU_SOURCE', '-DHAVE_BUILD_CONFIG_H', '-fPIC', '-DPIC', '-Idefault', '-I..', '-I/home/rdieter1/cvs.fedoraproject.org/work/libagnostic/libdesktop-agnostic-0.3.90/build/default/libdesktop-agnostic', '-I/home/rdieter1/cvs.fedoraproject.org/work/libagnostic/libdesktop-agnostic-0.3.90/libdesktop-agnostic', '-I/home/rdieter1/cvs.fedoraproject.org/work/libagnostic/libdesktop-agnostic-0.3.90/build/default/', '-I/home/rdieter1/cvs.fedoraproject.org/work/libagnostic/libdesktop-agnostic-0.3.90', '-I/usr/include/glib-2.0', '-I/usr/lib64/glib-2.0/include', '-I/usr/include/gtk-2.0', '-I/usr/lib64/gtk-2.0/include', '-I/usr/include/pango-1.0', '-I/usr/include/cairo', '-I/usr/include/pixman-1', '-I/usr/include/freetype2', '-I/usr/include/libpng12', '-I/usr/include/atk-1.0', 'default/libdesktop-agnostic/ui-icon-chooser-dialog.c', '-c', '-o', 'default/libdesktop-agnostic/ui-icon-chooser-dialog_5.o']

Comment 8 Rex Dieter 2010-04-15 02:01:31 UTC
$ rpmlint *.rpm x86_64/*rpm
libdesktop-agnostic.src:57: W: mixed-use-of-spaces-and-tabs (spaces: line 6, tab: line 57)
gir1.0-desktopagnostic.x86_64: W: spelling-error Summary(en_US) GObject -> G Object, Object, Subject
gir1.0-desktopagnostic.x86_64: W: spelling-error Summary(en_US) libdesktop -> lib desktop, lib-desktop, desktop
gir1.0-desktopagnostic.x86_64: W: only-non-binary-in-usr-lib
libdesktop-agnostic.x86_64: W: non-conffile-in-etc /etc/xdg/libdesktop-agnostic/desktop-agnostic.ini
python-desktop-agnostic.x86_64: W: spelling-error Summary(en_US) libdesktop -> lib desktop, lib-desktop, desktop
10 packages and 0 specfiles checked; 0 errors, 6 warnings.

6.  SHOULD use
%config /etc/xdg/libdesktop-agnostic/desktop-agnostic.ini
(unless there's some good reason not to).

Comment 12 Rex Dieter 2010-04-21 16:06:29 UTC
I like the simplification of the packaging and subpkgs.

wrt item 6, rpmlint notes now,
libdesktop-agnostic.x86_64: W: conffile-without-noreplace-flag /etc/xdg/libdesktop-agnostic/desktop-agnostic.ini

so maybe better to use
%config(noreplace)
so that end-user modifications don't get lost on upgrades (unless there's good reason to not preserve them).


nitty gritty,

naming: ok

macros: ok

licensing: ok

sources: ok
$ md5sum *.gz
67141440b7fa77e2d704e7368c067212  libdesktop-agnostic-0.3.90.tar.gz

dir ownership: ok (well, looks like ownership of %{_libdir}/desktop-agnostic/modules , got lost, re-add that to the main pkg please)

static libs: ok

libtool archives: ok


APPROVED.  (I'll trust you to address the dir ownership and %config items above prior to releasing anything).

Comment 13 leigh scott 2010-04-21 16:23:44 UTC
New Package CVS Request
=======================
Package Name: libdesktop-agnostic
Short Provides an extensible configuration API
Owners: leigh123linux
Branches: F-11 F-12 F-13
InitialCC: leigh123linux

Comment 14 leigh scott 2010-04-21 16:37:45 UTC
(In reply to comment #12)
> 
> 
> APPROVED.  (I'll trust you to address the dir ownership and %config items above
> prior to releasing anything).    



Thank you for the review, I will address the last two points when I commit to CVS.

Comment 15 Kevin Fenzi 2010-04-24 00:37:54 UTC
Your "Short Description:" line seems malformed and is causing our cvs processing
script to complain. ;) Can you please add a new cvs request with that corrected?

Comment 16 leigh scott 2010-04-24 04:58:01 UTC
New Package CVS Request
=======================
Package Name: libdesktop-agnostic
Short Description: Provides an extensible configuration API
Owners: leigh123linux
Branches: F-11 F-12 F-13
InitialCC: leigh123linux

Comment 17 Kevin Fenzi 2010-04-24 05:01:35 UTC
CVS done (by process-cvs-requests.py).


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