Bug 241387 - Review Request: codeina - GStreamer Codec Installation Application
Review Request: codeina - GStreamer Codec Installation Application
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Ray Strode [halfline]
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-05-25 12:17 EDT by Bastien Nocera
Modified: 2007-11-30 17:12 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-08-22 17:11:00 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
rstrode: fedora‑review+
wtogami: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Bastien Nocera 2007-05-25 12:17:03 EDT
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 16:15:55 EDT
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 19:21:11 EDT
(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 07:25:36 EDT
I've mentioned that the code should be moved to the python directories, or
$(datadir) upstream.
Comment 4 Matthias Clasen 2007-05-30 08:37:10 EDT
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-05-31 22:31:41 EDT
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 15:02:44 EDT
Any update on this, Bastien ?
Comment 7 Bastien Nocera 2007-06-04 16:01:03 EDT
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 13:59:44 EDT
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 14:04:44 EDT
Removing the fedora_cvs flag, I turned it on accidentally.
Comment 10 Warren Togami 2007-08-21 15:05:04 EDT
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 17:13:00 EDT
(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 17:18:24 EDT
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 18:43:35 EDT
(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 09:58:36 EDT
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 10:07:36 EDT
(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 10:13:03 EDT
New Package CVS Request
=======================
Package Name: codeina
Short Description: GStreamer Codec Installation Application
Owners: hadess thomasvs
Branches: devel
InitialCC: bnocera@redhat.com thomas@apestaart.org
Commits by cvsextras: yes
Comment 17 Bastien Nocera 2007-08-22 17:11:00 EDT
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.