Bug 442693 - Review Request: geoclue - Geoclue is a modular geoinformation service
Review Request: geoclue - Geoclue is a modular geoinformation service
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Extras Quality Assurance
: Reopened
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-04-16 06:21 EDT by Peter Robinson
Modified: 2008-05-18 17:06 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-05-18 17:06:04 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mclasen: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Peter Robinson 2008-04-16 06:21:47 EDT
Spec URL: http://fedora.roving-it.com/rawhide/geoclue.spec
SRPM URL: http://fedora.roving-it.com/rawhide/geoclue-0.10-0.1.20080415git.fc9.src.rpm
Description: Geoclue is a modular geoinformation service built on top of the D-Bus messaging system. The goal of the Geoclue project is to make creating location-aware applications as simple as possible.
Comment 1 Matthias Clasen 2008-04-25 09:44:46 EDT
Hey, thanks for picking this up, it was on my list of things to look at...


Some preliminary comments on the spec file:


This looks wrong, only devel packages should require devel packages:

Requires: gtk2-devel 


These will almost certainly be pulled in automatically be library dependencies,
so no need to list them explicitly:
 
Requires: glib2
Requires: libxml2
Requires: GConf2


The devel package must require pkgconfig, since it contains a pc file.


The description is misleading, since the devel package should _not_ contain
static libraries. It should contain the unversioned symlinks (.so) for shared
libraries.


Running autogen.sh should only be necessary when building from a checkout. Even
if this package is based on a git snapshot right now, the tarball should be
produced by 'make dist' and the autogen.sh call should be unnecessary. If it
were necessary for some reason, then you are missing the necessary BuildRequires
for automake, autoconf, etc


missing from the file list:
%dir %{_datadir}/geoclue-providers
%dir %{_includedir}/geoclue


Comment 2 Matthias Clasen 2008-04-25 10:04:16 EDT
As expected, trying to build your srpm in mock fails with

which: no libtoolize in
(/usr/lib/ccache:/usr/local/sbin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/X11R6/bin:/root/bin:/builddir/bin)
libtoolize not found. Please install it.

due to the missing BuildRequires for the autogen.sh call
Comment 3 Peter Robinson 2008-04-25 16:57:34 EDT
No problems. Been looking for some packages to maintain for a while to get
involved :)

geoclue has just done another release so the autogen.sh stuff can disappear, but
there's a missing file from the tar ball as far as I can tell so I'll upload a
new version once I can get it to build.

For the static libraries do I just rm them from the build?
Comment 5 Matthias Clasen 2008-04-25 17:27:27 EDT
with static libraries, do you mean .la files ? those are actually libtool
archives, and yes, rm is the right treatment for them. Actual static libaries
are .a files, but you are configuring with --disable-static, so those should not
be produced in the first place. I they are, then again rm is the easiest solution.
Comment 6 Peter Robinson 2008-04-25 17:37:41 EDT
the .la files are removed in the last spec file so I think I've got the
requirements now.
Comment 7 Matthias Clasen 2008-04-25 19:00:09 EDT
After adding 

BuildRequires: gtk2-devel 
BuildRequires: NetworkManager-devel

I was able to build this in mock, so we are getting somewhere. rpmlint has some
complaints: 

geoclue.i386: E: zero-length /usr/share/doc/geoclue-0.11.1/ChangeLog
geoclue.i386: E: library-without-ldconfig-postin /usr/lib/libgeoclue.so.0.0.0
geoclue.i386: E: library-without-ldconfig-postun /usr/lib/libgeoclue.so.0.0.0
geoclue.i386: E: zero-length /usr/share/doc/geoclue-0.11.1/NEWS
geoclue.i386: E: description-line-too-long Geoclue is a modular geoinformation
service built on top of the D-Bus messaging system. The goal of the Geoclue
project is to make creating location-aware applications as simple as possible.
geoclue.i386: W: non-standard-group System
geoclue.i386: W: invalid-license LGPL

all of these are easy fixes, thankfully. 
Comment 8 Peter Robinson 2008-04-25 19:40:07 EDT
OK, I think I've fixed all of those errors. I've put the main library rpm in the
"System Environment/Libraries" but not sure what group the test gui should go
in. Maybe its just late but I can't see anything in the packaging guidelines
about groups.

SPEC: http://fedora.roving-it.com/rawhide/geoclue.spec
SRPM: http://fedora.roving-it.com/rawhide/geoclue-0.11.1-2.fc9.src.rpm
Comment 9 Matthias Clasen 2008-04-25 20:34:04 EDT
Yeah, looks fine now, basically. For rpm groups, I would just pick something like
Development/Libraries, since the demo is about showing how to use the geoclue
libraries.

I'll do a formal review sometime this weekend.
Comment 10 Peter Robinson 2008-04-26 04:47:42 EDT
Fixed up the group for the test gui, also removed the gtk requirement as it
should get it automatically.

SPEC: http://fedora.roving-it.com/rawhide/geoclue.spec
SRPM: http://fedora.roving-it.com/rawhide/geoclue-0.11.1-3.fc9.src.rpm

Thanks for the feedback.
Comment 11 Matthias Clasen 2008-04-28 09:13:37 EDT
Builds fine in mock. 

[mclasen@localhost Desktop]$ rpmlint
/var/lib/mock/fedora-9-i386/result/geoclue-*.rpm
geoclue-devel.i386: W: no-documentation
geoclue-gui.i386: W: no-documentation

Both of which are ignorable warnings.


package name: ok
spec file name: ok
packaging guidelines: ok
license: ok
license field: ok
license file: ok
spec file language: ok
spec file legibility: outstanding
upstream sources: ok
buildable: yes
excludearch: n/a
build deps: ok
locale handling: ok
ldconfig: ok
relocatable: n/a
directory ownership: ok
duplicate files: ok
file permissions: ok
%clean: ok
content: permissible
documentation: the api docs should be moved to the -devel package
headers: ok
static libs: n/a
pkgconfig files: ok
shared libraries: ok
devel package: ok
libtool archives: ok
gui apps: n/a
file ownership: ok
%install: ok
utf8 filenames: ok


Approved, but please move the api docs to the -devel package.

I'll also note that the first attemot to run the test gui yields a segfault for
me, the second run succeeds. Something to look into...


Comment 12 Peter Robinson 2008-04-28 10:06:47 EDT
I've move the api docs out to the devel rpm.

SPEC: http://fedora.roving-it.com/rawhide/geoclue.spec
SRPM: http://fedora.roving-it.com/rawhide/geoclue-0.11.1-4.fc9.src.rpm

I'm seeing the same issues with the test gui as well, I've filed an upstream
bug. b.f.o bug # 15745
Comment 13 Matthias Clasen 2008-04-28 10:22:07 EDT
I've already set the approval flag, so you can proceed and request cvs now,
unless you need sponsorship, in which case I cannot help :-(
Comment 14 Matthias Clasen 2008-05-05 08:43:55 EDT
Peter, have you made any progress towards finding a sponsor yet ?
Comment 15 Peter Robinson 2008-05-06 09:17:24 EDT
No, not having much luck on that front, also been very busy with other things
too. I'm not 100% on the best way to get sponsorship, although have been looking
at reviews for other packages but haven't posted any updates to them as yet. Any
other pointers for sponsorship?
Comment 16 Matthias Clasen 2008-05-12 19:22:54 EDT
I have recently become a sponsor, so I can now sponsor you.

The canonical information about this is 
http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored

You'll have to set up a Fedora Account first. For that, see 

http://fedoraproject.org/wiki/PackageMaintainers/Join#head-a601c13b0950a89568deafa65f505b4b58ee869b
Comment 17 Peter Robinson 2008-05-13 06:32:40 EDT
Hi Matthias,

I already have a fedoraprojects account. Username is pbrobinson
Comment 18 Matthias Clasen 2008-05-13 20:20:54 EDT
After some head scratching, I have figured out that you need to go the account
system ( http://admin.fedoraproject.org/accounts ), login and request membership
in the cvsextras group. 
Comment 19 Matthias Clasen 2008-05-13 21:01:01 EDT
Actually, I could do it myself. You should be able to proceed and request cvs now.
Comment 20 Peter Robinson 2008-05-15 09:01:10 EDT
New Package CVS Request
=======================
Package Name: geoclue
Short Description: Geoclue is a modular geoinformation service
Owners: pbrobinson
Branches: F-8 F-9
InitialCC: 
Cvsextras Commits: yes
Comment 21 Peter Robinson 2008-05-15 10:03:29 EDT
I think I inadvertently closed this by accident  
Comment 22 Kevin Fenzi 2008-05-15 12:11:57 EDT
cvs done (with "a modular geoinformation service" as short description). 
Comment 23 Peter Robinson 2008-05-16 09:37:00 EDT
In CVS and built in Koji. As far as I can see this now will be pushed to
rawhide, and I need to request builds Bodhi for F9/F8?

https://koji.fedoraproject.org/koji/taskinfo?taskID=612341
Comment 24 Peter Robinson 2008-05-18 17:06:04 EDT
Fixed. geoclue is now in rawhide and Fedora-9 testing updates. Updated package
to include sub packages for gypsy and gpsd

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