Bug 241387 - Review Request: codeina - GStreamer Codec Installation Application
Summary: Review Request: codeina - GStreamer Codec Installation Application
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Ray Strode [halfline]
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-05-25 16:17 UTC by Bastien Nocera
Modified: 2007-11-30 22:12 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-08-22 21:11:00 UTC
Type: ---
Embargoed:
rstrode: fedora-review+
wtogami: fedora-cvs+


Attachments (Terms of Use)

Description Bastien Nocera 2007-05-25 16:17:03 UTC
Spec URL: http://www.hadess.net/tmp/codeina/codeina.spec
SRPM URL: http://www.hadess.net/tmp/codeina/codeina-0.10.0.1-0.20070525.170400.src.rpm
Description: Codeina installs codecs from the Fluendo webshop for GStreamer.

Based on ThomasVS' package, with today's SVN snapshot.

Comment 1 Matthias Clasen 2007-05-29 20:15:55 UTC
First look:

- packaging guidelines strongly encourage a full source url

- shouldn't the python stuff go in /usr/lib/python2.5 ?

- either way, as long as it lives in libdir, the package cannot be noarch


There are also some missing dependencies, apparently:

[mclasen@dhcp83-186 devel]$ codeina 
GTK Accessibility Module initialized
Traceback (most recent call last):
  File "/usr/bin/codeina", line 101, in <module>
    _run_codeina()
  File "/usr/bin/codeina", line 89, in _run_codeina
    from codeina import main
  File "/usr/lib/codeina/python/codeina/main.py", line 36, in <module>
    from codeina import configure, check, install, myutils, productlist
  File "/usr/lib/codeina/python/codeina/install.py", line 32, in <module>
    import myutils
  File "/usr/lib/codeina/python/codeina/myutils.py", line 29, in <module>
    from xdg import BaseDirectory
ImportError: No module named xdg


Comment 2 Bastien Nocera 2007-05-29 23:21:11 UTC
(In reply to comment #1)
> First look:
> 
> - packaging guidelines strongly encourage a full source url

No releases done yet, so no URL.

> - shouldn't the python stuff go in /usr/lib/python2.5 ?

I'd rather it went in /usr/share altogether actually.

> - either way, as long as it lives in libdir, the package cannot be noarch

So I should move the python code to /usr/share (there's no compiled bits).

> There are also some missing dependencies, apparently:
> 
> [mclasen@dhcp83-186 devel]$ codeina 
<snip>
> ImportError: No module named xdg

Yeah, it's missing pyxdg.

Comment 3 Bastien Nocera 2007-05-30 11:25:36 UTC
I've mentioned that the code should be moved to the python directories, or
$(datadir) upstream.

Comment 4 Matthias Clasen 2007-05-30 12:37:10 UTC
Ok, I'll run down the review checklist once there is a srpm with these issues
fixed (well, the source url thing was already explained away)

Comment 5 Matthias Clasen 2007-06-01 02:31:41 UTC
See http://fedoraproject.org/wiki/PackagingDrafts/Python for the official gospel
of where to put python and how to put that in the spec.

Comment 6 Matthias Clasen 2007-06-04 19:02:44 UTC
Any update on this, Bastien ?

Comment 7 Bastien Nocera 2007-06-04 20:01:03 UTC
Nothing yet, waiting on the Codeina guys to update their code (Edward is
supposed to work on it soon).

Comment 8 Bastien Nocera 2007-08-21 17:59:44 UTC
Updated package and spec:
http://people.redhat.com/bnocera/codeina/codeina-0.10.0.1-0.20070525.170400.2.src.rpm
http://people.redhat.com/bnocera/codeina/codeina.spec

It took me a while to work around codeina's broken dir names, but I managed it
in the end. The source SVN wasn't updated as there were no changes since the
last SVN snapshot.

Comment 9 Bastien Nocera 2007-08-21 18:04:44 UTC
Removing the fedora_cvs flag, I turned it on accidentally.

Comment 10 Warren Togami 2007-08-21 19:05:04 UTC
With the absence of a full URL to source, could you at least include anonymous
checkout instructions in a comment above it?


Comment 11 Bastien Nocera 2007-08-21 21:13:00 UTC
(In reply to comment #10)
> With the absence of a full URL to source, could you at least include anonymous
> checkout instructions in a comment above it?

Sure, I'll add the details we have from the CodecBuddy wiki page.

Comment 12 Ray Strode [halfline] 2007-08-21 21:18:24 UTC
maybe add a %{?dist} tag?

what's the 
BuildRequires:  gcc-c++
followed by:
BuildArch:      noarch

about?

You use $RPM_BUILD_ROOT in some places and %{buildroot} in others, which is a
little inconsistent

mock build fails with 

Possible unintended interpolation of @INTLTOOL_ICONV in string at
../intltool-merge line 96
Global symbol "@INTLTOOL_ICONV" requires explicit package name at
../intltool-merge line 96.

not sure what that's about


Comment 13 Bastien Nocera 2007-08-21 22:43:35 UTC
(In reply to comment #12)
> maybe add a %{?dist} tag?
> 
> what's the 
> BuildRequires:  gcc-c++
> followed by:
> BuildArch:      noarch
> 
> about?

You didn't see the "sigh, libtool" comment. Basically, libtool needed it, and
complained if it's not there. Not the case anymore though, thus removed.

> You use $RPM_BUILD_ROOT in some places and %{buildroot} in others, which is a
> little inconsistent

Fixed.

> mock build fails with 
> 
> Possible unintended interpolation of @INTLTOOL_ICONV in string at
> ../intltool-merge line 96
> Global symbol "@INTLTOOL_ICONV" requires explicit package name at
> ../intltool-merge line 96.
> 
> not sure what that's about

Version mismatch between the tarball intltool and the system one. Running
intltoolize fixes it.

Build properly under mock.

http://people.redhat.com/bnocera/codeina/codeina-0.10.0.1-0.20070525.170400.3.fc8.src.rpm
http://people.redhat.com/bnocera/codeina/codeina.spec

Comment 14 Ray Strode [halfline] 2007-08-22 13:58:36 UTC
Only thing that bothers me a little is

install -D -m 0755 %{SOURCE1}
$RPM_BUILD_ROOT%{_libexecdir}/gst-install-plugins-helper.sh

should that go into %{_libexecdir}/codeina/gst-install-plugins-helper.sh

?  Regardless of the answer to that question looks good.

Comment 15 Bastien Nocera 2007-08-22 14:07:36 UTC
(In reply to comment #14)
> Only thing that bothers me a little is
> 
> install -D -m 0755 %{SOURCE1}
> $RPM_BUILD_ROOT%{_libexecdir}/gst-install-plugins-helper.sh
> 
> should that go into %{_libexecdir}/codeina/gst-install-plugins-helper.sh

No, gst-install-plugins-helper is the hook for gstreamer uses to call the plugin
helper, so this needs to match the path used in GStreamer.

> ?  Regardless of the answer to that question looks good.

The question, or the package? :)

Comment 16 Bastien Nocera 2007-08-22 14:13:03 UTC
New Package CVS Request
=======================
Package Name: codeina
Short Description: GStreamer Codec Installation Application
Owners: hadess thomasvs
Branches: devel
InitialCC: bnocera thomas
Commits by cvsextras: yes

Comment 17 Bastien Nocera 2007-08-22 21:11:00 UTC
It was missing a gawk BR (is it not in the minimum builds anymore?), pushed to
rawhide.


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