Bug 454668 - Review Request: gupnp-vala - vala bindings for gupnp
Summary: Review Request: gupnp-vala - vala bindings for gupnp
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
low
Target Milestone: ---
Assignee: Richard W.M. Jones
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 446639 447457 454664
Blocks: 488096
TreeView+ depends on / blocked
 
Reported: 2008-07-09 16:34 UTC by Peter Robinson
Modified: 2009-03-18 19:02 UTC (History)
4 users (show)

Fixed In Version: 0.5.3-4.fc10
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-03-10 23:45:04 UTC
Type: ---
Embargoed:
rjones: fedora-review+
dennis: fedora-cvs+


Attachments (Terms of Use)
Patch to add BR libsoup-devel (784 bytes, patch)
2009-01-14 11:15 UTC, Richard W.M. Jones
no flags Details | Diff
Some dependencies fixes, plus changes for noarch (1.71 KB, patch)
2009-03-02 16:20 UTC, Michel Lind
no flags Details | Diff

Description Peter Robinson 2008-07-09 16:34:56 UTC
SPEC: http://pbrobinson.fedorapeople.org/gupnp-vala.spec
SRPM: http://pbrobinson.fedorapeople.org/gupnp-vala-0.2-1.fc9.src.rpm

GUPnP is an object-oriented open source framework for creating UPnP
devices and control points, written in C using GObject and libsoup.
The GUPnP API is intended to be easy to use, efficient and flexible.

This package adds vala language bindings

Comment 1 Richard W.M. Jones 2008-12-16 12:45:54 UTC
Missing BuildRequires on vala-tools (for vapigen).

After that it fails to build:

make[2]: Entering directory `/home/rjones/rpmbuild/BUILD/gupnp-vala-0.2/tests'
    VALAC test-publisher
    VALAC server-test
    VALAC proxy-test
    VALAC browsing-test
    VALAC test-browser
server-test.vala:80.9-80.41: error: use of possibly unassigned local variable `filter'
make[2]: *** [server-test] Error 1
make[2]: *** Waiting for unfinished jobs....
/tmp/ccmQDEmL.o: In function `test_browsing_test_main':
browsing-test.c:(.text+0xb1): undefined reference to `g_thread_init'
collect2: ld returned 1 exit status
error: cc exited with status 256
make[2]: *** [browsing-test] Error 1
/tmp/ccA2KrmW.o: In function `test_proxy_test_main':
proxy-test.c:(.text+0x6b): undefined reference to `g_thread_init'
collect2: ld returned 1 exit status
error: cc exited with status 256
make[2]: *** [proxy-test] Error 1
make[2]: Leaving directory `/home/rjones/rpmbuild/BUILD/gupnp-vala-0.2/tests'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/home/rjones/rpmbuild/BUILD/gupnp-vala-0.2'
make: *** [all] Error 2

? rpmlint output
? package name satisfies the packaging naming guidelines
? specfile name matches the package base name
? package should satisfy packaging guidelines
? license meets guidelines and is acceptable to Fedora
? license matches the actual package license
? %doc includes license file
? spec file written in American English
? spec file is legible
+ upstream sources match sources in the srpm
  a7b78c99346ac4dd79847a060ac3cfd8
? package successfully builds on at least one architecture
? ExcludeArch bugs filed
? BuildRequires list all build dependencies
? %find_lang instead of %{_datadir}/locale/*
? binary RPM with shared library files must call ldconfig in %post and %postun
? does not use Prefix: /usr
? package owns all directories it creates
? no duplicate files in %files
? %defattr line
? %clean contains rm -rf $RPM_BUILD_ROOT
? consistent use of macros
? package must contain code or permissible content
? large documentation files should go in -doc subpackage
? files marked %doc should not affect package
? header files should be in -devel
? static libraries should be in -static
? packages containing pkgconfig (.pc) files need 'Requires: pkgconfig'
? libfoo.so must go in -devel
? -devel must require the fully versioned base
? packages should not contain libtool .la files
? packages containing GUI apps must include %{name}.desktop file
? packages must not own files or directories owned by other packages
? %install must start with rm -rf %{buildroot} etc.
? filenames must be valid UTF-8

Optional:

? if there is no license file, packager should query upstream
? translations of description and summary for non-English languages, if available
? reviewer should build the package in mock
? the package should build into binary RPMs on all supported architectures
? review should test the package functions as described
? scriptlets should be sane
? pkgconfig files should go in -devel
? shouldn't have file dependencies outside /etc /bin /sbin /usr/bin or /usr/sbin

Comment 2 Peter Robinson 2008-12-31 23:15:28 UTC
New upstream release that fixes build issues. SPEC is same location.
SRPM: http://pbrobinson.fedorapeople.org/gupnp-vala-0.4-1.fc10.src.rpm

Comment 4 Richard W.M. Jones 2009-01-14 09:26:16 UTC
OK I'll look at this today.

Comment 5 Richard W.M. Jones 2009-01-14 11:15:07 UTC
Created attachment 328966 [details]
Patch to add BR libsoup-devel

You need the patch attached here.  I will continue the review
assuming you have added this patch.

Comment 6 Richard W.M. Jones 2009-01-14 11:22:13 UTC
- rpmlint output

gupnp-vala.x86_64: E: zero-length /usr/share/vala/vapi/gssdp-1.0.deps
gupnp-vala.x86_64: E: no-binary
gupnp-vala-debuginfo.x86_64: E: empty-debuginfo-package
3 packages and 0 specfiles checked; 3 errors, 0 warnings.

I think the first & third errors are real ones which
need to be looked at.  Not sure about the 'no-binary'
error.

+ package name satisfies the packaging naming guidelines
+ specfile name matches the package base name
+ package should satisfy packaging guidelines
+ license meets guidelines and is acceptable to Fedora
- license matches the actual package license

No, the license is LGPLv2+

+ %doc includes license file
+ spec file written in American English
+ spec file is legible
+ upstream sources match sources in the srpm
  26f9c6d5de9a300cf2ec3cc04313e2ea 104744
+ package successfully builds on at least one architecture
  x86_64
n/a ExcludeArch bugs filed
+ BuildRequires list all build dependencies

(assuming you add the BR in the patch in comment 5)

+ %find_lang instead of %{_datadir}/locale/*
n/a binary RPM with shared library files must call ldconfig in %post and %postun
+ does not use Prefix: /usr
+ package owns all directories it creates
+ no duplicate files in %files
+ %defattr line
+ %clean contains rm -rf $RPM_BUILD_ROOT
+ consistent use of macros
+ package must contain code or permissible content
n/a large documentation files should go in -doc subpackage
+ files marked %doc should not affect package
n/a header files should be in -devel
n/a static libraries should be in -static
n/a packages containing pkgconfig (.pc) files need 'Requires: pkgconfig'
n/a libfoo.so must go in -devel
n/a -devel must require the fully versioned base
n/a packages should not contain libtool .la files
n/a packages containing GUI apps must include %{name}.desktop file
+ packages must not own files or directories owned by other packages
+ %install must start with rm -rf %{buildroot} etc.
+ filenames must be valid UTF-8

Optional:

n/a if there is no license file, packager should query upstream
n/a translations of description and summary for non-English languages, if
available
- reviewer should build the package in mock
- the package should build into binary RPMs on all supported architectures
- review should test the package functions as described
n/a scriptlets should be sane
n/a pkgconfig files should go in -devel
+ shouldn't have file dependencies outside /etc /bin /sbin /usr/bin or
/usr/sbin

A few things to fix there.

Comment 7 Peter Robinson 2009-01-20 14:48:45 UTC
> gupnp-vala.x86_64: E: zero-length /usr/share/vala/vapi/gssdp-1.0.deps
> gupnp-vala.x86_64: E: no-binary
> gupnp-vala-debuginfo.x86_64: E: empty-debuginfo-package
> 3 packages and 0 specfiles checked; 3 errors, 0 warnings.
> 
> I think the first & third errors are real ones which
> need to be looked at.  Not sure about the 'no-binary'
> error.

I've queried the first one upstream and awaiting a response but I'm not sure whether the 3rd one is actually an issue as vala actually generates C code which is then compiled do I don't think language bindings would generate executable code and hence there wouldn't be any debuginfo. See http://live.gnome.org/Vala/ for basic details. As I can't see any vala specific guidelines at https://fedoraproject.org/wiki/Packaging/Guidelines I'm going to query fedora-devel

> - license matches the actual package license
> 
> No, the license is LGPLv2+
 
I've fixed this locally but I haven't put up a new version until I've fixed/clarified the others above.

Comment 8 Richard W.M. Jones 2009-02-24 11:24:07 UTC
Any progress on this one?

Comment 9 Peter Robinson 2009-02-24 12:28:18 UTC
I've got no response from my email about vala packaging guidelines to fedora-devel so not quite sure where to take it from here.

Comment 10 Peter Robinson 2009-02-26 00:15:43 UTC
I've updated the package with the latest version. 

SPEC: http://pbrobinson.fedorapeople.org/gupnp-vala.spec
SRPM: http://pbrobinson.fedorapeople.org/gupnp-vala-0.5.3-1.fc10.src.rpm

As this package is language bindings for a language that generates C code for compiling I don't believe the rpmlint output is really valid. The files included are all for development not running of apps.

$ rpmlint /home/perobinson/rpmbuild/RPMS/x86_64/gupnp-vala-0.5.3-1.fc10.x86_64.rpm
gupnp-vala.x86_64: W: devel-file-in-non-devel-package /usr/lib64/pkgconfig/gupnp-vala-1.0.pc
gupnp-vala.x86_64: E: no-binary
1 packages and 0 specfiles checked; 1 errors, 1 warnings.

Comment 11 Michel Lind 2009-03-02 15:03:31 UTC
gupnp-vala is indeed a development-only package, and it's too bad that rpmlint cannot really handle that.

I'll be using it for building rygel (review request coming soon, will block on this); could I help with any remaining issue? (I maintain the main vala packages)

Comment 12 Peter Robinson 2009-03-02 15:12:14 UTC
Hi Michal, I actually have a rygel package built that I was planning on putting in for a review request too.

Comment 13 Michel Lind 2009-03-02 15:33:03 UTC
It's Michel :) Could you submit the rygel package for review? I'll take a look at it.

Some comments on gupnp-vala (indented because I copied-and-pasted; I tried submitting right after your reply)


    I just filed a bug report for gupnp-devel's missing dependencies:
    https://bugzilla.redhat.com/show_bug.cgi?id=488078

    You might want to note it in the spec; once this bug is closed, the BR on
    libsoup-devel can go away. The BR on gssdp-devel can already be removed, since
    it's pulled in by gupnp-devel.

    Looks like gupnp-devel need to be depended on directly, due to the versioning
    requirement.

    Also, this should probably be noarch, and Requires: vala. Unfortunately, until
    the gupnp packages in updates-testing land, we'd have to Koji-build this on
    Rawhide only. I'm testing the suggested changes right now:

    http://koji.fedoraproject.org/koji/taskinfo?taskID=1213450

Comment 14 Michel Lind 2009-03-02 15:50:25 UTC
Updating libgee right now, for rigel.

Looks like my noarch suggestion does not work -- configure does not like noarch-redhat-linux. Any idea how to solve this (if the package is indeed noarch)?

I'll redo the build, with arch, and then compare the generated files.

Comment 15 Michel Lind 2009-03-02 16:20:30 UTC
Created attachment 333757 [details]
Some dependencies fixes, plus changes for noarch

%configure builds for noarch just fine if we override --target. This is safe, as the generated files are bit-by-bit identical anyway.

I've turned on unit tests as well, even though none seem to be enabled at the moment (make check returns really quickly).

Comment 16 Peter Robinson 2009-03-02 16:47:13 UTC
Sorry, I didn't mean to get your name wrong but I was typing this as I was running out the door for a fire alarm. 

I'll do the review request for rygel shortly and also look at the gupnp-devel bug shortly. For the noarch its on my ToDo list to investigate before the F11 beta but the last couple of weeks at work has been hectic!

Comment 17 Richard W.M. Jones 2009-03-10 15:04:31 UTC
I'm happy with this package now.

All the issues which I found in comment 6 have been corrected
by the new package in comment 10.

I'm also happy if you wish to apply the patch from comment 15.

---------------------------------
So, APPROVED by rjones
---------------------------------

Comment 18 Peter Robinson 2009-03-10 15:56:30 UTC
New Package CVS Request
=======================
Package Name: gupnp-vala
Short Description: Vala language bindings for the GUPnP framework.
Owners: pbrobinson
Branches: F-10 EL-5
InitialCC:

Comment 19 Peter Robinson 2009-03-10 15:59:30 UTC
Thanks Richard. Will add Michel's patch shortly.

Comment 20 Dennis Gilmore 2009-03-10 20:18:37 UTC
CVS Done

Comment 21 Peter Robinson 2009-03-10 23:05:21 UTC
Imported and built in rawhide

Comment 22 Fedora Update System 2009-03-10 23:42:51 UTC
gupnp-vala-0.5.3-4.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/gupnp-vala-0.5.3-4.fc10

Comment 23 Peter Robinson 2009-03-10 23:45:04 UTC
Closing as its headed to rawhide and submitted as an update for F-10

Comment 24 Fedora Update System 2009-03-18 19:02:03 UTC
gupnp-vala-0.5.3-4.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.


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